Fixing null pointer deference

Originally I was checking dr->dr_dbuf->db_level == 0 in dbuf_dirty_is_direct_write(). Howver, this can lead to a NULL ponter dereference if the dr_dbuf is no longer set. I updated dbuf_dirty_is_direct_write() to now also take a dmu_buf_impl_t to check if db->db_level == 0. This failure was caught on the Fedora 37 CI running in test enospc_rm. Below is the stack trace. [ 9851.511608] BUG: kernel NULL pointer dereference, address: 0000000000000068 [ 9851.515922] #PF: supervisor read access in kernel mode [ 9851.519462] #PF: error_code(0x0000) - not-present page [ 9851.522992] PGD 0 P4D 0 [ 9851.525684] Oops: 0000 [openzfs#1] PREEMPT SMP PTI [ 9851.528878] CPU: 0 PID: 1272993 Comm: fio Tainted: P OE 6.5.12-100.fc37.x86_64 openzfs#1 [ 9851.535266] Hardware name: Amazon EC2 m5d.large/, BIOS 1.0 10/16/2017 [ 9851.539226] RIP: 0010:dbuf_dirty_is_direct_write+0xb/0x40 [zfs] [ 9851.543379] Code: 10 74 02 31 c0 5b c3 cc cc cc cc 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 31 c0 48 85 ff 74 31 48 8b 57 20 <80> 7a 68 00 75 27 8b 87 64 01 00 00 85 c0 75 1b 83 bf 58 01 00 00 [ 9851.554719] RSP: 0018:ffff9b5b8305f8e8 EFLAGS: 00010286 [ 9851.558276] RAX: 0000000000000000 RBX: ffff9b5b8569b0b8 RCX: 0000000000000000 [ 9851.562481] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8f2e97de9e00 [ 9851.566672] RBP: 0000000000020000 R08: 0000000000000000 R09: ffff8f2f70e94000 [ 9851.570851] R10: 0000000000000001 R11: 0000000000000110 R12: ffff8f2f774ae4c0 [ 9851.575032] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 9851.579209] FS: 00007f57c5542240(0000) GS:ffff8f2faa800000(0000) knlGS:0000000000000000 [ 9851.585357] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 9851.589064] CR2: 0000000000000068 CR3: 00000001f9a38001 CR4: 00000000007706f0 [ 9851.593256] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 9851.597440] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 9851.601618] PKRU: 55555554 [ 9851.604341] Call Trace: [ 9851.606981] <TASK> [ 9851.609515] ? __die+0x23/0x70 [ 9851.612388] ? page_fault_oops+0x171/0x4e0 [ 9851.615571] ? exc_page_fault+0x77/0x170 [ 9851.618704] ? asm_exc_page_fault+0x26/0x30 [ 9851.621900] ? dbuf_dirty_is_direct_write+0xb/0x40 [zfs] [ 9851.625828] zfs_get_data+0x407/0x820 [zfs] [ 9851.629400] zil_lwb_commit+0x18d/0x3f0 [zfs] [ 9851.633026] zil_lwb_write_issue+0x92/0xbb0 [zfs] [ 9851.636758] zil_commit_waiter_timeout+0x1f3/0x580 [zfs] [ 9851.640696] zil_commit_waiter+0x1ff/0x3a0 [zfs] [ 9851.644402] zil_commit_impl+0x71/0xd0 [zfs] [ 9851.647998] zfs_write+0xb51/0xdc0 [zfs] [ 9851.651467] zpl_iter_write_buffered+0xc9/0x140 [zfs] [ 9851.655337] zpl_iter_write+0xc0/0x110 [zfs] [ 9851.658920] vfs_write+0x23e/0x420 [ 9851.661871] __x64_sys_pwrite64+0x98/0xd0 [ 9851.665013] do_syscall_64+0x5f/0x90 [ 9851.668027] ? ksys_fadvise64_64+0x57/0xa0 [ 9851.671212] ? syscall_exit_to_user_mode+0x2b/0x40 [ 9851.674594] ? do_syscall_64+0x6b/0x90 [ 9851.677655] ? syscall_exit_to_user_mode+0x2b/0x40 [ 9851.681051] ? do_syscall_64+0x6b/0x90 [ 9851.684128] ? exc_page_fault+0x77/0x170 [ 9851.687256] entry_SYSCALL_64_after_hwframe+0x6e/0xd8 [ 9851.690759] RIP: 0033:0x7f57c563c377 Signed-off-by: Brian Atkinson <[email protected]>
bwatkinson · Sep 10, 2024 · 65e9b0c · 65e9b0c
1 parent 7fab6b7
commit 65e9b0c
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 7 deletions.
diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h
@@ -491,12 +491,12 @@ dbuf_get_dirty_direct(dmu_buf_impl_t *db)
 }
 
 static inline boolean_t
