On Wed, Feb 17, 2016 at 11:27:29AM -0700, Todd C. Miller wrote:
> There is currently code that checks for mnt_maxsymlinklen <= 0.
> Removing the cast will cause other problems for ffs if the maxsymlinklen
> value is negative.  I don't think it is safe to make this change
> unless mnt_maxsymlinklen is made unsigned in struct mount and a
> check is added to the assignment of mnt_maxsymlinklen from
> fs_maxsymlinklen in ufs/ffs/ffs_vfsops.c to avoid assigning a
> negative value.

The diff below addresses the issues you mentioned. It converts
mnt_maxsymlinklen to unsigned and adds a check to ffs_validate() that
makes sure, that fs_maxsymlinklen is >= 0. That function is called
during mount and on fsck. This should make sure we won't get a bogus
fs_maxsymlinklen from the superblock.

natano


Index: sys/mount.h
===================================================================
RCS file: /cvs/src/sys/sys/mount.h,v
retrieving revision 1.121
diff -u -p -r1.121 mount.h
--- sys/mount.h 8 Sep 2014 01:47:06 -0000       1.121
+++ sys/mount.h 21 Feb 2016 00:44:11 -0000
@@ -355,7 +355,7 @@ struct mount {
        struct vnodelst mnt_vnodelist;          /* list of vnodes this mount */
        struct rwlock   mnt_lock;               /* mount structure lock */
        int             mnt_flag;               /* flags */
-       int             mnt_maxsymlinklen;      /* max size of short symlink */
+       unsigned int    mnt_maxsymlinklen;      /* max size of short symlink */
        struct statfs   mnt_stat;               /* cache of filesystem stats */
        void            *mnt_data;              /* private data */
 };
Index: ufs/ext2fs/ext2fs_readwrite.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_readwrite.c,v
retrieving revision 1.37
diff -u -p -r1.37 ext2fs_readwrite.c
--- ufs/ext2fs/ext2fs_readwrite.c       16 Feb 2016 17:56:12 -0000      1.37
+++ ufs/ext2fs/ext2fs_readwrite.c       21 Feb 2016 00:44:11 -0000
@@ -95,7 +95,7 @@ ext2_ind_read(struct vnode *vp, struct i
                panic("%s: mode", "ext2fs_read");
 
        if (vp->v_type == VLNK) {
-               if ((int)ext2fs_size(ip) < vp->v_mount->mnt_maxsymlinklen ||
+               if (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");
Index: ufs/ext2fs/ext2fs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_vnops.c,v
retrieving revision 1.74
diff -u -p -r1.74 ext2fs_vnops.c
--- ufs/ext2fs/ext2fs_vnops.c   16 Feb 2016 17:56:12 -0000      1.74
+++ ufs/ext2fs/ext2fs_vnops.c   21 Feb 2016 00:44:11 -0000
@@ -322,7 +322,7 @@ ext2fs_setattr(void *v)
        if (vap->va_mode != (mode_t)VNOVAL) {
                if (vp->v_mount->mnt_flag & MNT_RDONLY)
                        return (EROFS);
-               error = ext2fs_chmod(vp, (int)vap->va_mode, cred, p);
+               error = ext2fs_chmod(vp, vap->va_mode, cred, p);
        }
        return (error);
 }
Index: ufs/ffs/ffs_vfsops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.150
diff -u -p -r1.150 ffs_vfsops.c
--- ufs/ffs/ffs_vfsops.c        12 Jan 2016 11:41:00 -0000      1.150
+++ ufs/ffs/ffs_vfsops.c        21 Feb 2016 00:44:11 -0000
@@ -642,6 +642,9 @@ ffs_validate(struct fs *fsp)
        if ((u_int)fsp->fs_frag > MAXFRAG || fragtbl[fsp->fs_frag] == NULL)
                return (0); /* Invalid number of fragments */
 
+       if (fsp->fs_maxsymlinklen < 0)
+               return (0); /* Invalid max size of short symlink */
+
        return (1); /* Super block is okay */
 }
 
Index: ufs/ffs/ffs_vnops.c
===================================================================
RCS file: /cvs/src/sys/ufs/ffs/ffs_vnops.c,v
retrieving revision 1.82
diff -u -p -r1.82 ffs_vnops.c
--- ufs/ffs/ffs_vnops.c 16 Feb 2016 17:56:12 -0000      1.82
+++ ufs/ffs/ffs_vnops.c 21 Feb 2016 00:44:11 -0000
@@ -207,7 +207,7 @@ ffs_read(void *v)
                panic("ffs_read: mode");
 
        if (vp->v_type == VLNK) {
-               if ((int)DIP(ip, size) < vp->v_mount->mnt_maxsymlinklen ||
+               if (DIP(ip, size) < vp->v_mount->mnt_maxsymlinklen ||
                    (vp->v_mount->mnt_maxsymlinklen == 0 &&
                     DIP(ip, blocks) == 0))
                        panic("ffs_read: short symlink");

Reply via email to