[PATCHv2 fs/bfs 0/2] bfs: fix null-ptr-deref and possible warning in bfs_move_block() func

[PATCHv2 fs/bfs 0/2] bfs: fix null-ptr-deref and possible warning in bfs_move_block() func

[kovalev]

https://syzkaller.appspot.com/bug?extid=d98fd19acd08b36ff422

[PATCHv2 fs/bfs 1/2] bfs: prevent null pointer dereference in bfs_move_block()
---
v2: corrected the commit message and explicitly initialized
the return variable with zero (Markus Elfring)
---
[PATCHv2 fs/bfs 2/2] bfs: add buffer_uptodate check before mark_buffer_dirty
---
v2: renamed the return variable  err -> ret (Markus Elfring)
---

[PATCHv2 fs/bfs 1/2] bfs: prevent null pointer dereference in bfs_move_block()

[kovalev]

From: Vasiliy Kovalev <kovalev@altlinux.org>

Detect a failed sb_getblk() call (before copying data)
so that null pointer dereferences should not happen any more.

Found by Syzkaller:

KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
CPU: 1 PID: 1069 Comm: mark_buffer_dir Tainted: G W 6.10.0-un-def-alt0.rc7
RIP: 0010:bfs_get_block+0x3ab/0xe80 [bfs]
Call Trace:
<TASK>
? show_regs+0x8d/0xa0
? die_addr+0x50/0xd0
? exc_general_protection+0x148/0x220
? asm_exc_general_protection+0x22/0x30
? bfs_get_block+0x3ab/0xe80 [bfs]
? bfs_get_block+0x370/0xe80 [bfs]
? __pfx_bfs_get_block+0x10/0x10 [bfs]
__block_write_begin_int+0x4ae/0x16a0
? __pfx_bfs_get_block+0x10/0x10 [bfs]
? __pfx___block_write_begin_int+0x10/0x10
block_write_begin+0xb5/0x410
? __pfx_bfs_get_block+0x10/0x10 [bfs]
bfs_write_begin+0x32/0xe0 [bfs]
generic_perform_write+0x265/0x610
? __pfx_generic_perform_write+0x10/0x10
? generic_write_checks+0x323/0x4a0
? __pfx_generic_file_write_iter+0x10/0x10
__generic_file_write_iter+0x16a/0x1b0
generic_file_write_iter+0xf0/0x360
? __pfx_generic_file_write_iter+0x10/0x10
vfs_write+0x670/0x1120
? __pfx_vfs_write+0x10/0x10
ksys_write+0x127/0x260
? __pfx_ksys_write+0x10/0x10
do_syscall_64+0x9f/0x190
? __ct_user_enter+0x74/0xc0
? syscall_exit_to_user_mode+0xbb/0x1d0
? do_syscall_64+0xab/0x190
? ct_kernel_exit.isra.0+0xbb/0xe0
? __ct_user_enter+0x74/0xc0
? syscall_exit_to_user_mode+0xbb/0x1d0
? do_syscall_64+0xab/0x190
? ct_kernel_exit.isra.0+0xbb/0xe0
? clear_bhb_loop+0x45/0xa0
? clear_bhb_loop+0x45/0xa0
? clear_bhb_loop+0x45/0xa0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7f2bc708ed29

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+d98fd19acd08b36ff422@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
---
 fs/bfs/file.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/bfs/file.c b/fs/bfs/file.c
