2024-04-23 22:25:40

by syzbot

[permalink] [raw]
Subject: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

Hello,

syzbot found the following issue on:

HEAD commit: 3cdb45594619 Merge tag 's390-6.9-4' of git://git.kernel.or..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=115a2547180000
kernel config: https://syzkaller.appspot.com/x/.config?x=f47e5e015c177e57
dashboard link: https://syzkaller.appspot.com/bug?extid=241c815bda521982cb49
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=109bb8f7180000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17e54bab180000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/4befd8e98bed/disk-3cdb4559.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/efc09f95602f/vmlinux-3cdb4559.xz
kernel image: https://storage.googleapis.com/syzbot-assets/29a54be03694/bzImage-3cdb4559.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/784deb2bb8a2/mount_0.gz

Bisection is inconclusive: the issue happens on the oldest tested release.

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=14dc8b53180000
final oops: https://syzkaller.appspot.com/x/report.txt?x=16dc8b53180000
console output: https://syzkaller.appspot.com/x/log.txt?x=12dc8b53180000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]

------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_imap.c:886:2
index 524288 is out of range for type 'struct mutex[128]'
CPU: 1 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc4-syzkaller-00173-g3cdb45594619 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
ubsan_epilogue lib/ubsan.c:217 [inline]
__ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:415
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
---[ end trace ]---
Kernel panic - not syncing: UBSAN: panic_on_warn set ...
CPU: 1 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc4-syzkaller-00173-g3cdb45594619 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
panic+0x349/0x860 kernel/panic.c:348
check_panic_on_warn+0x86/0xb0 kernel/panic.c:241
ubsan_epilogue lib/ubsan.c:222 [inline]
__ubsan_handle_out_of_bounds+0x141/0x150 lib/ubsan.c:415
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at [email protected].

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup


2024-04-24 04:12:10

by Jeongjun Park

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

please test array-index-out-of-bounds in diFree

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master

---
fs/jfs/jfs_imap.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..977751b30489 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -881,6 +881,11 @@ int diFree(struct inode *ip)
*/
agno = BLKTOAG(JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb));

+ if(agno >= MAXAG){
+ jfs_error(ip->i_sb, "invalid array index (agno >= MAXAG), agno = %d\n", agno);
+ return -ENOMEM;
+ }
+
/* Lock the AG specific inode map information
*/
AG_LOCK(imap, agno);
--
2.34.1

2024-04-24 04:39:19

by syzbot

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: [email protected]

Tested on:

commit: 9d1ddab2 Merge tag '6.9-rc5-smb-client-fixes' of git:/..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master
console output: https://syzkaller.appspot.com/x/log.txt?x=13b29a80980000
kernel config: https://syzkaller.appspot.com/x/.config?x=5a05c230e142f2bc
dashboard link: https://syzkaller.appspot.com/bug?extid=241c815bda521982cb49
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=17b75ae7180000

Note: testing is done by a robot and is best-effort only.

2024-04-24 17:52:03

by Jeongjun Park

[permalink] [raw]
Subject: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

[syzbot report]
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_imap.c:886:2
index 524288 is out of range for type 'struct mutex[128]'
CPU: 0 PID: 113 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00036-g9d1ddab261f3 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
ubsan_epilogue lib/ubsan.c:231 [inline]
__ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
---[ end trace ]---
Kernel panic - not syncing: UBSAN: panic_on_warn set ...
CPU: 1 PID: 113 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00036-g9d1ddab261f3 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
panic+0x349/0x860 kernel/panic.c:348
check_panic_on_warn+0x86/0xb0 kernel/panic.c:241
ubsan_epilogue lib/ubsan.c:236 [inline]
__ubsan_handle_out_of_bounds+0x141/0x150 lib/ubsan.c:429
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Kernel Offset: disabled
Rebooting in 86400 seconds..
===========================================================

Due to overflow, a value that is too large is entered into the agno
value. Therefore, we need to add code to check the agno value.

Reported-by: [email protected]
Signed-off-by: Jeongjun Park <[email protected]>
---
fs/jfs/jfs_imap.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..0aac083bc0db 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -881,6 +881,11 @@ int diFree(struct inode *ip)
*/
agno = BLKTOAG(JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb));

