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 <allan@klarasystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Brian Atkinson <batkinson@lanl.gov>
Closes #16337
This commit is contained in:
Brian Atkinson 2024-08-01 21:22:43 -04:00 committed by GitHub
parent c092bddfe7
commit c8184d714b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 15 additions and 1 deletions

View File

@ -2705,6 +2705,9 @@ void
dmu_buf_will_clone(dmu_buf_t *db_fake, dmu_tx_t *tx) dmu_buf_will_clone(dmu_buf_t *db_fake, dmu_tx_t *tx)
{ {
dmu_buf_impl_t *db = (dmu_buf_impl_t *)db_fake; 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 * 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)); VERIFY(!dbuf_undirty(db, tx));
ASSERT0P(dbuf_find_dirty_eq(db, tx->tx_txg)); ASSERT0P(dbuf_find_dirty_eq(db, tx->tx_txg));
if (db->db_buf != NULL) { 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; db->db_buf = NULL;
dbuf_clear_data(db); dbuf_clear_data(db);
} }
ASSERT3P(db->db_buf, ==, NULL);
ASSERT3P(db->db.db_data, ==, NULL);
db->db_state = DB_NOFILL; db->db_state = DB_NOFILL;
DTRACE_SET_STATE(db, "allocating NOFILL buffer for clone"); DTRACE_SET_STATE(db, "allocating NOFILL buffer for clone");