From 6ecdff93ddff8a2161ffab287a78c81db773e3ac Mon Sep 17 00:00:00 2001 From: Rob N Date: Wed, 29 Nov 2023 04:53:04 +1100 Subject: [PATCH] dmu_buf_will_clone: fix race in transition back to NOFILL Previously, dmu_buf_will_clone() would roll back any dirty record, but would not clean out the modified data nor reset the state before releasing the lock. That leaves the last-written data in db_data, but the dbuf in the wrong state. This is eventually corrected when the dbuf state is made NOFILL, and dbuf_noread() called (which clears out the old data), but at this point its too late, because the lock was already dropped with that invalid state. Any caller acquiring the lock before the call into dmu_buf_will_not_fill() can find what appears to be a clean, readable buffer, and would take the wrong state from it: it should be getting the data from the cloned block, not from earlier (unwritten) dirty data. Even after the state was switched to NOFILL, the old data was still not cleaned out until dbuf_noread(), which is another gap for a caller to take the lock and read the wrong data. This commit fixes all this by properly cleaning up the previous state and then setting the new state before dropping the lock. The DBUF_VERIFY() calls confirm that the dbuf is in a valid state when the lock is down. Sponsored-by: Klara, Inc. Sponsored-By: OpenDrives Inc. Reviewed-by: Brian Behlendorf Reviewed-by: Pawel Jakub Dawidek Signed-off-by: Rob Norris Closes #15566 Closes #15526 --- module/zfs/dbuf.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index f2831a0e8abf..5a7fe42b602a 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -2700,15 +2700,23 @@ dmu_buf_will_clone(dmu_buf_t *db_fake, dmu_tx_t *tx) * writes and clones into this block. */ mutex_enter(&db->db_mtx); + DBUF_VERIFY(db); VERIFY(!dbuf_undirty(db, tx)); ASSERT3P(dbuf_find_dirty_eq(db, tx->tx_txg), ==, NULL); if (db->db_buf != NULL) { arc_buf_destroy(db->db_buf, db); db->db_buf = NULL; + dbuf_clear_data(db); } + + db->db_state = DB_NOFILL; + DTRACE_SET_STATE(db, "allocating NOFILL buffer for clone"); + + DBUF_VERIFY(db); mutex_exit(&db->db_mtx); - dmu_buf_will_not_fill(db_fake, tx); + dbuf_noread(db); + (void) dbuf_dirty(db, tx); } void