+ if(agno >= MAXAG || agno < 0){
+ jfs_error(ip->i_sb, "invalid array index (0 <= agno < MAXAG), agno = %d\n", agno);
+ return -ENOMEM;
+ }
+
/* Lock the AG specific inode map information
*/
AG_LOCK(imap, agno);
--
2.34.1

2024-04-24 20:46:54

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

On Thu, Apr 25, 2024 at 02:22:40AM +0900, Jeongjun Park wrote:
> Due to overflow, a value that is too large is entered into the agno
> value. Therefore, we need to add code to check the agno value.

This is clearly wrong.

#define BLKTOAG(b,sbi) ((b) >> ((sbi)->bmap->db_agl2size))

I'd suggest that something has either corrupted the sbi->bmap
pointer or the sbi->bmap->db_agl2size value.

All your patch does is cover up the problem, not fix it.

> Reported-by: [email protected]
> Signed-off-by: Jeongjun Park <[email protected]>
> ---
> fs/jfs/jfs_imap.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..0aac083bc0db 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -881,6 +881,11 @@ int diFree(struct inode *ip)
> */
> agno = BLKTOAG(JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb));
>
> + if(agno >= MAXAG || agno < 0){
> + jfs_error(ip->i_sb, "invalid array index (0 <= agno < MAXAG), agno = %d\n", agno);
> + return -ENOMEM;
> + }
> +
> /* Lock the AG specific inode map information
> */
> AG_LOCK(imap, agno);
> --
> 2.34.1
>

2024-04-25 03:16:11

by Jeongjun Park

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

please test array-index-out-of-bounds in diFree

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master

---
fs/jfs/jfs_dmap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index cb3cda1390ad..773b9263e497 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -206,6 +206,7 @@ int dbMount(struct inode *ipbmap)
bmp->db_agwidth = le32_to_cpu(dbmp_le->dn_agwidth);
bmp->db_agstart = le32_to_cpu(dbmp_le->dn_agstart);
bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
+ printk(KERN_DEBUG "dmMount : %d\n",bmp->db_agl2size);
if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
bmp->db_agl2size < 0) {
err = -EINVAL;
@@ -316,6 +317,7 @@ int dbSync(struct inode *ipbmap)
dbmp_le->dn_agwidth = cpu_to_le32(bmp->db_agwidth);
dbmp_le->dn_agstart = cpu_to_le32(bmp->db_agstart);
dbmp_le->dn_agl2size = cpu_to_le32(bmp->db_agl2size);
+ printk(KERN_DEBUG "dbSync : %d\n",bmp->db_agl2size);
for (i = 0; i < MAXAG; i++)
dbmp_le->dn_agfree[i] = cpu_to_le64(bmp->db_agfree[i]);
dbmp_le->dn_agsize = cpu_to_le64(bmp->db_agsize);
@@ -3393,7 +3395,7 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)

bmp->db_agl2size = l2agsize;
bmp->db_agsize = 1 << l2agsize;
-
+ printk(KERN_DEBUG "dbExtendFS : %d\n",bmp->db_agl2size);
/* compute new number of AG */
agno = bmp->db_numag;
bmp->db_numag = newsize >> l2agsize;
--
2.34.1

2024-04-25 04:33:17

by syzbot

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
UBSAN: array-index-out-of-bounds in diFree

------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_imap.c:886:2
index 524288 is out of range for type 'struct mutex[128]'
CPU: 1 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
ubsan_epilogue lib/ubsan.c:231 [inline]
__ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
---[ end trace ]---
Kernel panic - not syncing: UBSAN: panic_on_warn set ...
CPU: 0 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
panic+0x349/0x860 kernel/panic.c:348
check_panic_on_warn+0x86/0xb0 kernel/panic.c:241
ubsan_epilogue lib/ubsan.c:236 [inline]
__ubsan_handle_out_of_bounds+0x141/0x150 lib/ubsan.c:429
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: e88c4cfc Merge tag 'for-6.9-rc5-tag' of git://git.kern..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master
console output: https://syzkaller.appspot.com/x/log.txt?x=142ec887180000
kernel config: https://syzkaller.appspot.com/x/.config?x=5a05c230e142f2bc
dashboard link: https://syzkaller.appspot.com/bug?extid=241c815bda521982cb49
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=130ccd27180000


