Re: knlist_cleardel() panic

On Sun, Mar 02, 2008 at 04:37:06PM -0800, John-Mark Gurney wrote:
> Kostik Belousov wrote this message on Sun, Mar 02, 2008 at 22:06 +0200:
> > On Sun, Mar 02, 2008 at 11:53:34AM -0800, John-Mark Gurney wrote:
> > > Kostik Belousov wrote this message on Sun, Mar 02, 2008 at 17:25 +0200:
> > > > The panic below was already reported, but now I got it on my desktop
> > > > and was able to investigate further.
> > > > 
> > > > #5  0xc06daf36 in trap (frame=0xe8093b1c)
> > > >     at /usr/bsd/src/sys/i386/i386/trap.c:490
> > > > #6  0xc06c0b4b in calltrap () at /usr/bsd/src/sys/i386/i386/exception.s:139
> > > > #7  0xc0493968 in knlist_cleardel (knl=0xcabec128, td=0x0, islocked=1, 
> > > >     killkn=0) at atomic.h:149
> > > > #8  0xc04f520c in pipeclose (cpipe=0xcabec0b8)
> > > >     at /usr/bsd/src/sys/kern/sys_pipe.c:1508
> > > > #9  0xc04f5320 in pipe_close (fp=0xc5ce8630, td=0xcac01aa0)
> > > >     at /usr/bsd/src/sys/kern/sys_pipe.c:1425
> > > > #10 0xc0489442 in fdrop (fp=0xc5ce8630, td=0xcac01aa0) at file.h:297
> > > > #11 0xc048accf in closef (fp=0xc5ce8630, td=0xcac01aa0)
> > > >     at /usr/bsd/src/sys/kern/kern_descrip.c:1958
> > > > #12 0xc048b1ff in kern_close (td=0xcac01aa0, fd=10)
> > > >     at /usr/bsd/src/sys/kern/kern_descrip.c:1054
> > > > #13 0xc048b2da in close (td=0xcac01aa0, uap=0xe8093cfc)
> > > >     at /usr/bsd/src/sys/kern/kern_descrip.c:1006
> > > > ---Type <return> to continue, or q <return> to quit---
> > > > #14 0xc06da865 in syscall (frame=0xe8093d38)
> > > >     at /usr/bsd/src/sys/i386/i386/trap.c:1035
> > > > #15 0xc06c0bb0 in Xint0x80_syscall ()
> > > >     at /usr/bsd/src/sys/i386/i386/exception.s:196
> > > > 
> > > > At the frame 8, we have
> > > > (kgdb) p/x *(knl->kl_list->slh_first)
> > > > $9 = {kn_link = {sle_next = 0x0}, kn_selnext = {sle_next = 0x0},
> > > >   kn_knlist = 0x0, kn_tqe = {tqe_next = 0xc58de484, tqe_prev = 0xc5e9ab20},
> > > >   kn_kq = 0x0, kn_kevent = {ident = 0x0, filter = 0x0, flags = 0x0,
> > > >     fflags = 0x0, data = 0x0, udata = 0x0}, kn_status = 0x20,
> > > >   kn_sfflags = 0x0, kn_sdata = 0x0, kn_ptr = {p_fp = 0x0, p_proc = 0x0,
> > > >     p_aio = 0x0, p_lio = 0x0}, kn_fop = 0x0, kn_hook = 0x0}
> > > > 
> > > > The knote is KN_MARKER, and the kn_kq is NULL. The result is that KQ_LOCK
> > > > in the knlist_cleardel()::SLIST_FOREACH_SAFE() loop dereferences NULL and
> > > > panics.
> > > > 
> > > > Does the following change makes any sense?
> > > 
> > > I thought this was a bug, but upon further examination, there is
> > > something wrong...  a KN_MARKER knote should never be on the knlist..
> > > It is only ever added to kq's even list, never to an object's list...
> > > 
> > > If you could walk the knl->kl_list through kn_selnext, and ensure
> > > that the kn w/ kN_MARKER exists would be good...  It should be
> > > last one as kn_selnext is NULL...
> > 
> > As I shown above, the KN_MARKER was found on the cpipe->pipe_sel.si_note,
> > 
> > (kgdb) p cpipe->pipe_sel.si_note
> > $1 = {kl_list = {slh_first = 0xc58df330}, 
> >   kl_lock = 0xc0493a20 <knlist_mtx_lock>, 
> >   kl_unlock = 0xc0493370 <knlist_mtx_unlock>, 
> >   kl_locked = 0xc0493350 <knlist_mtx_locked>, kl_lockarg = 0xcabec170
> > }
> > 
> > (kgdb) p/x (knl->kl_list->slh_first)
> > $2 = 0xc58df330
> > 
> > (kgdb) p/x *(knl->kl_list->slh_first)
> > $3 = {kn_link = {sle_next = 0x0}, kn_selnext = {sle_next = 0x0}, 
> >   kn_knlist = 0x0, kn_tqe = {tqe_next = 0xc58de484, tqe_prev = 0xc5e9ab20}, 
> >   kn_kq = 0x0, kn_kevent = {ident = 0x0, filter = 0x0, flags = 0x0, 
> >     fflags = 0x0, data = 0x0, udata = 0x0}, kn_status = 0x20, 
> >   kn_sfflags = 0x0, kn_sdata = 0x0, kn_ptr = {p_fp = 0x0, p_proc = 0x0, 
> >     p_aio = 0x0, p_lio = 0x0}, kn_fop = 0x0, kn_hook = 0x0}
> > > 
> > > I can't think of a way that this can happen, as the only way to get
> > > on the knlist is by calling knlist_add, and it should only ever be
> > > called from f_attach, which is called in one place, and has to have
> > > kn_kq set properly...  Are there any custom patches on the system?
> > 
> > This is the stock RELENG_7 as of today.
> 
> Do you have a reproducable test case that demonstrates this?
> 
> You can try the attached patch, but I'm pretty sure that it won't be
> triggered...
> 
> -- 
>   John-Mark Gurney				Voice: +1 415 225 5579
> 
>      "All that I will do, has been done, All that I have, has not."

