Author: fsu
Date: Mon Mar  4 10:42:25 2019
New Revision: 344751
URL: https://svnweb.freebsd.org/changeset/base/344751

Log:
  Make superblock reading logic more strict.
  
  Add more on-disk superblock consistency checks to ext2_compute_sb_data() 
function.
  It should decrease the probability of mounting filesystems with corrupted 
superblock data.
  
  Reviewed by:    pfg
  MFC after:      1 week
  
  Differential Revision:    https://reviews.freebsd.org/D19322

Modified:
  head/sys/fs/ext2fs/ext2_alloc.c
  head/sys/fs/ext2fs/ext2_extern.h
  head/sys/fs/ext2fs/ext2_vfsops.c
  head/sys/fs/ext2fs/ext2fs.h

Modified: head/sys/fs/ext2fs/ext2_alloc.c
==============================================================================
--- head/sys/fs/ext2fs/ext2_alloc.c     Mon Mar  4 06:43:00 2019        
(r344750)
+++ head/sys/fs/ext2fs/ext2_alloc.c     Mon Mar  4 10:42:25 2019        
(r344751)
@@ -457,7 +457,7 @@ noinodes:
 /*
  * 64-bit compatible getters and setters for struct ext2_gd from ext2fs.h
  */
-static uint64_t
+uint64_t
 e2fs_gd_get_b_bitmap(struct ext2_gd *gd)
 {
 
@@ -465,7 +465,7 @@ e2fs_gd_get_b_bitmap(struct ext2_gd *gd)
            gd->ext2bgd_b_bitmap);
 }
 
-static uint64_t
+uint64_t
 e2fs_gd_get_i_bitmap(struct ext2_gd *gd)
 {
 
@@ -754,7 +754,7 @@ ext2_hashalloc(struct inode *ip, int cg, long pref, in
        return (0);
 }
 
-static unsigned long
+static uint64_t
 ext2_cg_number_gdb_nometa(struct m_ext2fs *fs, int cg)
 {
 
@@ -768,7 +768,7 @@ ext2_cg_number_gdb_nometa(struct m_ext2fs *fs, int cg)
            EXT2_DESCS_PER_BLOCK(fs));
 }
 
-static unsigned long
+static uint64_t
 ext2_cg_number_gdb_meta(struct m_ext2fs *fs, int cg)
 {
        unsigned long metagroup;
@@ -784,7 +784,7 @@ ext2_cg_number_gdb_meta(struct m_ext2fs *fs, int cg)
        return (0);
 }
 