2024-04-25 05:00:07

by Jeongjun Park

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

please test array-index-out-of-bounds in diFree

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master

---
fs/jfs/jfs_dmap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index cb3cda1390ad..773b9263e497 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -206,6 +206,7 @@ int dbMount(struct inode *ipbmap)
bmp->db_agwidth = le32_to_cpu(dbmp_le->dn_agwidth);
bmp->db_agstart = le32_to_cpu(dbmp_le->dn_agstart);
bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
+ printk("dmMount : %d\n",bmp->db_agl2size);
if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
bmp->db_agl2size < 0) {
err = -EINVAL;
@@ -316,6 +317,7 @@ int dbSync(struct inode *ipbmap)
dbmp_le->dn_agwidth = cpu_to_le32(bmp->db_agwidth);
dbmp_le->dn_agstart = cpu_to_le32(bmp->db_agstart);
dbmp_le->dn_agl2size = cpu_to_le32(bmp->db_agl2size);
+ printk("dbSync : %d\n",bmp->db_agl2size);
for (i = 0; i < MAXAG; i++)
dbmp_le->dn_agfree[i] = cpu_to_le64(bmp->db_agfree[i]);
dbmp_le->dn_agsize = cpu_to_le64(bmp->db_agsize);
@@ -3393,7 +3395,7 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)

bmp->db_agl2size = l2agsize;
bmp->db_agsize = 1 << l2agsize;
-
+ printk("dbExtendFS : %d\n",bmp->db_agl2size);
/* compute new number of AG */
agno = bmp->db_numag;
bmp->db_numag = newsize >> l2agsize;
--
2.34.1

2024-04-25 05:50:15

by syzbot

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
UBSAN: array-index-out-of-bounds in diFree

------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_imap.c:886:2
index 524288 is out of range for type 'struct mutex[128]'
CPU: 1 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
ubsan_epilogue lib/ubsan.c:231 [inline]
__ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
---[ end trace ]---
Kernel panic - not syncing: UBSAN: panic_on_warn set ...
CPU: 1 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
panic+0x349/0x860 kernel/panic.c:348
check_panic_on_warn+0x86/0xb0 kernel/panic.c:241
ubsan_epilogue lib/ubsan.c:236 [inline]
__ubsan_handle_out_of_bounds+0x141/0x150 lib/ubsan.c:429
diFree+0x21c3/0x2fb0 fs/jfs/jfs_imap.c:886
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: e88c4cfc Merge tag 'for-6.9-rc5-tag' of git://git.kern..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master
console output: https://syzkaller.appspot.com/x/log.txt?x=1104a90f180000
kernel config: https://syzkaller.appspot.com/x/.config?x=5a05c230e142f2bc
dashboard link: https://syzkaller.appspot.com/bug?extid=241c815bda521982cb49
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=17d15237180000


2024-04-25 08:57:18

by Jeongjun Park

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

please test array-index-out-of-bounds in diFree

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master

---
fs/jfs/jfs_dmap.c | 4 +++-
fs/jfs/jfs_imap.c | 8 ++++++--
2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index cb3cda1390ad..6681a6272ae7 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -206,6 +206,7 @@ int dbMount(struct inode *ipbmap)
bmp->db_agwidth = le32_to_cpu(dbmp_le->dn_agwidth);
bmp->db_agstart = le32_to_cpu(dbmp_le->dn_agstart);
bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
+ printk("dmMount : %d\n",bmp->db_agl2size);
if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
bmp->db_agl2size < 0) {
err = -EINVAL;
@@ -316,6 +317,7 @@ int dbSync(struct inode *ipbmap)
dbmp_le->dn_agwidth = cpu_to_le32(bmp->db_agwidth);
dbmp_le->dn_agstart = cpu_to_le32(bmp->db_agstart);
dbmp_le->dn_agl2size = cpu_to_le32(bmp->db_agl2size);
+ printk("dbSync : %d\n",bmp->db_agl2size);
for (i = 0; i < MAXAG; i++)
dbmp_le->dn_agfree[i] = cpu_to_le64(bmp->db_agfree[i]);
dbmp_le->dn_agsize = cpu_to_le64(bmp->db_agsize);
@@ -3393,7 +3395,7 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)

