On 07/04/20(Tue) 19:58, Vitaliy Makkoveev wrote:
>
>
> > On 7 Apr 2020, at 17:43, Martin Pieuchot <m...@openbsd.org> wrote:
> >
> > On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote:
> >> As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4)
> >> code has some NET_LOCK() dances which make it unsafe. [...]
> >
> > The easiest way to fix that is to move if_detach() out of pppx_if_destroy().
>
> pppxioctl() is under KERNEL_LOCK(). pppxioctl() holds NET_LOCK() it’s whole
> runtime. So your suggestion is (pseudo code):
>
> KERNEL_LOCK();
>
> pppx_ioctl()
> {
> NET_LOCK();
>
> switch() {
> case PIPEXDSESSION:
> NET_UNLOCK();
> /*
> * KERNEL_LOCK() can be released here and concurrent
> * thread with PIPEXDSESSION case will enter here
> */
> if_detach();
> NET_LOCK();
>
> pppx_if_destroy();
> break;
> }
>
> NET_UNLOCK();
> }
>
> KERNEL_UNLOCK();
>
> I understood well?
That or the other way around. The question is who can grab a reference
on `ifp'? What is currently serializing this? Is the NET_LOCK() at all
necessary in pppx_ioctl() what is it protecting? All the pipex(4) code
seems to be running under KERNEL_LOCK() anyway.
I'm wondering if it isn't simpler to answer those questions and fix the
current code once pppx(4) is using pipex_add_session().
