d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Move a racy assertion in filt_pipewrite().

Browse Source 
EVFILT_WRITE knotes for pipes live on the knlist for the other end of the
pipe.  Since they do not hold a reference on the corresponding file
structure, they may be removed from the knlist by pipeclose() while still
remaining active.  In this case, there is no knlist lock acquired before
filt_pipewrite() is called, so the assertion fails.

Fix the problem by first checking whether that end of the pipe has been
closed.  These checks are memory safe since the knote holds a reference
on one end of the pipe, and the pipe structure is not freed until both
ends are closed.  The checks are not racy since PIPE_EOF is never cleared
after being set, and pipe_present is never set back to PIPE_ACTIVE after
pipeclose() has been called.

PR:		235640
Reported and tested by:	pho
Reviewed by:	kib
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D19224
This commit is contained in:
Mark Johnston 2019-02-19 15:46:43 +00:00
parent c9172fb4f1
commit 18a7de663b
1 changed files with 6 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
sys/kern/sys_pipe.c
View File

@ -1741,15 +1741,19 @@ static int
filt_pipewrite(struct knote *kn, long hint) filt_pipewrite(struct knote *kn, long hint)
{ {
struct pipe *wpipe; struct pipe *wpipe;
/*
* If this end of the pipe is closed, the knote was removed from the
* knlist and the list lock (i.e., the pipe lock) is therefore not held.
*/
wpipe = kn->kn_hook; wpipe = kn->kn_hook;
PIPE_LOCK_ASSERT(wpipe, MA_OWNED);
if (wpipe->pipe_present != PIPE_ACTIVE || if (wpipe->pipe_present != PIPE_ACTIVE ||
(wpipe->pipe_state & PIPE_EOF)) { (wpipe->pipe_state & PIPE_EOF)) {
kn->kn_data = 0; kn->kn_data = 0;
kn->kn_flags |= EV_EOF; kn->kn_flags |= EV_EOF;
return (1); return (1);
} }
PIPE_LOCK_ASSERT(wpipe, MA_OWNED);
kn->kn_data = (wpipe->pipe_buffer.size > 0) ? kn->kn_data = (wpipe->pipe_buffer.size > 0) ?
(wpipe->pipe_buffer.size - wpipe->pipe_buffer.cnt) : PIPE_BUF; (wpipe->pipe_buffer.size - wpipe->pipe_buffer.cnt) : PIPE_BUF;
if (wpipe->pipe_state & PIPE_DIRECTW) if (wpipe->pipe_state & PIPE_DIRECTW)