index 0dceefc54b48a..e99dc8ace2027 100644
--- a/fs/bfs/file.c
+++ b/fs/bfs/file.c
@@ -34,16 +34,22 @@ static int bfs_move_block(unsigned long from, unsigned long to,
 					struct super_block *sb)
 {
 	struct buffer_head *bh, *new;
+	int ret = 0;
 
 	bh = sb_bread(sb, from);
 	if (!bh)
 		return -EIO;
 	new = sb_getblk(sb, to);
+	if (unlikely(!new)) {
+		ret = -EIO;
+		goto out_err_new;
+	}
 	memcpy(new->b_data, bh->b_data, bh->b_size);
 	mark_buffer_dirty(new);
-	bforget(bh);
 	brelse(new);
-	return 0;
+out_err_new:
+	bforget(bh);
+	return ret;
 }
 
 static int bfs_move_blocks(struct super_block *sb, unsigned long start,
-- 
2.33.8

[PATCHv2 fs/bfs 2/2] bfs: add buffer_uptodate check before mark_buffer_dirty call

[kovalev]

From: Vasiliy Kovalev <kovalev@altlinux.org>

Add a check in bfs_move_block to ensure the new buffer is up-to-date
(buffer_uptodate) before calling mark_buffer_dirty.

Found by Syzkaller:

WARNING: CPU: 1 PID: 1046 at fs/buffer.c:1183 mark_buffer_dirty+0x394/0x3f0
CPU: 1 PID: 1046 Comm: mark_buffer_dir Not tainted 6.10.0-un-def-alt0.rc7.kasan
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-alt1 04/01/2014
RIP: 0010:mark_buffer_dirty+0x394/0x3f0
Call Trace:
<TASK>
? show_regs+0x8d/0xa0
? __warn+0xe6/0x380
? mark_buffer_dirty+0x394/0x3f0
? report_bug+0x348/0x480
? handle_bug+0x60/0xc0
? exc_invalid_op+0x13/0x50
? asm_exc_invalid_op+0x16/0x20
? mark_buffer_dirty+0x394/0x3f0
? mark_buffer_dirty+0x394/0x3f0
bfs_get_block+0x3ec/0xe80 [bfs]
? __pfx_bfs_get_block+0x10/0x10 [bfs]
__block_write_begin_int+0x4ae/0x16a0
? __pfx_bfs_get_block+0x10/0x10 [bfs]
? __pfx___block_write_begin_int+0x10/0x10
block_write_begin+0xb5/0x410
? __pfx_bfs_get_block+0x10/0x10 [bfs]
bfs_write_begin+0x32/0xe0 [bfs]
generic_perform_write+0x265/0x610
? __pfx_generic_perform_write+0x10/0x10
? generic_write_checks+0x323/0x4a0
? __pfx_generic_file_write_iter+0x10/0x10
__generic_file_write_iter+0x16a/0x1b0
generic_file_write_iter+0xf0/0x360
? __pfx_generic_file_write_iter+0x10/0x10
vfs_write+0x670/0x1120
? __pfx_vfs_write+0x10/0x10
ksys_write+0x127/0x260
? __pfx_ksys_write+0x10/0x10
do_syscall_64+0x9f/0x190
? do_syscall_64+0xab/0x190
? syscall_exit_to_user_mode+0xbb/0x1d0
? do_syscall_64+0xab/0x190
? lock_release+0x241/0x730
? __ct_user_enter+0xb3/0xc0
? __pfx_lock_release+0x10/0x10
? get_vtime_delta+0x116/0x270
? ct_kernel_exit.isra.0+0xbb/0xe0
? __ct_user_enter+0x74/0xc0
? syscall_exit_to_user_mode+0xbb/0x1d0
? do_syscall_64+0xab/0x190
? do_syscall_64+0xab/0x190
? ct_kernel_exit.isra.0+0xbb/0xe0
? clear_bhb_loop+0x45/0xa0
? clear_bhb_loop+0x45/0xa0
? clear_bhb_loop+0x45/0xa0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7f5bb79a4d2

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+d98fd19acd08b36ff422@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d98fd19acd08b36ff422
Cc: stable@vger.kernel.org
Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
---
 fs/bfs/file.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/bfs/file.c b/fs/bfs/file.c
index e99dc8ace2027..9599b41cbe91b 100644
--- a/fs/bfs/file.c
+++ b/fs/bfs/file.c
@@ -44,8 +44,13 @@ static int bfs_move_block(unsigned long from, unsigned long to,
 		ret = -EIO;
 		goto out_err_new;
 	}
+	if (!buffer_uptodate(new)) {
+		ret = -EIO;
+		goto out_err;
+	}
 	memcpy(new->b_data, bh->b_data, bh->b_size);
 	mark_buffer_dirty(new);
+out_err:
 	brelse(new);
 out_err_new:
 	bforget(bh);
-- 
2.33.8

Re: [PATCH v2 fs/bfs 0/2] bfs: fix null-ptr-deref and possible warning in bfs_move_block() func

[Markus Elfring]

…
> [PATCHv2 fs/bfs 1/2] bfs: prevent null pointer dereference in bfs_move_block()
…

I find it usually helpful to separate the version identifier from the previous key word.

How do you think about to improve the outline another bit (also for the cover letter)?

Regards,
Markus

Re: [PATCH v2 fs/bfs 0/2] bfs: fix null-ptr-deref and possible warning in bfs_move_block() func

[kovalev]

11.07.2024 10:47, Markus Elfring wrote:
> …
>> [PATCHv2 fs/bfs 1/2] bfs: prevent null pointer dereference in bfs_move_block()
> …
> 
> I find it usually helpful to separate the version identifier from the previous key word.
> 
> How do you think about to improve the outline another bit (also for the cover letter)?

I will take your recommendation into account when submitting the next 
versions, if there are any comments on the patches themselves.

> Regards,
> Markus

-- 
Thanks,
Vasiliy Kovalev