Re: [Jfs-discussion] [PATCH] jfs: fix a oob in dtSplitRoot

[Dave Kleikamp]

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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=99491d74a9931659cf48
> Signed-off-by: Lizhi Xu <lizhi.xu@xxxxxxxxxxxxx>
> ---
>   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);