FS:JFS:UBSAN:array-index-out-of-bounds in xtInsert

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

Posted by Osama Muhammad 2 years, 3 months ago

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@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=55a7541cfd25df68109e
Signed-off-by: Osama Muhammad <osmtendev@gmail.com>
Signed-off-by: Mushahid Hussain <mushi.shar@gmail.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) {
+		rc = -EINVAL;
+		goto out;
+	}
 	xad = &p->xad[index];
 	XT_PUTENTRY(xad, xflag, xoff, xlen, xaddr);
 
-- 
2.34.1

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

Posted by Dave Kleikamp 2 years, 3 months ago

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@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=55a7541cfd25df68109e
> Signed-off-by: Osama Muhammad <osmtendev@gmail.com>
> Signed-off-by: Mushahid Hussain <mushi.shar@gmail.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);
>