Date: Sun, 30 May 1999 19:18:53 -0700 (PDT) From: Matthew Dillon <dillon@apollo.backplane.com> To: Alan Cox <alc@cs.rice.edu> Cc: hackers@freebsd.org Subject: Re: pipe race Message-ID: <199905310218.TAA71169@apollo.backplane.com> References: <19990530152238.A81441@nonpc.cs.rice.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
Ouch. Definite problems in both the reader and the writer.
The writer calculates the writable space before locking, so if you
have two simultanious writers where one blocks in uiomove(), both
try to write to the same buffer area. *plus* the counters get
screwed up (and would probably lead to a crash)
The reader has the same problem.
If the writer is blocked or switches out of uiomove(), it can get
confused if the reader attempts to reset its buffer pointers at the
same time ( prior to the write completing ):
if ((rpipe->pipe_busy == 0) && (rpipe->pipe_state & PIPE_WANT)) {
( this condition may not occur because the writer could
be blocked or switched in uiomove(), so PIPE_WANT will
not be set )
rpipe->pipe_state &= ~(PIPE_WANT|PIPE_WANTW);
wakeup(rpipe);
} else if (rpipe->pipe_buffer.cnt < MINPIPESIZE) {
/*
* If there is no more to read in the pipe, reset
* its pointers to the beginning. This improves
* cache hit stats.
*/
if (rpipe->pipe_buffer.cnt == 0) {
if ((error == 0) && (error = pipelock(rpipe,1)) == 0) {
rpipe->pipe_buffer.in = 0;
rpipe->pipe_buffer.out = 0;
pipeunlock(rpipe);
}
}
The writer can also get confused if it tries to resize the buffer. It
calculates whether to resize the buffer prior to locking, then resizes
the buffer without rechecking the calculations.
I should have patches soon. I believe the reader-resetting-buffer-pointers
problem is what caused my friend's data corruption.
-Matt
:I took a look at the pipe code this morning. Whether or not this is
:the cause of your problem, I don't know, but there sure are snippets
:of code that set off alarms in my head, e.g.,
:
:from pipe_read:
:
: /*
: * normal pipe buffer receive
: */
: if (rpipe->pipe_buffer.cnt > 0) {
: size = rpipe->pipe_buffer.size - rpipe->pipe_buffer.out;
: if (size > rpipe->pipe_buffer.cnt)
: size = rpipe->pipe_buffer.cnt;
: if (size > (u_int) uio->uio_resid)
: size = (u_int) uio->uio_resid;
: if ((error = pipelock(rpipe,1)) == 0) {
: error = uiomove( &rpipe->pipe_buffer.buffer[rpipe->pipe_buffer.out],
: size, uio);
: pipeunlock(rpipe);
: }
:
:Suppose you block in pipelock. Who's to say the "size" is still
:valid when you wake up?
:
Matthew Dillon
<dillon@backplane.com>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199905310218.TAA71169>