> --- kern_event.c.orig	2007-07-14 14:23:30.000000000 -0700
> +++ kern_event.c	2008-03-02 16:26:59.000000000 -0800
> _at__at_ -1590,6 +1590,8 _at__at_
>  	KQ_NOTOWNED(kn->kn_kq);
>  	KASSERT((kn->kn_status & (KN_INFLUX|KN_DETACHED)) ==
>  	    (KN_INFLUX|KN_DETACHED), ("knote not KN_INFLUX and KN_DETACHED"));
> +	KASSERT((kn->kn_status & (KN_MARKER)) == 0,
> +	    ("knote has KN_MARKER set"));
>  	if (!islocked)
>  		knl->kl_lock(knl->kl_lockarg);
>  	SLIST_INSERT_HEAD(&knl->kl_list, kn, kn_selnext);

When the pipe_peer is being closed, and the pipeclose() has set the
pipe_present = 0 and enters knlist_cleardel(), the PIPE_MTX may be
dropped when knl->kl_list cannot be cleared due to influx knotes.

If the following often encountered code fragment
		if (!(kn->kn_status & KN_DETACHED))
			kn->kn_fop->f_detach(kn);
		knote_drop(kn, td); [1]
is executed while the knlist lock is dropped, then the knote memory is freed
by the knote_drop() [1] without knote being removed from the knlist, since
the filt_pipedetach() contains the following:
	if (kn->kn_filter == EVFILT_WRITE) {
		if (!cpipe->pipe_peer->pipe_present) {
			PIPE_UNLOCK(cpipe);
			return;

Now, the memory may be reused in the zone, for instance, for the marker
knote. And, we ending up the marker on the knlist.

Does this look plausible ?

Thanks.