From c8184d714b1f5748f90d7a17c391902774b092d0 Mon Sep 17 00:00:00 2001 From: Brian Atkinson Date: Thu, 1 Aug 2024 21:22:43 -0400 Subject: [PATCH] Block cloning conditionally destroy ARC buffer dmu_buf_will_clone() calls arc_buf_destroy() if there is an associated ARC buffer with the dbuf. However, this can only be done conditionally. If the previous dirty record's dr_data is pointed at db_dbf then destroying it can lead to NULL pointer deference when syncing out the previous dirty record. This updates dmu_buf_fill_clone() to only call arc_buf_destroy() if the previous dirty records dr_data is not pointing to db_buf. The block clone wil still set the dbuf's db_buf and db_data to NULL, but this will not cause any issues as any previous dirty record dr_data will still be pointing at the ARC buffer. Reviewed-by: Allan Jude Reviewed-by: Brian Behlendorf Reviewed-by: Alexander Motin Signed-off-by: Brian Atkinson Closes #16337 --- module/zfs/dbuf.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 3173e06974..099883ba26 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -2705,6 +2705,9 @@ void dmu_buf_will_clone(dmu_buf_t *db_fake, dmu_tx_t *tx) { dmu_buf_impl_t *db = (dmu_buf_impl_t *)db_fake; + ASSERT0(db->db_level); + ASSERT(db->db_blkid != DMU_BONUS_BLKID); + ASSERT(db->db.db_object != DMU_META_DNODE_OBJECT); /* * Block cloning: We are going to clone into this block, so undirty @@ -2716,11 +2719,22 @@ dmu_buf_will_clone(dmu_buf_t *db_fake, dmu_tx_t *tx) VERIFY(!dbuf_undirty(db, tx)); ASSERT0P(dbuf_find_dirty_eq(db, tx->tx_txg)); if (db->db_buf != NULL) { - arc_buf_destroy(db->db_buf, db); + /* + * If there is an associated ARC buffer with this dbuf we can + * only destroy it if the previous dirty record does not + * reference it. + */ + dbuf_dirty_record_t *dr = list_head(&db->db_dirty_records); + if (dr == NULL || dr->dt.dl.dr_data != db->db_buf) + arc_buf_destroy(db->db_buf, db); + db->db_buf = NULL; dbuf_clear_data(db); } + ASSERT3P(db->db_buf, ==, NULL); + ASSERT3P(db->db.db_data, ==, NULL); + db->db_state = DB_NOFILL; DTRACE_SET_STATE(db, "allocating NOFILL buffer for clone");