-static unsigned long
+uint64_t
 ext2_cg_number_gdb(struct m_ext2fs *fs, int cg)
 {
        unsigned long first_meta_bg, metagroup;

Modified: head/sys/fs/ext2fs/ext2_extern.h
==============================================================================
--- head/sys/fs/ext2fs/ext2_extern.h    Mon Mar  4 06:43:00 2019        
(r344750)
+++ head/sys/fs/ext2fs/ext2_extern.h    Mon Mar  4 10:42:25 2019        
(r344751)
@@ -91,6 +91,7 @@ int   ext2_dirrewrite(struct inode *,
 int    ext2_dirempty(struct inode *, ino_t, struct ucred *);
 int    ext2_checkpath(struct inode *, struct inode *, struct ucred *);
 int    ext2_cg_has_sb(struct m_ext2fs *fs, int cg);
+uint64_t       ext2_cg_number_gdb(struct m_ext2fs *fs, int cg);
 int    ext2_inactive(struct vop_inactive_args *);
 int    ext2_htree_add_entry(struct vnode *, struct ext2fs_direct_2 *,
            struct componentname *);
@@ -104,6 +105,8 @@ int ext2_htree_lookup(struct inode *, const char *, in
 int    ext2_search_dirblock(struct inode *, void *, int *, const char *, int,
            int *, doff_t *, doff_t *, doff_t *, struct ext2fs_searchslot *);
 uint32_t       e2fs_gd_get_ndirs(struct ext2_gd *gd);
+uint64_t       e2fs_gd_get_b_bitmap(struct ext2_gd *);
+uint64_t       e2fs_gd_get_i_bitmap(struct ext2_gd *);
 uint64_t       e2fs_gd_get_i_tables(struct ext2_gd *);
 void   ext2_sb_csum_set_seed(struct m_ext2fs *);
 int    ext2_sb_csum_verify(struct m_ext2fs *);

Modified: head/sys/fs/ext2fs/ext2_vfsops.c
==============================================================================
--- head/sys/fs/ext2fs/ext2_vfsops.c    Mon Mar  4 06:43:00 2019        
(r344750)
+++ head/sys/fs/ext2fs/ext2_vfsops.c    Mon Mar  4 10:42:25 2019        
(r344751)
@@ -98,7 +98,7 @@ VFS_SET(ext2fs_vfsops, ext2fs, 0);
 
 static int     ext2_check_sb_compat(struct ext2fs *es, struct cdev *dev,
                    int ronly);
-static int     compute_sb_data(struct vnode * devvp,
+static int     ext2_compute_sb_data(struct vnode * devvp,
                    struct ext2fs * es, struct m_ext2fs * fs);
 
 static const char *ext2_opts[] = { "acls", "async", "noatime", "noclusterr", 
@@ -321,7 +321,7 @@ ext2_check_sb_compat(struct ext2fs *es, struct cdev *d
 }
 
 static e4fs_daddr_t
-cg_location(struct m_ext2fs *fs, int number)
+ext2_cg_location(struct m_ext2fs *fs, int number)
 {
        int cg, descpb, logical_sb, has_super = 0;
 
@@ -350,82 +350,196 @@ cg_location(struct m_ext2fs *fs, int number)
            fs->e2fs->e2fs_first_dblock);
 }
 
+static int
+ext2_cg_validate(struct m_ext2fs *fs)
+{
+       uint64_t b_bitmap;
+       uint64_t i_bitmap;
+       uint64_t i_tables;
+       uint64_t first_block, last_block, last_cg_block;
+       struct ext2_gd *gd;
+       unsigned int i, cg_count;
+
+       first_block = fs->e2fs->e2fs_first_dblock;
+       last_cg_block = ext2_cg_number_gdb(fs, 0);
+       cg_count = fs->e2fs_gcount;
+
+       for (i = 0; i < fs->e2fs_gcount; i++) {
+               gd = &fs->e2fs_gd[i];
+
+               if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_FLEX_BG) ||
+                   i == fs->e2fs_gcount - 1) {
+                       last_block = fs->e2fs_bcount - 1;
+               } else {
+                       last_block = first_block +
+                           (EXT2_BLOCKS_PER_GROUP(fs) - 1);
+               }
+
+               if ((cg_count == fs->e2fs_gcount) &&
+                   !(gd->ext4bgd_flags & EXT2_BG_INODE_ZEROED))
+                       cg_count = i;
+
+               b_bitmap = e2fs_gd_get_b_bitmap(gd);
+               if (b_bitmap == 0) {
+                       printf("ext2fs: cg %u: block bitmap is zero\n", i);
+                       return (EINVAL);
+
+               }
+               if (b_bitmap <= last_cg_block) {
+                       printf("ext2fs: cg %u: block bitmap overlaps gds\n", i);
+                       return (EINVAL);
+               }
+               if (b_bitmap < first_block || b_bitmap > last_block) {
+                       printf("ext2fs: cg %u: block bitmap not in group, 
blk=%ju\n",
+                           i, b_bitmap);
+                       return (EINVAL);
+               }
+
+               i_bitmap = e2fs_gd_get_i_bitmap(gd);
+               if (i_bitmap == 0) {
+                       printf("ext2fs: cg %u: inode bitmap is zero\n", i);
+                       return (EINVAL);
+               }
+               if (i_bitmap <= last_cg_block) {
+                       printf("ext2fs: cg %u: inode bitmap overlaps gds\n", i);
+                       return (EINVAL);
+               }
+               if (i_bitmap < first_block || i_bitmap > last_block) {
+                       printf("ext2fs: cg %u: inode bitmap not in group 
blk=%ju\n",
+                           i, i_bitmap);
+                       return (EINVAL);
+               }
+
+               i_tables = e2fs_gd_get_i_tables(gd);
+               if (i_tables == 0) {
+                       printf("ext2fs: cg %u: inode table is zero\n", i);
+                       return (EINVAL);
+               }
+               if (i_tables <= last_cg_block) {
+                       printf("ext2fs: cg %u: inode talbes overlaps gds\n", i);
+                       return (EINVAL);
+               }
+               if (i_tables < first_block ||
+                   i_tables + fs->e2fs_itpg - 1 > last_block) {
+                       printf("ext2fs: cg %u: inode tables not in group 
blk=%ju\n",
+                           i, i_tables);
+                       return (EINVAL);
+               }
+
+               if (!EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_FLEX_BG))
+                       first_block += EXT2_BLOCKS_PER_GROUP(fs);
+       }
+
+       return (0);
+}
+
 /*
  * This computes the fields of the m_ext2fs structure from the
  * data in the ext2fs structure read in.
  */
 static int
-compute_sb_data(struct vnode *devvp, struct ext2fs *es,
+ext2_compute_sb_data(struct vnode *devvp, struct ext2fs *es,
     struct m_ext2fs *fs)
 {
-       int g_count = 0, error;
-       int i, j;
        struct buf *bp;
        uint32_t e2fs_descpb, e2fs_gdbcount_alloc;
+       int i, j;
+       int g_count = 0;
+       int error;
 
-       fs->e2fs_bcount = es->e2fs_bcount;
-       fs->e2fs_rbcount = es->e2fs_rbcount;
-       fs->e2fs_fbcount = es->e2fs_fbcount;
-       if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_64BIT)) {
-               fs->e2fs_bcount |= (uint64_t)(es->e4fs_bcount_hi) << 32;
-               fs->e2fs_rbcount |= (uint64_t)(es->e4fs_rbcount_hi) << 32;
-               fs->e2fs_fbcount |= (uint64_t)(es->e4fs_fbcount_hi) << 32;
+       /* Check checksum features */
+       if (EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_GDT_CSUM) &&
+           EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_METADATA_CKSUM)) {
+               printf("ext2fs: incorrect checksum features combination\n");
+               return (EINVAL);
        }