bmp->db_agl2size = l2agsize;
bmp->db_agsize = 1 << l2agsize;
-
+ printk("dbExtendFS : %d\n",bmp->db_agl2size);
/* compute new number of AG */
agno = bmp->db_numag;
bmp->db_numag = newsize >> l2agsize;
diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..c39f7685aa97 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -339,7 +339,7 @@ int diRead(struct inode *ip)

/* get the ag for the iag */
agstart = le64_to_cpu(iagp->agstart);
-
+ printk("diRead : %ld\n",agstart);
release_metapage(mp);

rel_inode = (ino & (INOSPERPAGE - 1));
@@ -881,6 +881,8 @@ int diFree(struct inode *ip)
*/
agno = BLKTOAG(JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb));

+ printk("%lu %d %d",JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb)->bmap->db_agl2size, agno);
+
/* Lock the AG specific inode map information
*/
AG_LOCK(imap, agno);
@@ -1298,6 +1300,7 @@ diInitInode(struct inode *ip, int iagno, int ino, int extno, struct iag * iagp)
jfs_ip->ixpxd = iagp->inoext[extno];
jfs_ip->agstart = le64_to_cpu(iagp->agstart);
jfs_ip->active_ag = -1;
+ printk("diInitInode : %lu",jfs_ip->agstart);
}


@@ -1908,6 +1911,7 @@ static int diAllocExt(struct inomap * imap, int agno, struct inode *ip)
*/
iagp->agstart =
cpu_to_le64(AGTOBLK(agno, imap->im_ipimap));
+ printk("diAllocExt : %lu\n",iagp->agstart);
} else {
/* read the iag.
*/
@@ -2898,7 +2902,7 @@ int diExtendFS(struct inode *ipimap, struct inode *ipbmap)
agstart = le64_to_cpu(iagp->agstart);
n = agstart >> mp->db_agl2size;
iagp->agstart = cpu_to_le64((s64)n << mp->db_agl2size);
-
+ printk("diExtendFs : %lu %d\n",agstart, mp->db_agl2size);
/* compute backed inodes */
numinos = (EXTSPERIAG - le32_to_cpu(iagp->nfreeexts))
<< L2INOSPEREXT;
--
2.34.1

2024-04-25 09:04:59

by Jeongjun Park

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

please test array-index-out-of-bounds in diFree

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master

---
fs/jfs/jfs_dmap.c | 4 +++-
fs/jfs/jfs_imap.c | 8 ++++++--
2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index cb3cda1390ad..6681a6272ae7 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -206,6 +206,7 @@ int dbMount(struct inode *ipbmap)
bmp->db_agwidth = le32_to_cpu(dbmp_le->dn_agwidth);
bmp->db_agstart = le32_to_cpu(dbmp_le->dn_agstart);
bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
+ printk("dmMount : %d\n",bmp->db_agl2size);
if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
bmp->db_agl2size < 0) {
err = -EINVAL;
@@ -316,6 +317,7 @@ int dbSync(struct inode *ipbmap)
dbmp_le->dn_agwidth = cpu_to_le32(bmp->db_agwidth);
dbmp_le->dn_agstart = cpu_to_le32(bmp->db_agstart);
dbmp_le->dn_agl2size = cpu_to_le32(bmp->db_agl2size);
+ printk("dbSync : %d\n",bmp->db_agl2size);
for (i = 0; i < MAXAG; i++)
dbmp_le->dn_agfree[i] = cpu_to_le64(bmp->db_agfree[i]);
dbmp_le->dn_agsize = cpu_to_le64(bmp->db_agsize);
@@ -3393,7 +3395,7 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)

bmp->db_agl2size = l2agsize;
bmp->db_agsize = 1 << l2agsize;
-
+ printk("dbExtendFS : %d\n",bmp->db_agl2size);
/* compute new number of AG */
agno = bmp->db_numag;
bmp->db_numag = newsize >> l2agsize;
diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..c39f7685aa97 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -339,7 +339,7 @@ int diRead(struct inode *ip)

/* get the ag for the iag */
agstart = le64_to_cpu(iagp->agstart);
-
+ printk("diRead : %ld\n",agstart);
release_metapage(mp);

