Page MenuHomeFreeBSD

Move a racy assertion in filt_pipewrite().
ClosedPublic

Authored by markj on Feb 17 2019, 5:19 PM.

Details

Summary

Note that pipe_kqfilter() adds the knote to the list for the other end
of the pipe when initializing an EVFILT_WRITE knote. In particular,
the knote does not hold any kind of strong reference that prevents
pipeclose() from being called on the other end. pipeclose() removes
all knotes from that end's knlist, and after that happens,
kn_list_lock() is a no-op, so it's possible for filt_pipewrite() to be
called without the knlist lock held.

Move the assertion in filt_pipewrite() to handle this: we can safely
check for PIPE_EOF and pipe_present != PIPE_ACTIVE without the lock
since those are "sticky" states, i.e., PIPE_EOF is never cleared once it
is set, and pipe_state is only updated when the pipe is being closed.

Test Plan

Peter reported the panic caused by the failed assertion, and tested the patch.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 22543
Build 21684: arc lint + arc unit

Event Timeline

markj added reviewers: kib, mjg.

Why cannot the same happen for read side ?

This revision is now accepted and ready to land.Feb 17 2019, 5:42 PM
In D19224#411306, @kib wrote:

Why cannot the same happen for read side ?

pipe_kqfilter() does not handle EVFILT_READ and EVFILT_WRITE symmetrically. In the read case, the knote itself holds a reference on that end of the pipe, so pipeclose() will never be called.

In D19224#411306, @kib wrote:

Why cannot the same happen for read side ?

pipe_kqfilter() does not handle EVFILT_READ and EVFILT_WRITE symmetrically. In the read case, the knote itself holds a reference on that end of the pipe, so pipeclose() will never be called.

I should say, pipeclose() will not be called before the knote is dropped.

This revision was automatically updated to reflect the committed changes.