+
+       /* Precompute checksum seed for all metadata */
+       ext2_sb_csum_set_seed(fs);
+
+       /* Verify sb csum if possible */
+       if (EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_METADATA_CKSUM)) {
+               error = ext2_sb_csum_verify(fs);
+               if (error) {
+                       return (error);
+               }
+       }
+
+       /* Check for block size = 1K|2K|4K */
+       if (es->e2fs_log_bsize > 2) {
+               printf("ext2fs: bad block size: %d\n", es->e2fs_log_bsize);
+               return (EINVAL);
+       }
+
        fs->e2fs_bshift = EXT2_MIN_BLOCK_LOG_SIZE + es->e2fs_log_bsize;
        fs->e2fs_bsize = 1U << fs->e2fs_bshift;
        fs->e2fs_fsbtodb = es->e2fs_log_bsize + 1;
        fs->e2fs_qbmask = fs->e2fs_bsize - 1;
+
+       /* Check for fragment size */
+       if (es->e2fs_log_fsize >
+           (EXT2_MAX_FRAG_LOG_SIZE - EXT2_MIN_BLOCK_LOG_SIZE)) {
+               printf("ext2fs: invalid log cluster size: %u\n",
+                   es->e2fs_log_fsize);
+               return (EINVAL);
+       }
+
        fs->e2fs_fsize = EXT2_MIN_FRAG_SIZE << es->e2fs_log_fsize;
