[PATCHv2 fs/bfs 0/2] bfs: fix null-ptr-deref and possible warning in bfs_move_block() func
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) ---
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,
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);
…
[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
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
linux-stable-mirror@lists.linaro.org
-
kovalev＠altlinux.org -
Markus Elfring