ufs/ffs/ext2fs uiomove() conversion

[Martin Natano]

Below the conversion to uiomovei() for ufs. While there I changed all
instances of 'blkoffset', 'size' and 'xfersize' that where declared as
long integers to be plain integers instead for consistency with the
surrounding code. These variables are limited by fs_bsize and e2fs_bsize
respectively, which are int32_t. So, no overflow should be introduced bu
that change. Also, 'size' is passed to bread(), which expects an integer
parameter.

Index: ufs/ext2fs/ext2fs_lookup.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_lookup.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 ext2fs_lookup.c
--- ufs/ext2fs/ext2fs_lookup.c 14 Mar 2015 03:38:52 -0000 1.39
+++ ufs/ext2fs/ext2fs_lookup.c 7 Feb 2016 17:48:03 -0000
@@ -177,7 +177,7 @@ ext2fs_readdir(void *v)
break;
}
dstd.d_off = off + e2d_reclen;
- if ((error = uiomovei((caddr_t)&dstd, dstd.d_reclen, uio)) != 0) {
+ if ((error = uiomove((caddr_t)&dstd, dstd.d_reclen, uio)) != 0) {
break;
}
off = off + e2d_reclen;
Index: ufs/ext2fs/ext2fs_readwrite.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_readwrite.c,v
retrieving revision 1.36
diff -u -p -u -r1.36 ext2fs_readwrite.c
--- ufs/ext2fs/ext2fs_readwrite.c 12 Jan 2016 11:44:21 -0000 1.36
+++ ufs/ext2fs/ext2fs_readwrite.c 7 Feb 2016 17:48:03 -0000
@@ -87,7 +87,7 @@ ext2_ind_read(struct vnode *vp, struct i
struct buf *bp;
daddr_t lbn, nextlbn;
off_t bytesinfile;
- long size, xfersize, blkoffset;
+ int size, xfersize, blkoffset;
int error;

#ifdef DIAGNOSTIC
@@ -145,7 +145,7 @@ ext2_ind_read(struct vnode *vp, struct i
break;
xfersize = size;
}
- error = uiomovei((char *)bp->b_data + blkoffset, xfersize, uio);
+ error = uiomove((char *)bp->b_data + blkoffset, xfersize, uio);
if (error)
break;
brelse(bp);
@@ -168,7 +168,7 @@ ext4_ext_read(struct vnode *vp, struct i
size_t orig_resid;
daddr_t lbn, pos;
off_t bytesinfile;
- long size, xfersize, blkoffset;
+ int size, xfersize, blkoffset;
int error, cache_type;

memset(&path, 0, sizeof path);
@@ -225,7 +225,7 @@ ext4_ext_read(struct vnode *vp, struct i
}
xfersize = size;
}
- error = uiomovei(bp->b_data + blkoffset, xfersize, uio);
+ error = uiomove(bp->b_data + blkoffset, xfersize, uio);
brelse(bp);
if (error)
return (error);
@@ -248,7 +248,8 @@ ext2fs_write(void *v)
int32_t lbn;
off_t osize;
int blkoffset, error, flags, ioflag, size, xfersize;
- ssize_t resid, overrun;
+ size_t resid;
+ ssize_t overrun;

ioflag = ap->a_ioflag;
uio = ap->a_uio;
@@ -324,8 +325,7 @@ ext2fs_write(void *v)
if (size < xfersize)
xfersize = size;

- error =
- uiomovei((char *)bp->b_data + blkoffset, (int)xfersize, uio);
+ error = uiomove((char *)bp->b_data + blkoffset, xfersize, uio);
#if 0
if (ioflag & IO_NOCACHE)
bp->b_flags |= B_NOCACHE;
Index: ufs/ext2fs/ext2fs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_vnops.c,v
retrieving revision 1.73
diff -u -p -u -r1.73 ext2fs_vnops.c
--- ufs/ext2fs/ext2fs_vnops.c 17 Apr 2015 04:43:21 -0000 1.73
+++ ufs/ext2fs/ext2fs_vnops.c 7 Feb 2016 17:48:04 -0000
@@ -1116,12 +1116,12 @@ ext2fs_readlink(void *v)
struct vop_readlink_args *ap = v;
struct vnode *vp = ap->a_vp;
struct inode *ip = VTOI(vp);
- int isize;
+ u_int64_t isize;

isize = ext2fs_size(ip);
if (isize < vp->v_mount->mnt_maxsymlinklen ||
(vp->v_mount->mnt_maxsymlinklen == 0 && ip->i_e2fs_nblock == 0)) {
- return (uiomovei((char *)ip->i_e2din->e2di_shortlink, isize,
+ return (uiomove((char *)ip->i_e2din->e2di_shortlink, isize,
ap->a_uio));
}
return (VOP_READ(vp, ap->a_uio, 0, ap->a_cred));
Index: ufs/ffs/ffs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ffs/ffs_vnops.c,v
retrieving revision 1.81
diff -u -p -u -r1.81 ffs_vnops.c
--- ufs/ffs/ffs_vnops.c 12 Jan 2016 11:41:00 -0000 1.81
+++ ufs/ffs/ffs_vnops.c 7 Feb 2016 17:48:04 -0000
@@ -193,7 +193,7 @@ ffs_read(void *v)
struct buf *bp;
daddr_t lbn, nextlbn;
off_t bytesinfile;
- long size, xfersize, blkoffset;
+ int size, xfersize, blkoffset;
mode_t mode;
int error;

@@ -258,7 +258,7 @@ ffs_read(void *v)
break;
xfersize = size;
}
- error = uiomovei(bp->b_data + blkoffset, (int)xfersize, uio);
+ error = uiomove(bp->b_data + blkoffset, xfersize, uio);
if (error)
break;
brelse(bp);
@@ -287,7 +287,8 @@ ffs_write(void *v)
daddr_t lbn;
off_t osize;
int blkoffset, error, extended, flags, ioflag, size, xfersize;
- ssize_t resid, overrun;
+ size_t resid;
+ ssize_t overrun;

extended = 0;
ioflag = ap->a_ioflag;
@@ -362,8 +363,7 @@ ffs_write(void *v)
if (size < xfersize)
xfersize = size;

- error =
- uiomovei(bp->b_data + blkoffset, xfersize, uio);
+ error = uiomove(bp->b_data + blkoffset, xfersize, uio);

if (error != 0)
memset(bp->b_data + blkoffset, 0, xfersize);
Index: ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.124
diff -u -p -u -r1.124 ufs_vnops.c
--- ufs/ufs/ufs_vnops.c 4 Feb 2016 12:45:03 -0000 1.124
+++ ufs/ufs/ufs_vnops.c 7 Feb 2016 17:48:11 -0000
@@ -1491,7 +1491,7 @@ ufs_readdir(void *v)
memset(u.dn.d_name + u.dn.d_namlen, 0, u.dn.d_reclen
- u.dn.d_namlen - offsetof(struct dirent, d_name));

- error = uiomovei(&u.dn, u.dn.d_reclen, uio);
+ error = uiomove(&u.dn, u.dn.d_reclen, uio);
dp = (struct direct *)((char *)dp + dp->d_reclen);
}

@@ -1519,12 +1519,12 @@ ufs_readlink(void *v)
struct vop_readlink_args *ap = v;
struct vnode *vp = ap->a_vp;
struct inode *ip = VTOI(vp);
- int isize;
+ u_int64_t isize;

isize = DIP(ip, size);
if (isize < vp->v_mount->mnt_maxsymlinklen ||
(vp->v_mount->mnt_maxsymlinklen == 0 && DIP(ip, blocks) == 0)) {
- return (uiomovei((char *)SHORTLINK(ip), isize, ap->a_uio));
+ return (uiomove((char *)SHORTLINK(ip), isize, ap->a_uio));
}
return (VOP_READ(vp, ap->a_uio, 0, ap->a_cred));
}

[Stefan Kempf]

Post by Martin Natano
Below the conversion to uiomovei() for ufs. While there I changed all
instances of 'blkoffset', 'size' and 'xfersize' that where declared as
long integers to be plain integers instead for consistency with the
surrounding code. These variables are limited by fs_bsize and e2fs_bsize
respectively, which are int32_t. So, no overflow should be introduced bu
that change. Also, 'size' is passed to bread(), which expects an integer
parameter.

These all look good. It's easy to see that these variables have all
small max. values.

One unrelated thing noted while reviewing:

ufs/ext2fs/ext2fs_readwrite.c:
static int
ext2_ind_read(struct vnode *vp, struct inode *ip, struct m_ext2fs *fs,
struct uio *uio)
{
...

if (vp->v_type == VLNK) {
if ((int)ext2fs_size(ip) < vp->v_mount->mnt_maxsymlinklen ||
^^^^^

(vp->v_mount->mnt_maxsymlinklen == 0 &&
ip->i_e2fs_nblock == 0))
panic("%s: short symlink", "ext2fs_read");

ufs/ffs/ffs_vnops.c:
int
ffs_read(void *v)
{
...
if (vp->v_type == VLNK) {
if ((int)DIP(ip, size) < vp->v_mount->mnt_maxsymlinklen ||
^^^^
(vp->v_mount->mnt_maxsymlinklen == 0 &&
DIP(ip, blocks) == 0))
panic("ffs_read: short symlink");

This code checks that the filesystem read routines are not invoked on
symlinks where the destination path fits into the inode itself.
We should get rid of these truncating casts in a separate diff.

Post by Martin Natano
Index: ufs/ext2fs/ext2fs_lookup.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_lookup.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 ext2fs_lookup.c
--- ufs/ext2fs/ext2fs_lookup.c 14 Mar 2015 03:38:52 -0000 1.39
+++ ufs/ext2fs/ext2fs_lookup.c 7 Feb 2016 17:48:03 -0000
@@ -177,7 +177,7 @@ ext2fs_readdir(void *v)
break;
}
dstd.d_off = off + e2d_reclen;
- if ((error = uiomovei((caddr_t)&dstd, dstd.d_reclen, uio)) != 0) {
+ if ((error = uiomove((caddr_t)&dstd, dstd.d_reclen, uio)) != 0) {
break;
}
off = off + e2d_reclen;
Index: ufs/ext2fs/ext2fs_readwrite.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_readwrite.c,v
retrieving revision 1.36
diff -u -p -u -r1.36 ext2fs_readwrite.c
--- ufs/ext2fs/ext2fs_readwrite.c 12 Jan 2016 11:44:21 -0000 1.36
+++ ufs/ext2fs/ext2fs_readwrite.c 7 Feb 2016 17:48:03 -0000
@@ -87,7 +87,7 @@ ext2_ind_read(struct vnode *vp, struct i
struct buf *bp;
daddr_t lbn, nextlbn;
off_t bytesinfile;
- long size, xfersize, blkoffset;
+ int size, xfersize, blkoffset;
int error;
#ifdef DIAGNOSTIC
@@ -145,7 +145,7 @@ ext2_ind_read(struct vnode *vp, struct i
break;
xfersize = size;
}
- error = uiomovei((char *)bp->b_data + blkoffset, xfersize, uio);
+ error = uiomove((char *)bp->b_data + blkoffset, xfersize, uio);
if (error)
break;
brelse(bp);
@@ -168,7 +168,7 @@ ext4_ext_read(struct vnode *vp, struct i
size_t orig_resid;
daddr_t lbn, pos;
off_t bytesinfile;
- long size, xfersize, blkoffset;
+ int size, xfersize, blkoffset;
int error, cache_type;
memset(&path, 0, sizeof path);
@@ -225,7 +225,7 @@ ext4_ext_read(struct vnode *vp, struct i
}
xfersize = size;
}
- error = uiomovei(bp->b_data + blkoffset, xfersize, uio);
+ error = uiomove(bp->b_data + blkoffset, xfersize, uio);
brelse(bp);
if (error)
return (error);
@@ -248,7 +248,8 @@ ext2fs_write(void *v)
int32_t lbn;
off_t osize;
int blkoffset, error, flags, ioflag, size, xfersize;
- ssize_t resid, overrun;
+ size_t resid;
+ ssize_t overrun;
ioflag = ap->a_ioflag;
uio = ap->a_uio;
@@ -324,8 +325,7 @@ ext2fs_write(void *v)
if (size < xfersize)
xfersize = size;
- error =
- uiomovei((char *)bp->b_data + blkoffset, (int)xfersize, uio);
+ error = uiomove((char *)bp->b_data + blkoffset, xfersize, uio);
#if 0
if (ioflag & IO_NOCACHE)
bp->b_flags |= B_NOCACHE;
Index: ufs/ext2fs/ext2fs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_vnops.c,v
retrieving revision 1.73
diff -u -p -u -r1.73 ext2fs_vnops.c
--- ufs/ext2fs/ext2fs_vnops.c 17 Apr 2015 04:43:21 -0000 1.73
+++ ufs/ext2fs/ext2fs_vnops.c 7 Feb 2016 17:48:04 -0000
@@ -1116,12 +1116,12 @@ ext2fs_readlink(void *v)
struct vop_readlink_args *ap = v;
struct vnode *vp = ap->a_vp;
struct inode *ip = VTOI(vp);
- int isize;
+ u_int64_t isize;
isize = ext2fs_size(ip);
if (isize < vp->v_mount->mnt_maxsymlinklen ||
(vp->v_mount->mnt_maxsymlinklen == 0 && ip->i_e2fs_nblock == 0)) {
- return (uiomovei((char *)ip->i_e2din->e2di_shortlink, isize,
+ return (uiomove((char *)ip->i_e2din->e2di_shortlink, isize,
ap->a_uio));
}
return (VOP_READ(vp, ap->a_uio, 0, ap->a_cred));
Index: ufs/ffs/ffs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ffs/ffs_vnops.c,v
retrieving revision 1.81
diff -u -p -u -r1.81 ffs_vnops.c
--- ufs/ffs/ffs_vnops.c 12 Jan 2016 11:41:00 -0000 1.81
+++ ufs/ffs/ffs_vnops.c 7 Feb 2016 17:48:04 -0000
@@ -193,7 +193,7 @@ ffs_read(void *v)
struct buf *bp;
daddr_t lbn, nextlbn;
off_t bytesinfile;
- long size, xfersize, blkoffset;
+ int size, xfersize, blkoffset;
mode_t mode;
int error;
@@ -258,7 +258,7 @@ ffs_read(void *v)
break;
xfersize = size;
}
- error = uiomovei(bp->b_data + blkoffset, (int)xfersize, uio);
+ error = uiomove(bp->b_data + blkoffset, xfersize, uio);
if (error)
break;
brelse(bp);
@@ -287,7 +287,8 @@ ffs_write(void *v)
daddr_t lbn;
off_t osize;
int blkoffset, error, extended, flags, ioflag, size, xfersize;
- ssize_t resid, overrun;
+ size_t resid;
+ ssize_t overrun;
extended = 0;
ioflag = ap->a_ioflag;
@@ -362,8 +363,7 @@ ffs_write(void *v)
if (size < xfersize)
xfersize = size;
- error =
- uiomovei(bp->b_data + blkoffset, xfersize, uio);
+ error = uiomove(bp->b_data + blkoffset, xfersize, uio);
if (error != 0)
memset(bp->b_data + blkoffset, 0, xfersize);
Index: ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.124
diff -u -p -u -r1.124 ufs_vnops.c
--- ufs/ufs/ufs_vnops.c 4 Feb 2016 12:45:03 -0000 1.124
+++ ufs/ufs/ufs_vnops.c 7 Feb 2016 17:48:11 -0000
@@ -1491,7 +1491,7 @@ ufs_readdir(void *v)
memset(u.dn.d_name + u.dn.d_namlen, 0, u.dn.d_reclen
- u.dn.d_namlen - offsetof(struct dirent, d_name));
- error = uiomovei(&u.dn, u.dn.d_reclen, uio);
+ error = uiomove(&u.dn, u.dn.d_reclen, uio);
dp = (struct direct *)((char *)dp + dp->d_reclen);
}
@@ -1519,12 +1519,12 @@ ufs_readlink(void *v)
struct vop_readlink_args *ap = v;
struct vnode *vp = ap->a_vp;
struct inode *ip = VTOI(vp);
- int isize;
+ u_int64_t isize;
isize = DIP(ip, size);
if (isize < vp->v_mount->mnt_maxsymlinklen ||
(vp->v_mount->mnt_maxsymlinklen == 0 && DIP(ip, blocks) == 0)) {
- return (uiomovei((char *)SHORTLINK(ip), isize, ap->a_uio));
+ return (uiomove((char *)SHORTLINK(ip), isize, ap->a_uio));
}
return (VOP_READ(vp, ap->a_uio, 0, ap->a_cred));
}

[Martin Natano]

Post by Stefan Kempf
static int
ext2_ind_read(struct vnode *vp, struct inode *ip, struct m_ext2fs *fs,
struct uio *uio)
{
...
if (vp->v_type == VLNK) {
if ((int)ext2fs_size(ip) < vp->v_mount->mnt_maxsymlinklen ||
^^^^^
(vp->v_mount->mnt_maxsymlinklen == 0 &&
ip->i_e2fs_nblock == 0))
panic("%s: short symlink", "ext2fs_read");
int
ffs_read(void *v)
{
...
if (vp->v_type == VLNK) {
if ((int)DIP(ip, size) < vp->v_mount->mnt_maxsymlinklen ||
^^^^
(vp->v_mount->mnt_maxsymlinklen == 0 &&
DIP(ip, blocks) == 0))
panic("ffs_read: short symlink");
This code checks that the filesystem read routines are not invoked on
symlinks where the destination path fits into the inode itself.
We should get rid of these truncating casts in a separate diff.

I agree, the casts are wrong. Removing them would cause the compare to
be performed with u_int64_t parameters, which is IMHO the right thing to
do.