-       if (fs->e2fs_fsize)
-               fs->e2fs_fpb = fs->e2fs_bsize / fs->e2fs_fsize;
-       fs->e2fs_bpg = es->e2fs_bpg;
-       fs->e2fs_fpg = es->e2fs_fpg;
-       fs->e2fs_ipg = es->e2fs_ipg;
+       if (fs->e2fs_fsize != fs->e2fs_bsize) {
+               printf("ext2fs: fragment size (%u) != block size %u\n",
+                   fs->e2fs_fsize, fs->e2fs_bsize);
+               return (EINVAL);
+       }
+
+       fs->e2fs_fpb = fs->e2fs_bsize / fs->e2fs_fsize;
+
+       /* Check reserved gdt blocks for future filesystem expansion */
+       if (es->e2fs_reserved_ngdb > (fs->e2fs_bsize / 4)) {
+               printf("ext2fs: number of reserved GDT blocks too large: %u\n",
+                   es->e2fs_reserved_ngdb);
+               return (EINVAL);
+       }
+
        if (es->e2fs_rev == E2FS_REV0) {
                fs->e2fs_isize = E2FS_REV0_INODE_SIZE;
        } else {
                fs->e2fs_isize = es->e2fs_inode_size;
 
                /*
+                * Check first ino.
+                */
+               if (es->e2fs_first_ino < EXT2_FIRSTINO) {
+                       printf("ext2fs: invalid first ino: %u\n",
+                           es->e2fs_first_ino);
+                       return (EINVAL);
+               }
+
+               /*
                 * Simple sanity check for superblock inode size value.
                 */
                if (EXT2_INODE_SIZE(fs) < E2FS_REV0_INODE_SIZE ||
                    EXT2_INODE_SIZE(fs) > fs->e2fs_bsize ||
                    (fs->e2fs_isize & (fs->e2fs_isize - 1)) != 0) {
-                       printf("ext2fs: invalid inode size %d\n",
+                       printf("ext2fs: invalid inode size %u\n",
                            fs->e2fs_isize);
-                       return (EIO);
+                       return (EINVAL);
                }
        }
-       /* Check for extra isize in big inodes. */
-       if (EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_EXTRA_ISIZE) &&
-           EXT2_INODE_SIZE(fs) < sizeof(struct ext2fs_dinode)) {
-               printf("ext2fs: no space for extra inode timestamps\n");
-               return (EINVAL);
-       }
-       /* Check checksum features */
-       if (EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_GDT_CSUM) &&
-           EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_METADATA_CKSUM)) {
-               printf("ext2fs: incorrect checksum features combination\n");
-               return (EINVAL);
-       }
-       /* Check for group descriptor size */
+
+       /* Check group descriptors */
        if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_64BIT) &&
-           (es->e3fs_desc_size != sizeof(struct ext2_gd))) {
-               printf("ext2fs: group descriptor size unsupported %d\n",
-                   es->e3fs_desc_size);
-               return (EINVAL);
+           es->e3fs_desc_size != E2FS_64BIT_GD_SIZE) {
+                       printf("ext2fs: unsupported 64bit descriptor size %u\n",
+                           es->e3fs_desc_size);
+                       return (EINVAL);
        }
