Messages in this thread Patch in this message | | | Date | Sun, 16 Jul 2000 00:14:43 +0200 | From | Christian Ehrhardt <> | Subject | [PATCH] Race in pipe code |
| |
Hi,
I can prove the race now (sample code attached) and I have a patch that fixes it for me. If noone sees problems with the patch I'll send it to Linus. The patch was developped for 2.3.99-pre9 but it applies cleanly against 2.4.0-test4.
The nice(20) in the test program isn't needed, it just helps to trigger the race. About 50% of the time the program prints BBBBBBBBBB on stderr and 4000(!) A's on stdout which is clearly wrong. The patch basically makes sure that the same reader continues to read from the pipe after it went to sleep.
regards Christian
-- THAT'S ALL FOLKS! #include <unistd.h> #include <sys/types.h> #include <sys/wait.h>
int main () { int ret, pid, i; int fd[2]; char buf[5000];
pipe (fd); pid = fork (); if (!pid) { close (fd[0]); for (i=0; i<3000; i++) buf[i] = 'A'; write (fd[1], buf, 3000); for (i=0; i<10; i++) buf[i] = 'B'; write (fd[1], buf, 3000); wait (NULL); wait (NULL); } else { close (fd[1]); pid = fork (); if (!pid) { nice (20); ret = read (fd[0], buf, 4000); buf[ret] = '\n'; write (1, buf, ret+1); } else { ret = read (fd[0], buf, 10); buf[ret] = '\n'; write (2, buf, ret+1); } } return 0; } --- linux/include/linux/pipe_fs_i.h.vanilla Fri Jul 14 10:39:06 2000 +++ linux/include/linux/pipe_fs_i.h Sat Jul 15 21:15:50 2000 @@ -1,9 +1,12 @@ #ifndef _LINUX_PIPE_FS_I_H #define _LINUX_PIPE_FS_I_H +#include <asm/semaphore.h> + #define PIPEFS_MAGIC 0x50495045 struct pipe_inode_info { wait_queue_head_t wait; + struct semaphore read_sem; char *base; unsigned int start; unsigned int readers; @@ -20,6 +23,7 @@ #define PIPE_SEM(inode) (&(inode).i_sem) #define PIPE_WAIT(inode) (&(inode).i_pipe->wait) +#define PIPE_READ_SEM(inode) (&(inode).i_pipe->read_sem) #define PIPE_BASE(inode) ((inode).i_pipe->base) #define PIPE_START(inode) ((inode).i_pipe->start) #define PIPE_LEN(inode) ((inode).i_size) --- linux/fs/pipe.c.vanilla Sat Jul 15 21:14:00 2000 +++ linux/fs/pipe.c Sat Jul 15 22:01:55 2000 @@ -21,6 +21,11 @@ * * Reads with count = 0 should always return 0. * -- Julian Bradfield 1999-06-07. + * + * A single read should always return continuous bytes. + * Added PIPE_READ_SEM to fix a race where this wasn't true. + * -- Christian Ehrhardt 2000-07-15 + * */ /* Drop the inode semaphore and wait for a pipe event, atomically */ @@ -53,10 +58,12 @@ if (count == 0) goto out_nolock; - /* Get the pipe semaphore */ + /* Get the pipe semaphores */ ret = -ERESTARTSYS; - if (down_interruptible(PIPE_SEM(*inode))) + if (down_interruptible(PIPE_READ_SEM(*inode))) goto out_nolock; + if (down_interruptible(PIPE_SEM(*inode))) + goto out_readlock; if (PIPE_EMPTY(*inode)) { do_more_read: @@ -126,6 +133,8 @@ ret = read; out: up(PIPE_SEM(*inode)); +out_readlock: + up(PIPE_READ_SEM(*inode)); out_nolock: if (read) ret = read; @@ -453,6 +462,7 @@ goto fail_page; init_waitqueue_head(PIPE_WAIT(*inode)); + sema_init (PIPE_READ_SEM(*inode), 1); PIPE_BASE(*inode) = (char*) page; PIPE_START(*inode) = PIPE_LEN(*inode) = 0; PIPE_READERS(*inode) = PIPE_WRITERS(*inode) = 0;
| |