On Mon, Dec 21, 2020 at 04:51:45PM -0300, Martin Pieuchot wrote:
> On 21/12/20(Mon) 16:45, Visa Hankala wrote:
> > There is a slight inconsistency in klist_invalidate(). If the knote is
> > already in the event queue and has flag EV_ONESHOT, kqueue_scan() will
> > not invoke the newly set f_event. In this case, the kevent(2) system
> > call will return the knote's original event state that no longer
> > reflects the state that is reachable through the file descriptor
> > (the caller of klist_invalidate() has already revoked access to the
> > file or device).
>
> I don't understand the problem. Why should filt_dead() be called? Is
> it a race between two threads? Would you mind giving a scenario or some
> code example?
klist_invalidate() often is asynchronous relative to the thread that
registered the knote, so yes, sort of a race is involved.
filt_dead() sets the knote's event flags and data. Without the added
call, filt_dead() might not be invoked and the knote's fields may
retain old state that then gets reported to the caller of kqueue_scan().
In fact, this applies to any knote that has EV_ONESHOT set because the
scan loop skips f_event when the flag is present.
The kn->kn_fop->f_event(kn, 0) followed by knote_activate(kn) is
actually similar to activation through knote().
>
> > I think a proper fix is to invoke f_event manually to force the state
> > update.
> >
> > OK?
> >
> > Index: kern/kern_event.c
> > ===================================================================
> > RCS file: src/sys/kern/kern_event.c,v
> > retrieving revision 1.153
> > diff -u -p -r1.153 kern_event.c
> > --- kern/kern_event.c 20 Dec 2020 12:54:05 -0000 1.153
> > +++ kern/kern_event.c 21 Dec 2020 16:19:30 -0000
> > @@ -1618,6 +1618,7 @@ klist_invalidate(struct klist *list)
> > kn->kn_fop->f_detach(kn);
> > if (kn->kn_fop->f_flags & FILTEROP_ISFD) {
> > kn->kn_fop = &dead_filtops;
> > + kn->kn_fop->f_event(kn, 0);
> > knote_activate(kn);
> > s = splhigh();
> > knote_release(kn);
> >
>
