From d954ca19ba8b0c505e88a74a9681c4c81e7cfc57 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Tue, 13 Sep 2022 20:58:29 -0400 Subject: [PATCH] Fix theoretical "use-after-free" in dbuf_prefetch_indirect_done() Coverity complains about a "use-after-free" bug in `dbuf_prefetch_indirect_done()` because we use a pointer value after freeing its buffer. The pointer is used for refcounting in ARC (as the reference holder). There is a theoretical situation where the pointer would be reused in a way that causes the refcounting to collide, so we change the order in which we call arc_buf_destroy() and dbuf_prefetch_fini() to match the rest of the function. This prevents the theoretical situation from being a possibility. Also, we have a few return statements with a value, despite this being a void function. We clean those up while we are making changes here. Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Reviewed-by: Neal Gompa Signed-off-by: Richard Yao Closes #13869 --- module/zfs/dbuf.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index b2d1b9568..80cab8177 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -3254,7 +3254,8 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb, if (abuf == NULL) { ASSERT(zio == NULL || zio->io_error != 0); - return (dbuf_prefetch_fini(dpa, B_TRUE)); + dbuf_prefetch_fini(dpa, B_TRUE); + return; } ASSERT(zio == NULL || zio->io_error == 0); @@ -3287,7 +3288,8 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb, dpa->dpa_curlevel, curblkid, FTAG); if (db == NULL) { arc_buf_destroy(abuf, private); - return (dbuf_prefetch_fini(dpa, B_TRUE)); + dbuf_prefetch_fini(dpa, B_TRUE); + return; } (void) dbuf_read(db, NULL, DB_RF_MUST_SUCCEED | DB_RF_NOPREFETCH | DB_RF_HAVESTRUCT); @@ -3305,7 +3307,9 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb, dpa->dpa_dnode->dn_objset->os_dsl_dataset, SPA_FEATURE_REDACTED_DATASETS)); if (BP_IS_HOLE(bp) || BP_IS_REDACTED(bp)) { + arc_buf_destroy(abuf, private); dbuf_prefetch_fini(dpa, B_TRUE); + return; } else if (dpa->dpa_curlevel == dpa->dpa_zb.zb_level) { ASSERT3U(nextblkid, ==, dpa->dpa_zb.zb_blkid); dbuf_issue_final_prefetch(dpa, bp);