Author: markj Date: Sat Jun 29 16:05:52 2019 New Revision: 349546 URL: https://svnweb.freebsd.org/changeset/base/349546
Log: Fix mutual exclusion in pipe_direct_write(). We use PIPE_DIRECTW as a semaphore for direct writes to a pipe, where the reader copies data directly from pages mapped into the writer. However, when a reader finishes such a copy, it previously cleared PIPE_DIRECTW, allowing multiple writers to race and corrupt the state used to track wired pages belonging to the writer. Fix this by having the writer clear PIPE_DIRECTW and instead use the count of unread bytes to determine whether a write is finished. Reported by: syzbot+21811cc0a89b2a87a...@syzkaller.appspotmail.com Reviewed by: kib, mjg Tested by: pho MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D20784 Modified: head/sys/kern/sys_pipe.c Modified: head/sys/kern/sys_pipe.c ============================================================================== --- head/sys/kern/sys_pipe.c Sat Jun 29 15:28:36 2019 (r349545) +++ head/sys/kern/sys_pipe.c Sat Jun 29 16:05:52 2019 (r349546) @@ -724,7 +724,7 @@ pipe_read(struct file *fp, struct uio *uio, struct ucr rpipe->pipe_map.pos += size; rpipe->pipe_map.cnt -= size; if (rpipe->pipe_map.cnt == 0) { - rpipe->pipe_state &= ~(PIPE_DIRECTW|PIPE_WANTW); + rpipe->pipe_state &= ~PIPE_WANTW; wakeup(rpipe); } #endif @@ -855,13 +855,17 @@ pipe_build_write_buffer(struct pipe *wpipe, struct uio } /* - * unmap and unwire the process buffer + * Unwire the process buffer. */ static void pipe_destroy_write_buffer(struct pipe *wpipe) { PIPE_LOCK_ASSERT(wpipe, MA_OWNED); + KASSERT((wpipe->pipe_state & PIPE_DIRECTW) != 0, + ("%s: PIPE_DIRECTW not set on %p", __func__, wpipe)); + + wpipe->pipe_state &= ~PIPE_DIRECTW; vm_page_unhold_pages(wpipe->pipe_map.ms, wpipe->pipe_map.npages); wpipe->pipe_map.npages = 0; } @@ -880,13 +884,15 @@ pipe_clone_write_buffer(struct pipe *wpipe) int pos; PIPE_LOCK_ASSERT(wpipe, MA_OWNED); + KASSERT((wpipe->pipe_state & PIPE_DIRECTW) != 0, + ("%s: PIPE_DIRECTW not set on %p", __func__, wpipe)); + size = wpipe->pipe_map.cnt; pos = wpipe->pipe_map.pos; wpipe->pipe_buffer.in = size; wpipe->pipe_buffer.out = 0; wpipe->pipe_buffer.cnt = size; - wpipe->pipe_state &= ~PIPE_DIRECTW; PIPE_UNLOCK(wpipe); iov.iov_base = wpipe->pipe_buffer.buffer; @@ -925,7 +931,7 @@ retry: pipeunlock(wpipe); goto error1; } - while (wpipe->pipe_state & PIPE_DIRECTW) { + if (wpipe->pipe_state & PIPE_DIRECTW) { if (wpipe->pipe_state & PIPE_WANTR) { wpipe->pipe_state &= ~PIPE_WANTR; wakeup(wpipe); @@ -968,8 +974,7 @@ retry: goto error1; } - error = 0; - while (!error && (wpipe->pipe_state & PIPE_DIRECTW)) { + while (wpipe->pipe_map.cnt != 0) { if (wpipe->pipe_state & PIPE_EOF) { pipe_destroy_write_buffer(wpipe); pipeselwakeup(wpipe); @@ -987,20 +992,19 @@ retry: error = msleep(wpipe, PIPE_MTX(wpipe), PRIBIO | PCATCH, "pipdwt", 0); pipelock(wpipe, 0); + if (error != 0) + break; } if (wpipe->pipe_state & PIPE_EOF) error = EPIPE; - if (wpipe->pipe_state & PIPE_DIRECTW) { - /* - * this bit of trickery substitutes a kernel buffer for - * the process that might be going away. - */ + if (error == EINTR || error == ERESTART) pipe_clone_write_buffer(wpipe); - } else { + else pipe_destroy_write_buffer(wpipe); - } pipeunlock(wpipe); + KASSERT((wpipe->pipe_state & PIPE_DIRECTW) == 0, + ("pipe %p leaked PIPE_DIRECTW", wpipe)); return (error); error1: _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
