Illumos #3522

3522 zfs module should not allow uninitialized variables
Reviewed by: Sebastien Roy <seb@delphix.com>
Reviewed by: Adam Leventhal <ahl@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Garrett D'Amore <garrett@damore.org>

References:
  https://www.illumos.org/issues/3522
  illumos/illumos-gate@d5285cae913f4e01ffa0e6693a6d8ef1fbea30ba

Ported-by: Richard Yao <ryao@gentoo.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Porting notes:

1. ZFSOnLinux had already addressed many of these issues because of
   its use of -Wall. However, the manner in which they were addressed
   differed. The illumos fixes replace the ones previously made in
   ZFSOnLinux to reduce code differences.

2. Part of the upstream patch made a small change to arc.c that might
   address zfsonlinux/zfs#1334.

3. The initialization of aclsize in zfs_log_create() differs because
   vsecp is a NULL pointer on ZFSOnLinux.

4. The changes to zfs_register_callbacks() were dropped because it
   has diverged and needs to be resynced.

diff --git a/module/zfs/arc.c b/module/zfs/arc.c
index 9098988fddb87e933986e8a4a361cedf8b0c6b82..6ad145bc45f5b1aed482547b7503d7eeffd6175d 100644 (file)
--- a/module/zfs/arc.c
+++ b/module/zfs/arc.c
@@ -3089,7 +3089,7 @@ top:
                uint64_t size = BP_GET_LSIZE(bp);
                arc_callback_t  *acb;
                vdev_t *vd = NULL;