-dbuf_dirty_is_direct_write(dbuf_dirty_record_t *dr)
+dbuf_dirty_is_direct_write(dmu_buf_impl_t *db, dbuf_dirty_record_t *dr)
 {
 	boolean_t ret = B_FALSE;
+	ASSERT(MUTEX_HELD(&db->db_mtx));
 
-	if (dr != NULL && dr->dr_dbuf->db_level == 0 &&
-	    !dr->dt.dl.dr_brtwrite &&
+	if (dr != NULL && db->db_level == 0 && !dr->dt.dl.dr_brtwrite &&
 	    dr->dt.dl.dr_override_state == DR_OVERRIDDEN &&
 	    dr->dt.dl.dr_data == NULL) {
 		ret = B_TRUE;

diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c
@@ -1294,7 +1294,7 @@ dbuf_set_data(dmu_buf_impl_t *db, arc_buf_t *buf)
 	 * If there is a Direct I/O, set its data too. Then its state
 	 * will be the same as if we did a ZIL dmu_sync().
 	 */
-	if (dbuf_dirty_is_direct_write(dr_dio)) {
+	if (dbuf_dirty_is_direct_write(db, dr_dio)) {
 		dr_dio->dt.dl.dr_data = db->db_buf;
 	}
 
@@ -2221,7 +2221,8 @@ dbuf_redirty(dbuf_dirty_record_t *dr)
 		 * dr->dt.dl.dr_data and it will not be NULL here.
 		 */
 		if (dr->dt.dl.dr_data == NULL) {
-			ASSERT3B(dbuf_dirty_is_direct_write(dr), ==, B_TRUE);
+			ASSERT3B(dbuf_dirty_is_direct_write(db, dr), ==,
+			    B_TRUE);
 			dr->dt.dl.dr_data = db->db_buf;
 		}
 	}

diff --git a/module/zfs/dmu_direct.c b/module/zfs/dmu_direct.c
@@ -193,7 +193,7 @@ dmu_write_direct(zio_t *pio, dmu_buf_impl_t *db, abd_t *data, dmu_tx_t *tx)
 	 */
 	mutex_enter(&db->db_mtx);
 	dr_head = dbuf_find_dirty_eq(db, dmu_tx_get_txg(tx));
-	if (dbuf_dirty_is_direct_write(dr_head)) {
+	if (dbuf_dirty_is_direct_write(db, dr_head)) {
 		dmu_buf_undirty(db, tx);
 	}
 	mutex_exit(&db->db_mtx);

diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c
@@ -1119,6 +1119,7 @@ zfs_get_data(void *arg, uint64_t gen, lr_write_t *lr, char *buf,
 	zgd->zgd_grabbed_rangelock = !(rangelock_held);
 	dbuf_dirty_record_t *dr =
 	    dbuf_find_dirty_eq(db, lr->lr_common.lrc_txg);
+	boolean_t direct_write = dbuf_dirty_is_direct_write(db, dr);
 	mutex_exit(&db->db_mtx);
 
 	/*
@@ -1190,7 +1191,7 @@ zfs_get_data(void *arg, uint64_t gen, lr_write_t *lr, char *buf,
 		 * All Direct I/O writes will have already completed and the
 		 * block pointer can be immediately stored in the log record.
 		 */
-		if (dbuf_dirty_is_direct_write(dr)) {
+		if (direct_write) {
 			lr->lr_blkptr = dr->dt.dl.dr_overridden_by;
 			zfs_get_done(zgd, 0);
 			return (0);