From b5a60f7c12f9fa7b364cc6c94c8cec807abff4b1 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 16 Jul 2021 09:19:59 -0400 Subject: [PATCH 1/5] Initialize "autoreplace" in spa_ld_get_props() spa_prop_find() may fail to find the specified property, in which case it suppresses ENOENT from zap_lookup(). In this case, the return value is left uninitialized, so spa_autoreplace was being initialized using an uninitialized stack variable. This was found using KMSAN. It appears to be a regression from commit 9eb7b46ed0, which removed the initialization of "autoreplace" from the definition. Signed-off-by: Mark Johnston --- module/zfs/spa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/zfs/spa.c b/module/zfs/spa.c index f6dce076d136..1f49837f32c4 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -4183,7 +4183,7 @@ spa_ld_get_props(spa_t *spa) return (spa_vdev_err(rvd, VDEV_AUX_CORRUPT_DATA, EIO)); if (error == 0) { - uint64_t autoreplace; + uint64_t autoreplace = 0; spa_prop_find(spa, ZPOOL_PROP_BOOTFS, &spa->spa_bootfs); spa_prop_find(spa, ZPOOL_PROP_AUTOREPLACE, &autoreplace); From faf759069d8f0982855ff7497d8681d6c71d13f0 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 16 Jul 2021 09:27:11 -0400 Subject: [PATCH 2/5] Initialize all fields in zfs_log_xvattr() When logging TX_SETATTR, we could otherwise fail to initialize part of the corresponding ZIL record depending on which fields are present in the xvattr. Initialize the creation time and the AV scan timestamp to zero so that uninitialized bytes are not written to the ZIL. This was found using KMSAN. Signed-off-by: Mark Johnston --- module/zfs/zfs_log.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/module/zfs/zfs_log.c b/module/zfs/zfs_log.c index 30d5c4821ae5..c2f48210398c 100644 --- a/module/zfs/zfs_log.c +++ b/module/zfs/zfs_log.c @@ -126,9 +126,11 @@ zfs_log_xvattr(lr_attr_t *lrattr, xvattr_t *xvap) /* Now pack the attributes up in a single uint64_t */ attrs = (uint64_t *)bitmap; + *attrs = 0; crtime = attrs + 1; + bzero(crtime, 2 * sizeof (uint64_t)); scanstamp = (caddr_t)(crtime + 2); - *attrs = 0; + bzero(scanstamp, AV_SCANSTAMP_SZ); if (XVA_ISSET_REQ(xvap, XAT_READONLY)) *attrs |= (xoap->xoa_readonly == 0) ? 0 : XAT0_READONLY; From bdb7b086b859b0facc44e53f4e25a8eed7e4ea60 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 16 Jul 2021 09:34:54 -0400 Subject: [PATCH 3/5] Zero pad bytes when allocating a ZIL record When allocating a record, we round up the allocation size to a multiple of 8. In this case, any padding bytes should be zeroed, otherwise the contents of uninitialized memory are written to the ZIL. This was found using KMSAN. Signed-off-by: Mark Johnston --- module/zfs/zil.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 78d0711cce4e..71b3f1984cf7 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1785,18 +1785,19 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) } itx_t * -zil_itx_create(uint64_t txtype, size_t lrsize) +zil_itx_create(uint64_t txtype, size_t olrsize) { - size_t itxsize; + size_t itxsize, lrsize; itx_t *itx; - lrsize = P2ROUNDUP_TYPED(lrsize, sizeof (uint64_t), size_t); + lrsize = P2ROUNDUP_TYPED(olrsize, sizeof (uint64_t), size_t); itxsize = offsetof(itx_t, itx_lr) + lrsize; itx = zio_data_buf_alloc(itxsize); itx->itx_lr.lrc_txtype = txtype; itx->itx_lr.lrc_reclen = lrsize; itx->itx_lr.lrc_seq = 0; /* defensive */ + bzero((char *)&itx->itx_lr + olrsize, lrsize - olrsize); itx->itx_sync = B_TRUE; /* default is synchronous */ itx->itx_callback = NULL; itx->itx_callback_data = NULL; From e4237932e74e6cdf2823036cadce033673367b30 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 16 Jul 2021 10:05:28 -0400 Subject: [PATCH 4/5] Zero pad bytes following TX_WRITE log data When logging a TX_WRITE record in the case where file data has to be copied from the DMU, we pad the log record size to a multiple of 8 bytes. In this case, any padding bytes should be zeroed, otherwise the contents of uninitialized memory are written to the ZIL. This was found using KMSAN. Signed-off-by: Mark Johnston --- module/zfs/zil.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/module/zfs/zil.c b/module/zfs/zil.c index 71b3f1984cf7..1cb2b8fe8a3e 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -1619,7 +1619,7 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) lr_t *lrcb, *lrc; lr_write_t *lrwb, *lrw; char *lr_buf; - uint64_t dlen, dnow, lwb_sp, reclen, txg, max_log_data; + uint64_t dlen, dnow, dpad, lwb_sp, reclen, txg, max_log_data; ASSERT(MUTEX_HELD(&zilog->zl_issuer_lock)); ASSERT3P(lwb, !=, NULL); @@ -1653,8 +1653,9 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) if (lrc->lrc_txtype == TX_WRITE && itx->itx_wr_state == WR_NEED_COPY) { dlen = P2ROUNDUP_TYPED( lrw->lr_length, sizeof (uint64_t), uint64_t); + dpad = dlen - lrw->lr_length; } else { - dlen = 0; + dlen = dpad = 0; } reclen = lrc->lrc_reclen; zilog->zl_cur_used += (reclen + dlen); @@ -1748,6 +1749,9 @@ zil_lwb_commit(zilog_t *zilog, itx_t *itx, lwb_t *lwb) error = zilog->zl_get_data(itx->itx_private, itx->itx_gen, lrwb, dbuf, lwb, lwb->lwb_write_zio); + if (dbuf != NULL && error == 0 && dnow == dlen) + /* Zero any padding bytes in the last block. */ + bzero((char *)dbuf + lrwb->lr_length, dpad); if (error == EIO) { txg_wait_synced(zilog->zl_dmu_pool, txg); From f75b9094df9e924f617003a9ef1398a2e5d59f8b Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Fri, 16 Jul 2021 10:12:47 -0400 Subject: [PATCH 5/5] Initialize dn_next_type[] in the dnode constructor It seems nothing ensures that this array is zeroed when a dnode is freshly allocated, so in principle it retains the values from the previous allocation. In practice it seems to be the case that the fields should end up zeroed, but we can zero the field anyway for consistency. This was found using KMSAN. Signed-off-by: Mark Johnston --- module/zfs/dnode.c | 1 + 1 file changed, 1 insertion(+) diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c index b1813a8951d5..7f741542ce02 100644 --- a/module/zfs/dnode.c +++ b/module/zfs/dnode.c @@ -129,6 +129,7 @@ dnode_cons(void *arg, void *unused, int kmflag) zfs_refcount_create(&dn->dn_tx_holds); list_link_init(&dn->dn_link); + bzero(&dn->dn_next_type[0], sizeof (dn->dn_next_type)); bzero(&dn->dn_next_nblkptr[0], sizeof (dn->dn_next_nblkptr)); bzero(&dn->dn_next_nlevels[0], sizeof (dn->dn_next_nlevels)); bzero(&dn->dn_next_indblkshift[0], sizeof (dn->dn_next_indblkshift));