bvs@light.uucp (Bakul Shah) (09/22/89)
The attached program fails most of the time on V7, 4.3 BSD & Sun 3
systems; which seems to indicate the bug was inherited from V7.
(SVR3 seems to handle it right -- I haven't tried others).
Upon forking file-descriptors are shared by the parent & child.
But if a shared fd is to a regular file and both read from it at
the same time, they get the *same* data instead of serial pieces
of the file (also, the next few bytes are skipped). There is a
similar problem with write. The problem is where fp->f_offset is
updated once read/write is finished. It should be updated while
the inode is locked.
-- Bakul Shah <..!{ames,sun,ucbvax,uunet}!amdcad!light!bvs>
#! /bin/sh
cat > x.c <<EOF
#include <stdio.h>
extern void * malloc();
main(argc, argv)
int argc;
char * * argv;
{
char * buf;
int count, f, g;
unsigned int size;
/* default to stdin if no argv[1] or it is "-" */
f = (argc > 1 && strcmp(argv[1], "-")) ? open(argv[1]) : 0;
if (f < 0) { perror(argv[1]); exit(1); }
/* default to stdout if no argv[2] or it is "-" */
g = (argc > 2 && strcmp(argv[2], "-")) ? creat(argv[2], 0666) : 1;
if (g < 0) { perror(argv[2]); exit(1); }
/* default to 1 byte buffer if no argv[3] */
size = argc > 3 ? atoi(argv[3]) : 1;
buf = (char *)malloc(size);
if (!buf) { fprintf(stderr, "not enough space\n"); exit(1); }
fork();
while ((count = read(f, buf, size)) > 0)
write(g, buf, count);
}
EOF
cc x.c
# The correct output of a.out will be a permutation of input.
# On a V7 system pipes seem to have the same bug.
a.out x.c
chris@mimsy.UUCP (Chris Torek) (09/23/89)
In article <1989Sep22.160808.1407@light.uucp> bvs@light.uucp (Bakul Shah) writes: >The problem is where fp->f_offset is updated once read/write is finished. >It should be updated while the inode is locked. This bug was fixed quite some time ago (perhaps in 4.3-tahoe, perhaps a bit afterward). Here is the fix. Your lines numbers may not match. *** /tmp/,RCSt1003823 Sat Sep 23 00:28:18 1989 --- /tmp/,RCSt2003823 Sat Sep 23 00:28:21 1989 *************** *** 4,8 **** * specifies the terms and conditions for redistribution. * ! * @(#)sys_inode.c 7.1 (Berkeley) 6/5/86 */ --- 4,8 ---- * specifies the terms and conditions for redistribution. * ! * @(#)sys_inode.c 7.5.1.1 (Berkeley) 11/24/87 */ *************** *** 36,49 **** { register struct inode *ip = (struct inode *)fp->f_data; ! int error; ! if ((ip->i_mode&IFMT) == IFREG) { ILOCK(ip); ! if (fp->f_flag&FAPPEND && rw == UIO_WRITE) ! uio->uio_offset = fp->f_offset = ip->i_size; ! error = rwip(ip, uio, rw); IUNLOCK(ip); - } else - error = rwip(ip, uio, rw); return (error); } --- 36,53 ---- { register struct inode *ip = (struct inode *)fp->f_data; ! int count, error; ! if ((ip->i_mode&IFMT) != IFCHR) ILOCK(ip); ! if ((ip->i_mode&IFMT) == IFREG && ! (fp->f_flag&FAPPEND) && ! rw == UIO_WRITE) ! fp->f_offset = ip->i_size; ! uio->uio_offset = fp->f_offset; ! count = uio->uio_resid; ! error = rwip(ip, uio, rw); ! fp->f_offset += count - uio->uio_resid; ! if ((ip->i_mode&IFMT) != IFCHR) IUNLOCK(ip); return (error); } *************** *** 148,152 **** bn = fsbtodb(fs, bmap(ip, lbn, rw == UIO_WRITE ? B_WRITE: B_READ, ! (int)(on+n))); if (u.u_error || rw == UIO_WRITE && (long)bn < 0) return (u.u_error); --- 152,156 ---- bn = fsbtodb(fs, bmap(ip, lbn, rw == UIO_WRITE ? B_WRITE: B_READ, ! (int)(on + n))); if (u.u_error || rw == UIO_WRITE && (long)bn < 0) return (u.u_error); *************** *** 253,256 **** --- 257,261 ---- fp->f_flag)); } + /* NOTREACHED */ } *************** *** 319,323 **** register struct file *fp; { ! register struct inode *ip = (struct inode *)fp->f_data; dev_t dev; int flag, mode; --- 324,328 ---- register struct file *fp; { ! struct inode *ip = (struct inode *)fp->f_data; dev_t dev; int flag, mode; *************** *** 331,335 **** * will prevent close. */ ! fp->f_data = (caddr_t) 0; /* XXX */ dev = (dev_t)ip->i_rdev; mode = ip->i_mode & IFMT; --- 336,340 ---- * will prevent close. */ ! fp->f_data = (caddr_t) 0; dev = (dev_t)ip->i_rdev; mode = ip->i_mode & IFMT; *************** *** 364,368 **** /* * We don't want to really close the device if it is mounted ! * of if we're swapping on it. */ /* MOUNT TABLE SHOULD HOLD INODE */ --- 369,373 ---- /* * We don't want to really close the device if it is mounted ! * or if we're swapping on it. */ /* MOUNT TABLE SHOULD HOLD INODE */ -- In-Real-Life: Chris Torek, Univ of MD Comp Sci Dept (+1 301 454 7163) Domain: chris@mimsy.umd.edu Path: uunet!mimsy!chris
chris@mimsy.UUCP (Chris Torek) (09/24/89)
In article <19757@mimsy.UUCP> I wrote: >This bug was fixed quite some time ago (perhaps in 4.3-tahoe, perhaps a >bit afterward). Here is the fix. Your lines numbers may not match. Oops, you have to change sys_generic.c as well: *** /tmp/,RCSt1006180 Sat Sep 23 15:49:38 1989 --- /tmp/,RCSt2006180 Sat Sep 23 15:49:41 1989 *************** *** 136,140 **** } count = uio->uio_resid; - uio->uio_offset = fp->f_offset; if (setjmp(&u.u_qsave)) { if (uio->uio_resid == count) { --- 137,140 ---- *************** *** 147,151 **** u.u_error = (*fp->f_ops->fo_rw)(fp, rw, uio); u.u_r.r_val1 = count - uio->uio_resid; - fp->f_offset += u.u_r.r_val1; } --- 147,150 ---- -- In-Real-Life: Chris Torek, Univ of MD Comp Sci Dept (+1 301 454 7163) Domain: chris@mimsy.umd.edu Path: uunet!mimsy!chris