linux-kernel - Re: [PATCH] FS:JFS:UBSAN:array-index-out-of-bounds in xtInsert

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 29 Aug 2023 12:38:28 -0500
From:   Dave Kleikamp <dave.kleikamp@...cle.com>
To:     Osama Muhammad <osmtendev@...il.com>
Cc:     jfs-discussion@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
        syzbot+55a7541cfd25df68109e@...kaller.appspotmail.com,
        Mushahid Hussain <mushi.shar@...il.com>
Subject: Re: [PATCH] FS:JFS:UBSAN:array-index-out-of-bounds in xtInsert

On 8/21/23 1:20PM, Osama Muhammad wrote:
> Syzkaller reported the following issue:
> 
> UBSAN: array-index-out-of-bounds in fs/jfs/jfs_xtree.c:622:9
> index 19 is out of range for type 'xad_t [18]'
> CPU: 1 PID: 3614 Comm: syz-executor388 Not tainted 6.0.0-rc6-syzkaller-00321-g105a36f3694e #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/26/2022
> Call Trace:
>   <TASK>
>   __dump_stack lib/dump_stack.c:88 [inline]
>   dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
>   ubsan_epilogue lib/ubsan.c:151 [inline]
>   __ubsan_handle_out_of_bounds+0x107/0x150 lib/ubsan.c:283
>   xtInsert+0xfbe/0x1020 fs/jfs/jfs_xtree.c:622
>   extAlloc+0xaa4/0x1030 fs/jfs/jfs_extent.c:145
>   jfs_get_block+0x410/0xe30 fs/jfs/inode.c:248
>   __block_write_begin_int+0x6f6/0x1d70 fs/buffer.c:2006
>   __block_write_begin fs/buffer.c:2056 [inline]
>   block_write_begin+0x93/0x1e0 fs/buffer.c:2117
>   jfs_write_begin+0x2d/0x60 fs/jfs/inode.c:304
>   generic_perform_write+0x314/0x610 mm/filemap.c:3738
>   __generic_file_write_iter+0x176/0x400 mm/filemap.c:3866
>   generic_file_write_iter+0xab/0x310 mm/filemap.c:3898
>   do_iter_write+0x6f0/0xc50 fs/read_write.c:855
>   vfs_writev fs/read_write.c:928 [inline]
>   do_writev+0x27a/0x470 fs/read_write.c:971
>   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>   do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
>   entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x7f0e179f7fb9
> Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007ffed4fe4448 EFLAGS: 00000246 ORIG_RAX: 0000000000000014
> RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f0e179f7fb9
> RDX: 0000000000000001 RSI: 0000000020000000 RDI: 0000000000000003
> RBP: 00007f0e179b7780 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000f8008000
> R13: 0000000000000000 R14: 00080000000000f4 R15: 0000000000000000
>   </TASK>
> 
> The issue is caused when the value of index becomes greater than the
> max size of array. Introducing check before accessing solves the issue.
> 
> The patch is tested via syzbot.
> 
> Reported-by: syzbot+55a7541cfd25df68109e@...kaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=55a7541cfd25df68109e
> Signed-off-by: Osama Muhammad <osmtendev@...il.com>
> Signed-off-by: Mushahid Hussain <mushi.shar@...il.com>
> ---
>   fs/jfs/jfs_xtree.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/fs/jfs/jfs_xtree.c b/fs/jfs/jfs_xtree.c
> index 2d304cee8..034a1613f 100644
> --- a/fs/jfs/jfs_xtree.c
> +++ b/fs/jfs/jfs_xtree.c
> @@ -619,6 +619,10 @@ int xtInsert(tid_t tid,		/* transaction id */
>   			(nextindex - index) * sizeof(xad_t));
>   
>   	/* insert the new entry: mark the entry NEW */
> +	if (index >= XTROOTMAXSLOT) {

This isn't quite right. XTROOTMAXSLOT only pertains to the root of the 
xtree, but we may be working on another xtree page. The problem is the 
definition of the xad array in xtpage_t. I need to fix it so that the 
definition works for both root and leaf tree elements.

> +		rc = -EINVAL;
> +		goto out;
> +	}
>   	xad = &p->xad[index];
>   	XT_PUTENTRY(xad, xflag, xoff, xlen, xaddr);
>