-       /* Check for block size = 1K|2K|4K */
-       if (es->e2fs_log_bsize > 2) {
-               printf("ext2fs: bad block size: %d\n", es->e2fs_log_bsize);
+
+       fs->e2fs_bpg = es->e2fs_bpg;
+       fs->e2fs_fpg = es->e2fs_fpg;
+       if (fs->e2fs_bpg == 0 || fs->e2fs_fpg == 0) {
+               printf("ext2fs: zero blocks/fragments per group\n");
                return (EINVAL);
        }
-       /* Check for group size */
-       if (fs->e2fs_bpg == 0) {
-               printf("ext2fs: zero blocks per group\n");
-               return (EINVAL);
-       }
        if (fs->e2fs_bpg != fs->e2fs_bsize * 8) {
                printf("ext2fs: non-standard group size unsupported %d\n",
                    fs->e2fs_bpg);
@@ -433,26 +547,56 @@ compute_sb_data(struct vnode *devvp, struct ext2fs *es
        }
 
        fs->e2fs_ipb = fs->e2fs_bsize / EXT2_INODE_SIZE(fs);
-       if (fs->e2fs_ipg == 0) {
-               printf("ext2fs: zero inodes per group\n");
+       if (fs->e2fs_ipb == 0 ||
+           fs->e2fs_ipb > fs->e2fs_bsize / E2FS_REV0_INODE_SIZE) {
+               printf("ext2fs: bad inodes per block size\n");
                return (EINVAL);
        }
-       fs->e2fs_itpg = fs->e2fs_ipg / fs->e2fs_ipb;
-       /* Check for block consistency */
-       if (es->e2fs_first_dblock >= fs->e2fs_bcount) {
-               printf("ext2fs: invalid first data block\n");
+
+       fs->e2fs_ipg = es->e2fs_ipg;
+       if (fs->e2fs_ipg < fs->e2fs_ipb || fs->e2fs_ipg >  fs->e2fs_bsize * 8) {
+               printf("ext2fs: invalid inodes per group: %u\n", fs->e2fs_ipb);
                return (EINVAL);
        }
+
+       fs->e2fs_itpg = fs->e2fs_ipg / fs->e2fs_ipb;
+
+       fs->e2fs_bcount = es->e2fs_bcount;
+       fs->e2fs_rbcount = es->e2fs_rbcount;
+       fs->e2fs_fbcount = es->e2fs_fbcount;
+       if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_64BIT)) {
+               fs->e2fs_bcount |= (uint64_t)(es->e4fs_bcount_hi) << 32;
+               fs->e2fs_rbcount |= (uint64_t)(es->e4fs_rbcount_hi) << 32;
+               fs->e2fs_fbcount |= (uint64_t)(es->e4fs_fbcount_hi) << 32;
+       }
        if (fs->e2fs_rbcount > fs->e2fs_bcount ||
            fs->e2fs_fbcount > fs->e2fs_bcount) {
                printf("ext2fs: invalid block count\n");
                return (EINVAL);
        }
-       /* s_resuid / s_resgid ? */
+       if (es->e2fs_first_dblock >= fs->e2fs_bcount) {
+               printf("ext2fs: first data block out of range\n");
+               return (EINVAL);
+       }
+
        fs->e2fs_gcount = howmany(fs->e2fs_bcount - es->e2fs_first_dblock,
            EXT2_BLOCKS_PER_GROUP(fs));
+       if (fs->e2fs_gcount > ((uint64_t)1 << 32) - EXT2_DESCS_PER_BLOCK(fs)) {
+               printf("ext2fs: groups count too large: %u\n", fs->e2fs_gcount);
+               return (EINVAL);
+       }
+
+       /* Check for extra isize in big inodes. */
+       if (EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_EXTRA_ISIZE) &&
+           EXT2_INODE_SIZE(fs) < sizeof(struct ext2fs_dinode)) {
+               printf("ext2fs: no space for extra inode timestamps\n");
+               return (EINVAL);
+       }
+
+       /* s_resuid / s_resgid ? */
+
        if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_64BIT)) {
-               e2fs_descpb = fs->e2fs_bsize / sizeof(struct ext2_gd);
+               e2fs_descpb = fs->e2fs_bsize / E2FS_64BIT_GD_SIZE;
                e2fs_gdbcount_alloc = howmany(fs->e2fs_gcount, e2fs_descpb);
        } else {
                e2fs_descpb = fs->e2fs_bsize / E2FS_REV0_GD_SIZE;
@@ -467,7 +611,7 @@ compute_sb_data(struct vnode *devvp, struct ext2fs *es
 
        for (i = 0; i < fs->e2fs_gdbcount; i++) {
                error = bread(devvp,
-                   fsbtodb(fs, cg_location(fs, i)),
+                   fsbtodb(fs, ext2_cg_location(fs, i)),
                    fs->e2fs_bsize, NOCRED, &bp);
                if (error) {
                        free(fs->e2fs_contigdirs, M_EXT2MNT);
@@ -489,9 +633,13 @@ compute_sb_data(struct vnode *devvp, struct ext2fs *es
                brelse(bp);
                bp = NULL;
        }
-       /* Precompute checksum seed for all metadata */
-       ext2_sb_csum_set_seed(fs);
-       /* Verfy cg csum */
+
+       /* Validate cgs consistency */
+       error = ext2_cg_validate(fs);
+       if (error)
+               return (error);
+
+       /* Verfy cgs csum */
        if (EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_GDT_CSUM) ||
            EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_METADATA_CKSUM)) {
                error = ext2_gd_csum_verify(fs, devvp->v_rdev);
@@ -578,7 +726,7 @@ ext2_reload(struct mount *mp, struct thread *td)
        fs = VFSTOEXT2(mp)->um_e2fs;
        bcopy(bp->b_data, fs->e2fs, sizeof(struct ext2fs));
 
-       if ((error = compute_sb_data(devvp, es, fs)) != 0) {
+       if ((error = ext2_compute_sb_data(devvp, es, fs)) != 0) {
                brelse(bp);
                return (error);
        }
@@ -715,7 +863,7 @@ ext2_mountfs(struct vnode *devvp, struct mount *mp)
            M_EXT2MNT, M_WAITOK);
        mtx_init(EXT2_MTX(ump), "EXT2FS", "EXT2FS Lock", MTX_DEF);
        bcopy(es, ump->um_e2fs->e2fs, (u_int)sizeof(struct ext2fs));
-       if ((error = compute_sb_data(devvp, ump->um_e2fs->e2fs, ump->um_e2fs)))
+       if ((error = ext2_compute_sb_data(devvp, ump->um_e2fs->e2fs, 
ump->um_e2fs)))
                goto out;
 
        /*
@@ -1204,7 +1352,7 @@ ext2_cgupdate(struct ext2mount *mp, int waitfor)
 
        for (i = 0; i < fs->e2fs_gdbcount; i++) {
                bp = getblk(mp->um_devvp, fsbtodb(fs,
-                   cg_location(fs, i)),
+                   ext2_cg_location(fs, i)),
                    fs->e2fs_bsize, 0, 0, 0);
                if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_64BIT)) {
                        memcpy(bp->b_data, &fs->e2fs_gd[

Modified: head/sys/fs/ext2fs/ext2fs.h
==============================================================================
--- head/sys/fs/ext2fs/ext2fs.h Mon Mar  4 06:43:00 2019        (r344750)
+++ head/sys/fs/ext2fs/ext2fs.h Mon Mar  4 10:42:25 2019        (r344751)
@@ -395,6 +395,7 @@ struct ext2_gd {
 };
 
 #define        E2FS_REV0_GD_SIZE (sizeof(struct ext2_gd) / 2)
+#define        E2FS_64BIT_GD_SIZE (sizeof(struct ext2_gd))
 
 /*
  * Macro-instructions used to manage several block sizes
@@ -408,8 +409,8 @@ struct ext2_gd {
  * Macro-instructions used to manage fragments
  */
 #define        EXT2_MIN_FRAG_SIZE              1024
-#define        EXT2_MAX_FRAG_SIZE              4096
-#define        EXT2_MIN_FRAG_LOG_SIZE            10
+#define        EXT2_MIN_FRAG_LOG_SIZE          10
+#define        EXT2_MAX_FRAG_LOG_SIZE          30
 #define        EXT2_FRAG_SIZE(s)               (EXT2_SB(s)->e2fs_fsize)
 #define        EXT2_FRAGS_PER_BLOCK(s)         (EXT2_SB(s)->e2fs_fpb)
 
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to