[PATCH] jfs: fix a oob in dtSplitRoot

Lizhi Xu posted 1 patch 1 year, 2 months ago
fs/jfs/jfs_dtree.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] jfs: fix a oob in dtSplitRoot
Posted by Lizhi Xu 1 year, 2 months ago
syzbot report a array-index-out-of-bounds in dtSplitRoot. [1]

The second index value of the parent inode of the symbolic link is 4294967168.
When it is assigned to the stbl of type s8, an overflow value of -128 occurs,
which triggers oob.

To avoid this issue, add a check for the index of the slot before using it.

[1]
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_dtree.c:1997:37
index -128 is out of range for type 'struct dtslot[128]'
CPU: 1 UID: 0 PID: 5842 Comm: syz-executor268 Not tainted 6.12.0-syzkaller-09073-g9f16d5e6f220 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 ubsan_epilogue lib/ubsan.c:231 [inline]
 __ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
 dtSplitRoot+0xc9c/0x1930 fs/jfs/jfs_dtree.c:1997
 dtSplitUp fs/jfs/jfs_dtree.c:992 [inline]
 dtInsert+0x12cd/0x6c10 fs/jfs/jfs_dtree.c:870
 jfs_symlink+0x827/0x10f0 fs/jfs/namei.c:1020
 vfs_symlink+0x137/0x2e0 fs/namei.c:4669
 do_symlinkat+0x222/0x3a0 fs/namei.c:4695
 __do_sys_symlink fs/namei.c:4716 [inline]
 __se_sys_symlink fs/namei.c:4714 [inline]
 __x64_sys_symlink+0x7a/0x90 fs/namei.c:4714
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Reported-and-tested-by: syzbot+99491d74a9931659cf48@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=99491d74a9931659cf48
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/jfs/jfs_dtree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 8f85177f284b..71463ad751c2 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -1994,6 +1994,9 @@ static int dtSplitRoot(tid_t tid,
 
 		stbl = DT_GETSTBL(rp);
 		for (n = 0; n < rp->header.nextindex; n++) {
+			if (stbl[n] >= ARRAY_SIZE(rp->slot))
+				continue;
+
 			ldtentry = (struct ldtentry *) & rp->slot[stbl[n]];
 			modify_index(tid, ip, le32_to_cpu(ldtentry->index),
 				     rbn, n, &mp, &lblock);
-- 
2.43.0
Re: [Jfs-discussion] [PATCH] jfs: fix a oob in dtSplitRoot
Posted by Dave Kleikamp 11 months, 3 weeks ago
I'm catching up on some long-ignored emails and have some concerns about 
this patch.

On 11/29/24 5:16AM, Lizhi Xu via Jfs-discussion wrote:
> syzbot report a array-index-out-of-bounds in dtSplitRoot. [1]
> 
> The second index value of the parent inode of the symbolic link is 4294967168.
> When it is assigned to the stbl of type s8, an overflow value of -128 occurs,
> which triggers oob.

I don't quite understand where this assignment is. Where is the stbl 
being assigned 4294967168?

> 
> To avoid this issue, add a check for the index of the slot before using it.

This check simply ignores the problem. There is no report and no error 
returned. Ideally, this should result in an error and/or the filesystem 
being marked dirty.

> 
> [1]
> UBSAN: array-index-out-of-bounds in fs/jfs/jfs_dtree.c:1997:37
> index -128 is out of range for type 'struct dtslot[128]'
> CPU: 1 UID: 0 PID: 5842 Comm: syz-executor268 Not tainted 6.12.0-syzkaller-09073-g9f16d5e6f220 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> Call Trace:
>   <TASK>
>   __dump_stack lib/dump_stack.c:94 [inline]
>   dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
>   ubsan_epilogue lib/ubsan.c:231 [inline]
>   __ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
>   dtSplitRoot+0xc9c/0x1930 fs/jfs/jfs_dtree.c:1997
>   dtSplitUp fs/jfs/jfs_dtree.c:992 [inline]
>   dtInsert+0x12cd/0x6c10 fs/jfs/jfs_dtree.c:870
>   jfs_symlink+0x827/0x10f0 fs/jfs/namei.c:1020
>   vfs_symlink+0x137/0x2e0 fs/namei.c:4669
>   do_symlinkat+0x222/0x3a0 fs/namei.c:4695
>   __do_sys_symlink fs/namei.c:4716 [inline]
>   __se_sys_symlink fs/namei.c:4714 [inline]
>   __x64_sys_symlink+0x7a/0x90 fs/namei.c:4714
>   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>   do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>   entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> Reported-and-tested-by: syzbot+99491d74a9931659cf48@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=99491d74a9931659cf48
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>   fs/jfs/jfs_dtree.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
> index 8f85177f284b..71463ad751c2 100644
> --- a/fs/jfs/jfs_dtree.c
> +++ b/fs/jfs/jfs_dtree.c
> @@ -1994,6 +1994,9 @@ static int dtSplitRoot(tid_t tid,
>   
>   		stbl = DT_GETSTBL(rp);
>   		for (n = 0; n < rp->header.nextindex; n++) {
> +			if (stbl[n] >= ARRAY_SIZE(rp->slot))
> +				continue;
> +
>   			ldtentry = (struct ldtentry *) & rp->slot[stbl[n]];
>   			modify_index(tid, ip, le32_to_cpu(ldtentry->index),
>   				     rbn, n, &mp, &lblock);