-               uint64_t addr = -1;
+               uint64_t addr = 0;
                boolean_t devw = B_FALSE;
 
                if (hdr == NULL) {
@@ -3210,6 +3210,10 @@ top:
                                cb->l2rcb_flags = zio_flags;
                                cb->l2rcb_compress = hdr->b_l2hdr->b_compress;
 
+                               ASSERT(addr >= VDEV_LABEL_START_SIZE &&
+                                   addr + size < vd->vdev_psize -
+                                   VDEV_LABEL_END_SIZE);
+
                                /*
                                 * l2arc read.  The SCL_L2ARC lock will be
                                 * released by l2arc_read_done().
@@ -3480,8 +3484,8 @@ arc_release(arc_buf_t *buf, void *tag)
        if (l2hdr) {
                mutex_enter(&l2arc_buflist_mtx);
                hdr->b_l2hdr = NULL;
-               buf_size = hdr->b_size;
        }
+       buf_size = hdr->b_size;
 
        /*
         * Do we have more than one buf?

diff --git a/module/zfs/dmu.c b/module/zfs/dmu.c
index 4ec9cb46ad328b3dbf41ac545777f24a0d6fe91c..8ef74514abe9e489f90275039da6821efd55541c 100644 (file)
--- a/module/zfs/dmu.c
+++ b/module/zfs/dmu.c
@@ -400,8 +400,7 @@ dmu_buf_hold_array_by_dnode(dnode_t *dn, uint64_t offset, uint64_t length,
 
        if (dn->dn_objset->os_dsl_dataset)
                dp = dn->dn_objset->os_dsl_dataset->ds_dir->dd_pool;
-       if (dp && dsl_pool_sync_context(dp))
-               start = gethrtime();
+       start = gethrtime();
        zio = zio_root(dn->dn_objset->os_spa, NULL, NULL, ZIO_FLAG_CANFAIL);
        blkid = dbuf_whichblock(dn, offset);
        for (i = 0; i < nblks; i++) {

diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c
index 73807b6786bd6e102e21c4ec9598ed67c6cc4e88..f100692229509aa57ea440ac9c23fa2b1679d2bc 100644 (file)
--- a/module/zfs/dmu_objset.c
+++ b/module/zfs/dmu_objset.c
@@ -1198,7 +1198,8 @@ dmu_objset_userquota_get_ids(dnode_t *dn, boolean_t before, dmu_tx_t *tx)
        objset_t *os = dn->dn_objset;
        void *data = NULL;
        dmu_buf_impl_t *db = NULL;
-       uint64_t *user = NULL, *group = NULL;
+       uint64_t *user = NULL;
+       uint64_t *group = NULL;
        int flags = dn->dn_id_flags;
        int error;
        boolean_t have_spill = B_FALSE;

diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c
index 5c0ca4d96225b80c1e89a5773c3e40e5f4574ebe..33bcd2ab3b7b9552817720f33e896689f39889a7 100644 (file)
--- a/module/zfs/dsl_dataset.c
+++ b/module/zfs/dsl_dataset.c
@@ -431,11 +431,8 @@ dsl_dataset_hold_obj(dsl_pool_t *dp, uint64_t dsobj, void *tag,
                        ds->ds_reserved = ds->ds_quota = 0;
                }
 
-               if (err == 0) {
-                       winner = dmu_buf_set_user_ie(dbuf, ds, &ds->ds_phys,
-                           dsl_dataset_evict);
-               }
-               if (err || winner) {
+               if (err != 0 || (winner = dmu_buf_set_user_ie(dbuf, ds,
+                   &ds->ds_phys, dsl_dataset_evict)) != NULL) {
                        bplist_destroy(&ds->ds_pending_deadlist);
                        dsl_deadlist_close(&ds->ds_deadlist);
                        if (ds->ds_prev)

diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c
index 2e5034bdffdc01937772e4a149e198472b64f55e..694a7fd9c52278ce86f0b2851b9d5ef187580c17 100644 (file)
--- a/module/zfs/dsl_scan.c
+++ b/module/zfs/dsl_scan.c
@@ -1644,7 +1644,8 @@ dsl_scan_scrub_cb(dsl_pool_t *dp,
                zio_priority = ZIO_PRIORITY_SCRUB;
                needs_io = B_TRUE;
                scan_delay = zfs_scrub_delay;
-       } else if (scn->scn_phys.scn_func == POOL_SCAN_RESILVER) {
+       } else {
+               ASSERT3U(scn->scn_phys.scn_func, ==, POOL_SCAN_RESILVER);
                zio_flags |= ZIO_FLAG_RESILVER;
                zio_priority = ZIO_PRIORITY_RESILVER;
                needs_io = B_FALSE;

diff --git a/module/zfs/lzjb.c b/module/zfs/lzjb.c
index 43d0df055d8e1eb013c0af3682aa2424af043da7..7bad4f664b58acdec397ece03071f2cce06e8e13 100644 (file)
--- a/module/zfs/lzjb.c
+++ b/module/zfs/lzjb.c
@@ -50,7 +50,8 @@ lzjb_compress(void *s_start, void *d_start, size_t s_len, size_t d_len, int n)
 {
        uchar_t *src = s_start;
        uchar_t *dst = d_start;
-       uchar_t *cpy, *copymap = NULL;
+       uchar_t *cpy;
+       uchar_t *copymap = NULL;
        int copymask = 1 << (NBBY - 1);
        int mlen, offset, hash;
        uint16_t *hp;
@@ -104,7 +105,8 @@ lzjb_decompress(void *s_start, void *d_start, size_t s_len, size_t d_len, int n)
        uchar_t *src = s_start;
        uchar_t *dst = d_start;
        uchar_t *d_end = (uchar_t *)d_start + d_len;
-       uchar_t *cpy, copymap = 0;
+       uchar_t *cpy;
+       uchar_t copymap = 0;
        int copymask = 1 << (NBBY - 1);
 
        while (dst < d_end) {

diff --git a/module/zfs/sa.c b/module/zfs/sa.c
index bad6123aa0288a051105f84155e3ed8ef4ef1350..3a3dae354dad14213c5d197e35fdaedd8038855d 100644 (file)
--- a/module/zfs/sa.c
+++ b/module/zfs/sa.c
@@ -679,7 +679,8 @@ sa_build_layouts(sa_handle_t *hdl, sa_bulk_attr_t *attr_desc, int attr_count,
        int buf_space;
        sa_attr_type_t *attrs, *attrs_start;
        int i, lot_count;
-       int hdrsize, spillhdrsize = 0;
+       int hdrsize;
+       int spillhdrsize = 0;
        int used;
        dmu_object_type_t bonustype;
        sa_lot_t *lot;

diff --git a/module/zfs/spa.c b/module/zfs/spa.c
index 2bd011a0cbff01a857c9143e29e5621431bca7b3..b400896c296986dcb6893319b4d9c747d50df221 100644 (file)
--- a/module/zfs/spa.c
+++ b/module/zfs/spa.c
@@ -1443,7 +1443,7 @@ spa_load_l2cache(spa_t *spa)
        uint_t nl2cache;
        int i, j, oldnvdevs;
        uint64_t guid;
-       vdev_t *vd, **oldvdevs, **newvdevs = NULL;
+       vdev_t *vd, **oldvdevs, **newvdevs;
        spa_aux_vdev_t *sav = &spa->spa_l2cache;
 
        ASSERT(spa_config_held(spa, SCL_ALL, RW_WRITER) == SCL_ALL);
@@ -1454,6 +1454,7 @@ spa_load_l2cache(spa_t *spa)
                newvdevs = kmem_alloc(nl2cache * sizeof (void *), KM_PUSHPAGE);
        } else {
                nl2cache = 0;
+               newvdevs = NULL;
        }
 
        oldvdevs = sav->sav_vdevs;

diff --git a/module/zfs/vdev_raidz.c b/module/zfs/vdev_raidz.c
index 3e1878d377aff00eccf104450dd790c0c76f434f..1826bce673bb934b2bb35db9abf22cc6d88f94b3 100644 (file)
--- a/module/zfs/vdev_raidz.c
+++ b/module/zfs/vdev_raidz.c
@@ -1190,7 +1190,8 @@ vdev_raidz_matrix_reconstruct(raidz_map_t *rm, int n, int nmissing,
        uint64_t ccount;
        uint8_t *dst[VDEV_RAIDZ_MAXPARITY];
        uint64_t dcount[VDEV_RAIDZ_MAXPARITY];
-       uint8_t log = 0, val;
+       uint8_t log = 0;
+       uint8_t val;
        int ll;
        uint8_t *invlog[VDEV_RAIDZ_MAXPARITY];
        uint8_t *p, *pp;

diff --git a/module/zfs/zfs_fuid.c b/module/zfs/zfs_fuid.c
index debb5f86d377dc0f2d7c071c517c35822057e905..6ca61b87242f02d5524f95d80793886969408f22 100644 (file)
--- a/module/zfs/zfs_fuid.c
+++ b/module/zfs/zfs_fuid.c
@@ -565,9 +565,9 @@ zfs_fuid_create(zfs_sb_t *zsb, uint64_t id, cred_t *cr,
        uint32_t fuid_idx = FUID_INDEX(id);
        uint32_t rid;
        idmap_stat status;
-       uint64_t idx;
+       uint64_t idx = 0;
        zfs_fuid_t *zfuid = NULL;
-       zfs_fuid_info_t *fuidp;
+       zfs_fuid_info_t *fuidp = NULL;
 
        /*
         * If POSIX ID, or entry is already a FUID then
@@ -592,6 +592,9 @@ zfs_fuid_create(zfs_sb_t *zsb, uint64_t id, cred_t *cr,
                if (fuidp == NULL)
                        return (UID_NOBODY);
 
+               VERIFY3U(type, >=, ZFS_OWNER);
+               VERIFY3U(type, <=, ZFS_ACE_GROUP);
+
                switch (type) {
                case ZFS_ACE_USER:
                case ZFS_ACE_GROUP:
@@ -608,7 +611,7 @@ zfs_fuid_create(zfs_sb_t *zsb, uint64_t id, cred_t *cr,
                        idx = FUID_INDEX(fuidp->z_fuid_group);
                        break;
                };
-               domain = fuidp->z_domain_table[idx -1];
+               domain = fuidp->z_domain_table[idx - 1];
        } else {
                if (type == ZFS_OWNER || type == ZFS_ACE_USER)
                        status = kidmap_getsidbyuid(crgetzone(cr), id,

diff --git a/module/zfs/zfs_log.c b/module/zfs/zfs_log.c
index cbd6f1cb41a6bb518db1d4ea2d3965f27e66fde1..67b120436f6fc074425627198bdf879f6d7ebe33 100644 (file)
--- a/module/zfs/zfs_log.c
+++ b/module/zfs/zfs_log.c
@@ -239,10 +239,10 @@ zfs_log_create(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
        itx_t *itx;
        lr_create_t *lr;
        lr_acl_create_t *lracl;
-       xvattr_t *xvap = (xvattr_t *)vap;
        size_t aclsize = 0;
        size_t xvatsize = 0;
        size_t txsize;
+       xvattr_t *xvap = (xvattr_t *)vap;
        void *end;
        size_t lrsize;
        size_t namesize = strlen(name) + 1;
@@ -269,7 +269,6 @@ zfs_log_create(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
                txsize = sizeof (*lr) + namesize + fuidsz + xvatsize;
                lrsize = sizeof (*lr);
        } else {
-               aclsize = (vsecp) ? vsecp->vsa_aclentsz : 0;
                txsize =
                    sizeof (lr_acl_create_t) + namesize + fuidsz +
                    ZIL_ACE_LENGTH(aclsize) + xvatsize;

diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c
index 5ff1fdee29e48323c4f64ebd2555f1f4725b257a..37e3b5af02087fe8f853e5da28d7ed6f5c5b5afd 100644 (file)
--- a/module/zfs/zfs_vnops.c
+++ b/module/zfs/zfs_vnops.c
@@ -2434,7 +2434,7 @@ zfs_setattr(struct inode *ip, vattr_t *vap, int flags, cred_t *cr)
        vattr_t         oldva;
        xvattr_t        *tmpxvattr;
        uint_t          mask = vap->va_mask;
-       uint_t          saved_mask;
+       uint_t          saved_mask = 0;
        int             trim_mask = 0;
        uint64_t        new_mode;
        uint64_t        new_uid, new_gid;