rel_inode = (ino & (INOSPERPAGE - 1));
@@ -881,6 +881,8 @@ int diFree(struct inode *ip)
*/
agno = BLKTOAG(JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb));

+ printk("%lu %d %d",JFS_IP(ip)->agstart, JFS_SBI(ip->i_sb)->bmap->db_agl2size, agno);
+
/* Lock the AG specific inode map information
*/
AG_LOCK(imap, agno);
@@ -1298,6 +1300,7 @@ diInitInode(struct inode *ip, int iagno, int ino, int extno, struct iag * iagp)
jfs_ip->ixpxd = iagp->inoext[extno];
jfs_ip->agstart = le64_to_cpu(iagp->agstart);
jfs_ip->active_ag = -1;
+ printk("diInitInode : %lu",jfs_ip->agstart);
}


@@ -1908,6 +1911,7 @@ static int diAllocExt(struct inomap * imap, int agno, struct inode *ip)
*/
iagp->agstart =
cpu_to_le64(AGTOBLK(agno, imap->im_ipimap));
+ printk("diAllocExt : %lu\n",iagp->agstart);
} else {
/* read the iag.
*/
@@ -2898,7 +2902,7 @@ int diExtendFS(struct inode *ipimap, struct inode *ipbmap)
agstart = le64_to_cpu(iagp->agstart);
n = agstart >> mp->db_agl2size;
iagp->agstart = cpu_to_le64((s64)n << mp->db_agl2size);
-
+ printk("diExtendFs : %lu %d\n",agstart, mp->db_agl2size);
/* compute backed inodes */
numinos = (EXTSPERIAG - le32_to_cpu(iagp->nfreeexts))
<< L2INOSPEREXT;
--
2.34.1

2024-04-25 09:39:16

by syzbot

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
UBSAN: array-index-out-of-bounds in diFree

140741783322624 13 524288
------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_imap.c:888:2
index 524288 is out of range for type 'struct mutex[128]'
CPU: 1 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
ubsan_epilogue lib/ubsan.c:231 [inline]
__ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
diFree+0x21ec/0x2fe0 fs/jfs/jfs_imap.c:888
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
---[ end trace ]---
Kernel panic - not syncing: UBSAN: panic_on_warn set ...
CPU: 1 PID: 111 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
panic+0x349/0x860 kernel/panic.c:348
check_panic_on_warn+0x86/0xb0 kernel/panic.c:241
ubsan_epilogue lib/ubsan.c:236 [inline]
__ubsan_handle_out_of_bounds+0x141/0x150 lib/ubsan.c:429
diFree+0x21ec/0x2fe0 fs/jfs/jfs_imap.c:888
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: e88c4cfc Merge tag 'for-6.9-rc5-tag' of git://git.kern..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master
console output: https://syzkaller.appspot.com/x/log.txt?x=135717bb180000
kernel config: https://syzkaller.appspot.com/x/.config?x=5a05c230e142f2bc
dashboard link: https://syzkaller.appspot.com/bug?extid=241c815bda521982cb49
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=14aea028980000


2024-04-25 09:46:00

by syzbot

[permalink] [raw]
Subject: Re: [syzbot] [jfs?] UBSAN: array-index-out-of-bounds in diFree

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
UBSAN: array-index-out-of-bounds in diFree

140741783322624 13 524288
------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in fs/jfs/jfs_imap.c:888:2
index 524288 is out of range for type 'struct mutex[128]'
CPU: 0 PID: 112 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
ubsan_epilogue lib/ubsan.c:231 [inline]
__ubsan_handle_out_of_bounds+0x121/0x150 lib/ubsan.c:429
diFree+0x21ec/0x2fe0 fs/jfs/jfs_imap.c:888
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
---[ end trace ]---
Kernel panic - not syncing: UBSAN: panic_on_warn set ...
CPU: 0 PID: 112 Comm: jfsCommit Not tainted 6.9.0-rc5-syzkaller-00042-ge88c4cfcb7b8-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
panic+0x349/0x860 kernel/panic.c:348
check_panic_on_warn+0x86/0xb0 kernel/panic.c:241
ubsan_epilogue lib/ubsan.c:236 [inline]
__ubsan_handle_out_of_bounds+0x141/0x150 lib/ubsan.c:429
diFree+0x21ec/0x2fe0 fs/jfs/jfs_imap.c:888
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x2a8/0x630 fs/inode.c:667
txUpdateMap+0x829/0x9f0 fs/jfs/jfs_txnmgr.c:2367
txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
jfs_lazycommit+0x49a/0xb80 fs/jfs/jfs_txnmgr.c:2733
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: e88c4cfc Merge tag 'for-6.9-rc5-tag' of git://git.kern..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ master
console output: https://syzkaller.appspot.com/x/log.txt?x=1151ccef180000
kernel config: https://syzkaller.appspot.com/x/.config?x=5a05c230e142f2bc
dashboard link: https://syzkaller.appspot.com/bug?extid=241c815bda521982cb49
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=145717bb180000


2024-04-25 12:44:49

by Jeongjun Park

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

Through direct testing and debugging, I've determined that this
vulnerability occurs when mounting an incorrect image, leading to
the potential passing of an excessively large value to
'sbi->bmap->db_agl2size'. Importantly, there have been no instances
of memory corruption observed within 'sbi->bmap->db_agl2size'.

Therefore, I think implementing a patch that terminates the
function in cases where an invalid value is detected.

Thanks.

2024-04-25 14:11:34

by Jeongjun Park

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

Matthew Wilcox wrote:
> If that's the problem then the correct place to detect & reject this is
> during mount, not at inode free time.

I fixed the patch as you said. If you patch in this way, the
file system will not be affected by the vulnerability at all
due to the code structure.

Thanks.

---
fs/jfs/jfs_imap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..ba0aa2f145cc 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
int diRead(struct inode *ip)
{
struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
- int iagno, ino, extno, rc;
+ int iagno, ino, extno, rc, agno;
struct inode *ipimap;
struct dinode *dp;
struct iag *iagp;
@@ -339,6 +339,9 @@ int diRead(struct inode *ip)

/* get the ag for the iag */
agstart = le64_to_cpu(iagp->agstart);
+ agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
+ if(agno >= MAXAG || agno < 0)
+ return -EIO;

release_metapage(mp);

--
2.34.1

2024-04-25 14:14:53

by Dave Kleikamp

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

On 4/25/24 9:10AM, Jeongjun Park wrote:
> Matthew Wilcox wrote:
>> If that's the problem then the correct place to detect & reject this is
>> during mount, not at inode free time.
>
> I fixed the patch as you said. If you patch in this way, the
> file system will not be affected by the vulnerability at all
> due to the code structure.
>
> Thanks.
>
> ---
> fs/jfs/jfs_imap.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..ba0aa2f145cc 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
> int diRead(struct inode *ip)
> {
> struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
> - int iagno, ino, extno, rc;
> + int iagno, ino, extno, rc, agno;
> struct inode *ipimap;
> struct dinode *dp;
> struct iag *iagp;
> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>
> /* get the ag for the iag */
> agstart = le64_to_cpu(iagp->agstart);
> + agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
> + if(agno >= MAXAG || agno < 0)
> + return -EIO;

That's the right idea, but move the new code after the call to
release_metapage().
>
> release_metapage(mp);
>

2024-04-25 14:17:18

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

On Thu, Apr 25, 2024 at 11:10:38PM +0900, Jeongjun Park wrote:
> Matthew Wilcox wrote:
> > If that's the problem then the correct place to detect & reject this is
> > during mount, not at inode free time.
>
> I fixed the patch as you said. If you patch in this way, the
> file system will not be affected by the vulnerability at all
> due to the code structure.

It should be checked earlier than this. There's this code in
dbMount(). Why isn't this catching it?

bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
bmp->db_agl2size < 0) {
err = -EINVAL;
goto err_release_metapage;
}

if (((bmp->db_mapsize - 1) >> bmp->db_agl2size) > MAXAG) {
err = -EINVAL;
goto err_release_metapage;
}


> Thanks.
>
> ---
> fs/jfs/jfs_imap.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..ba0aa2f145cc 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
> int diRead(struct inode *ip)
> {
> struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
> - int iagno, ino, extno, rc;
> + int iagno, ino, extno, rc, agno;
> struct inode *ipimap;
> struct dinode *dp;
> struct iag *iagp;
> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>
> /* get the ag for the iag */
> agstart = le64_to_cpu(iagp->agstart);
> + agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
> + if(agno >= MAXAG || agno < 0)
> + return -EIO;
>
> release_metapage(mp);
>
> --
> 2.34.1

2024-04-25 14:27:05

by Dave Kleikamp

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

On 4/25/24 9:17AM, Matthew Wilcox wrote:
> On Thu, Apr 25, 2024 at 11:10:38PM +0900, Jeongjun Park wrote:
>> Matthew Wilcox wrote:
>>> If that's the problem then the correct place to detect & reject this is
>>> during mount, not at inode free time.
>>
>> I fixed the patch as you said. If you patch in this way, the
>> file system will not be affected by the vulnerability at all
>> due to the code structure.
>
> It should be checked earlier than this.

Right. This is preferable.

> There's this code in
> dbMount(). Why isn't this catching it?

db_agstart is not checked in this code. That may be the bad data in this
case.

There are probably dozens more sanity checks that are missing when data
is first read from the disk.

Shaggy

>
> bmp->db_agl2size = le32_to_cpu(dbmp_le->dn_agl2size);
> if (bmp->db_agl2size > L2MAXL2SIZE - L2MAXAG ||
> bmp->db_agl2size < 0) {
> err = -EINVAL;
> goto err_release_metapage;
> }
>
> if (((bmp->db_mapsize - 1) >> bmp->db_agl2size) > MAXAG) {
> err = -EINVAL;
> goto err_release_metapage;
> }
>
>
>> Thanks.
>>
>> ---
>> fs/jfs/jfs_imap.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>> index 2ec35889ad24..ba0aa2f145cc 100644
>> --- a/fs/jfs/jfs_imap.c
>> +++ b/fs/jfs/jfs_imap.c
>> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
>> int diRead(struct inode *ip)
>> {
>> struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
>> - int iagno, ino, extno, rc;
>> + int iagno, ino, extno, rc, agno;
>> struct inode *ipimap;
>> struct dinode *dp;
>> struct iag *iagp;
>> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>>
>> /* get the ag for the iag */
>> agstart = le64_to_cpu(iagp->agstart);
>> + agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
>> + if(agno >= MAXAG || agno < 0)
>> + return -EIO;
>>
>> release_metapage(mp);
>>
>> --
>> 2.34.1

2024-04-25 14:38:13

by Jeongjun Park

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

Matthew Wilcox wrote:
> It should be checked earlier than this. There's this code in
> dbMount(). Why isn't this catching it?

Send final patch. With the patch that modified the location of
release_metapage(), out-of-bounds vulnerabilities can now be
sufficiently prevented.

Reported-by: [email protected]
Signed-off-by: Jeongjun Park <[email protected]>
---
fs/jfs/jfs_imap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..cad1798dc892 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
int diRead(struct inode *ip)
{
struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
- int iagno, ino, extno, rc;
+ int iagno, ino, extno, rc, agno;
struct inode *ipimap;
struct dinode *dp;
struct iag *iagp;
@@ -339,8 +339,11 @@ int diRead(struct inode *ip)

/* get the ag for the iag */
agstart = le64_to_cpu(iagp->agstart);
+ agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));

release_metapage(mp);
+ if(agno >= MAXAG || agno < 0)
+ return -EIO;

rel_inode = (ino & (INOSPERPAGE - 1));
pageno = blkno >> sbi->l2nbperpage;
--
2.34.1

2024-04-25 14:50:40

by Jeongjun Park

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

Matthew Wilcox wrote:
> It should be checked earlier than this. There's this code in
> dbMount(). Why isn't this catching it?

This vulnerability occurs because a very large value can be passed
to iagp->agstart. So that code doesn't prevent the vulnerability.

2024-04-25 15:21:54

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

On Thu, Apr 25, 2024 at 09:44:33PM +0900, Jeongjun Park wrote:
> Through direct testing and debugging, I've determined that this
> vulnerability occurs when mounting an incorrect image, leading to
> the potential passing of an excessively large value to
> 'sbi->bmap->db_agl2size'. Importantly, there have been no instances
> of memory corruption observed within 'sbi->bmap->db_agl2size'.
>
> Therefore, I think implementing a patch that terminates the
> function in cases where an invalid value is detected.

If that's the problem then the correct place to detect & reject this is
during mount, not at inode free time.

2024-04-25 22:47:59

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

On Thu, Apr 25, 2024 at 11:24:34PM +0900, Jeongjun Park wrote:
> Matthew Wilcox wrote:
> > It should be checked earlier than this. There's this code in
> > dbMount(). Why isn't this catching it?
>
> This vulnerability occurs because a very large value can be passed
> to iagp->agstart. So that code doesn't prevent the vulnerability.

In your earlier mail, you said the large value was found in db_agl2size.
If the problem is in agstart then diRead() is the right place to check it.

2024-04-26 01:43:06

by Jeongjun Park

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

Matthew Wilcox wrote:
> In your earlier mail, you said the large value was found in db_agl2size.
> If the problem is in agstart then diRead() is the right place to check it.

Oh, I was so distracted last time that I wrote the explanation
incorrectly. I'm sorry.

To explain it accurately, if you pass a very large value to agstart
and set the value passed to db_agl2size to be small, it can be
manipulated so that a value greater than MAXAG is output when the
"agstart >> db_agl2size" operation is performed.
This results in an out-of-bounds vulnerability.

And the final patch before is the one that fixes diRead().

Thanks.

2024-04-26 02:34:35

by Jeongjun Park

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

I forgot to add Dave to the cc, so I'm sending it again.

Send final patch. With the patch that modified the location of
release_metapage(), out-of-bounds vulnerabilities can now be
sufficiently prevented.

Thanks.

Reported-by: [email protected]
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jeongjun Park <[email protected]>
---
fs/jfs/jfs_imap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..cad1798dc892 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
int diRead(struct inode *ip)
{
struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
- int iagno, ino, extno, rc;
+ int iagno, ino, extno, rc, agno;
struct inode *ipimap;
struct dinode *dp;
struct iag *iagp;
@@ -339,8 +339,11 @@ int diRead(struct inode *ip)

/* get the ag for the iag */
agstart = le64_to_cpu(iagp->agstart);
+ agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));

release_metapage(mp);
+ if(agno >= MAXAG || agno < 0)
+ return -EIO;

rel_inode = (ino & (INOSPERPAGE - 1));
pageno = blkno >> sbi->l2nbperpage;
--
2.34.1

2024-04-26 03:42:15

by Jeongjun Park

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

Matthew Wilcox wrote:
> This is not a good commit message.

> > + if(agno >= MAXAG || agno < 0)
>
> Please follow normal kernel whitespace rules -- one space between 'if'
> and the open paren.

Has confirmed. This is a patch that re-edited the relevant part to
comply with the rules.

Thanks.

Reported-by: [email protected]
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jeongjun Park <[email protected]>
---
fs/jfs/jfs_imap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 2ec35889ad24..1407feccbc2d 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
int diRead(struct inode *ip)
{
struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
- int iagno, ino, extno, rc;
+ int iagno, ino, extno, rc, agno;
struct inode *ipimap;
struct dinode *dp;
struct iag *iagp;
@@ -339,8 +339,11 @@ int diRead(struct inode *ip)

/* get the ag for the iag */
agstart = le64_to_cpu(iagp->agstart);
+ agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));

release_metapage(mp);
+ if (agno >= MAXAG || agno < 0)
+ return -EIO;

rel_inode = (ino & (INOSPERPAGE - 1));
pageno = blkno >> sbi->l2nbperpage;
--
2.34.1

2024-04-26 05:56:55

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree

On Fri, Apr 26, 2024 at 11:34:12AM +0900, Jeongjun Park wrote:
> I forgot to add Dave to the cc, so I'm sending it again.
>
> Send final patch. With the patch that modified the location of
> release_metapage(), out-of-bounds vulnerabilities can now be
> sufficiently prevented.

This is not a good commit message.

> + if(agno >= MAXAG || agno < 0)

Please follow normal kernel whitespace rules -- one space between 'if'
and the open paren.