From ce0e1cc402505493a890e7fc0819e582ae686b3b Mon Sep 17 00:00:00 2001 From: Pawel Jakub Dawidek Date: Fri, 24 Mar 2023 18:18:35 +0100 Subject: [PATCH] Fix cloning into already dirty dbufs. Undirty the dbuf and destroy its buffer when cloning into it. Coverity ID: CID-1535375 Reported-by: Richard Yao Reported-by: Benjamin Coddington Reviewed-by: Richard Yao Reviewed-by: Brian Behlendorf Reviewed-by: Alexander Motin Signed-off-by: Pawel Jakub Dawidek Closes #14655 --- include/sys/dbuf.h | 1 + module/zfs/dbuf.c | 3 +-- module/zfs/dmu.c | 35 ++++++++++++++++++++++++----------- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h index a06316362..fb26a83b1 100644 --- a/include/sys/dbuf.h +++ b/include/sys/dbuf.h @@ -382,6 +382,7 @@ void dbuf_assign_arcbuf(dmu_buf_impl_t *db, arc_buf_t *buf, dmu_tx_t *tx); dbuf_dirty_record_t *dbuf_dirty(dmu_buf_impl_t *db, dmu_tx_t *tx); dbuf_dirty_record_t *dbuf_dirty_lightweight(dnode_t *dn, uint64_t blkid, dmu_tx_t *tx); +boolean_t dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx); arc_buf_t *dbuf_loan_arcbuf(dmu_buf_impl_t *db); void dmu_buf_write_embedded(dmu_buf_t *dbuf, void *data, bp_embedded_type_t etype, enum zio_compress comp, diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 80ea1bfe4..617c85029 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -175,7 +175,6 @@ struct { continue; \ } -static boolean_t dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx); static void dbuf_write(dbuf_dirty_record_t *dr, arc_buf_t *data, dmu_tx_t *tx); static void dbuf_sync_leaf_verify_bonus_dnode(dbuf_dirty_record_t *dr); static int dbuf_read_verify_dnode_crypt(dmu_buf_impl_t *db, uint32_t flags); @@ -2518,7 +2517,7 @@ dbuf_undirty_bonus(dbuf_dirty_record_t *dr) * Undirty a buffer in the transaction group referenced by the given * transaction. Return whether this evicted the dbuf. */ -static boolean_t +boolean_t dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx) { uint64_t txg = tx->tx_txg; diff --git a/module/zfs/dmu.c b/module/zfs/dmu.c index e6bade11c..23b666752 100644 --- a/module/zfs/dmu.c +++ b/module/zfs/dmu.c @@ -2190,7 +2190,8 @@ dmu_read_l0_bps(objset_t *os, uint64_t object, uint64_t offset, uint64_t length, for (int i = 0; i < numbufs; i++) { dbuf = dbp[i]; db = (dmu_buf_impl_t *)dbuf; - bp = db->db_blkptr; + + mutex_enter(&db->db_mtx); /* * If the block is not on the disk yet, it has no BP assigned. @@ -2212,10 +2213,16 @@ dmu_read_l0_bps(objset_t *os, uint64_t object, uint64_t offset, uint64_t length, * The block was modified in the same * transaction group. */ + mutex_exit(&db->db_mtx); error = SET_ERROR(EAGAIN); goto out; } + } else { + bp = db->db_blkptr; } + + mutex_exit(&db->db_mtx); + if (bp == NULL) { /* * The block was created in this transaction group, @@ -2273,19 +2280,23 @@ dmu_brt_clone(objset_t *os, uint64_t object, uint64_t offset, uint64_t length, ASSERT(db->db_blkid != DMU_BONUS_BLKID); ASSERT(BP_IS_HOLE(bp) || dbuf->db_size == BP_GET_LSIZE(bp)); - if (db->db_state == DB_UNCACHED) { - /* - * XXX-PJD: If the dbuf is already cached, calling - * dmu_buf_will_not_fill() will panic on assertion - * (db->db_buf == NULL) in dbuf_clear_data(), - * which is called from dbuf_noread() in DB_NOFILL - * case. I'm not 100% sure this is the right thing - * to do, but it seems to work. - */ - dmu_buf_will_not_fill(dbuf, tx); + mutex_enter(&db->db_mtx); + + VERIFY(!dbuf_undirty(db, tx)); + ASSERT(list_head(&db->db_dirty_records) == NULL); + if (db->db_buf != NULL) { + arc_buf_destroy(db->db_buf, db); + db->db_buf = NULL; } + mutex_exit(&db->db_mtx); + + dmu_buf_will_not_fill(dbuf, tx); + + mutex_enter(&db->db_mtx); + dr = list_head(&db->db_dirty_records); + VERIFY(dr != NULL); ASSERT3U(dr->dr_txg, ==, tx->tx_txg); dl = &dr->dt.dl; dl->dr_overridden_by = *bp; @@ -2301,6 +2312,8 @@ dmu_brt_clone(objset_t *os, uint64_t object, uint64_t offset, uint64_t length, BP_PHYSICAL_BIRTH(bp); } + mutex_exit(&db->db_mtx); + /* * When data in embedded into BP there is no need to create * BRT entry as there is no data block. Just copy the BP as -- 2.39.5