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)
                ...

> 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) {
> 

Reply via email to