Martin Pieuchot wrote: > On 28/02/16(Sun) 13:14, Martin Natano wrote: > > The ext2fs_read() and ffs_read() functions return EFBIG when uio_offset > > if smaller than zero or larger than the maxium file size. However this > > doesn't seem to be in accordance with the POSIX read(2) specification, > > which requires EINVAL for an invalid offset (< 2) and a return of 0 > > (zero bytes transferred) for an offset that's at or paste the EOF. EFBIG > > actually means "File too large", which is clearly not true in both > > cases. > > This seems to be coherent with MSDOSFS. > > I'm also wondering when you say "an offset that's at or paste the > EOF" does that include ``uio_resid''? I mean shouldn't you check > for: > > if ((uio->uio_offset + uio->uio_resid) > fs->fs_maxfilesize) > ...
I think if uio->uio_offset < fs->fs_maxfilesize, then min(uio->uio_resid, fs->fs_maxfilesize - uio->uio_offset) bytes should be read. Maybe the check should be if (uio_offset >= fs->fs_maxfilesize) though. Otherwise access flags of the inode are changed at the end if ext2_ind_read() and ffs_read() even though there was no real access. > > Index: ufs/ext2fs/ext2fs_readwrite.c > > =================================================================== > > RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_readwrite.c,v > > retrieving revision 1.39 > > diff -u -p -r1.39 ext2fs_readwrite.c > > --- ufs/ext2fs/ext2fs_readwrite.c 27 Feb 2016 18:50:38 -0000 1.39 > > +++ ufs/ext2fs/ext2fs_readwrite.c 28 Feb 2016 09:10:16 -0000 > > @@ -100,9 +100,9 @@ ext2_ind_read(struct vnode *vp, struct i > > } else if (vp->v_type != VREG && vp->v_type != VDIR) > > panic("%s: type %d", "ext2fs_read", vp->v_type); > > #endif > > - if (e2fs_overflow(fs, 0, uio->uio_offset)) > > - return (EFBIG); > > - if (uio->uio_resid == 0) > > + if (uio->uio_offset < 0) > > + return (EINVAL); > > + if (uio->uio_offset > fs->e2fs_maxfilesize || uio->uio_resid == 0) > > return (0); > > > > for (error = 0, bp = NULL; uio->uio_resid > 0; bp = NULL) { > > @@ -163,7 +163,6 @@ ext4_ext_read(struct vnode *vp, struct i > > struct ext4_extent_path path; > > struct ext4_extent nex, *ep; > > struct buf *bp; > > - size_t orig_resid; > > daddr_t lbn, pos; > > off_t bytesinfile; > > int size, xfersize, blkoffset; > > @@ -171,12 +170,10 @@ ext4_ext_read(struct vnode *vp, struct i > > > > memset(&path, 0, sizeof path); > > > > - orig_resid = uio->uio_resid; > > - if (orig_resid == 0) > > + if (uio->uio_offset < 0) > > + return (EINVAL); > > + if (uio->uio_offset > fs->e2fs_maxfilesize || uio->uio_resid == 0) > > return (0); > > - > > - if (e2fs_overflow(fs, 0, uio->uio_offset)) > > - return (EFBIG); > > > > while (uio->uio_resid > 0) { > > if ((bytesinfile = ext2fs_size(ip) - uio->uio_offset) <= 0) > > Index: ufs/ffs/ffs_vnops.c > > =================================================================== > > RCS file: /cvs/src/sys/ufs/ffs/ffs_vnops.c,v > > retrieving revision 1.84 > > diff -u -p -r1.84 ffs_vnops.c > > --- ufs/ffs/ffs_vnops.c 27 Feb 2016 18:50:38 -0000 1.84 > > +++ ufs/ffs/ffs_vnops.c 28 Feb 2016 09:10:16 -0000 > > @@ -214,10 +214,9 @@ ffs_read(void *v) > > panic("ffs_read: type %d", vp->v_type); > > #endif > > fs = ip->i_fs; > > - if ((u_int64_t)uio->uio_offset > fs->fs_maxfilesize) > > - return (EFBIG); > > - > > - if (uio->uio_resid == 0) > > + if (uio->uio_offset < 0) > > + return (EINVAL); > > + if (uio->uio_offset > fs->fs_maxfilesize || uio->uio_resid == 0) > > return (0); > > > > for (error = 0, bp = NULL; uio->uio_resid > 0; bp = NULL) { > > >