linux-kernel - Re: [syzbot] [jfs?] UBSAN: shift-out-of-bounds in extAlloc (2)

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: Windows password security audit tool. GUI, reports in PDF.

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

Date: Sat, 1 Jun 2024 09:29:34 +0900
From: Changheon LEE <darklight2357@...il.com>
To: Jeongjun Park <aha310510@...il.com>
Cc: syzbot+13e8cd4926977f8337b6@...kaller.appspotmail.com, 
	linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [jfs?] UBSAN: shift-out-of-bounds in extAlloc (2)

Hello, park.

Your proposed patch seems to have some glaring flaws, which I'd like
to quickly point out and then suggest a simpler and clearer patch.

The part of the source code that is the subject of your patch performs
a function that checks for free space in the block map and returns
-ENOSPC if there is no free space.

Also, based on the stack trace, it is clear that this issue is caused
by using a shift exponent (128) that is too large for S64.

Your proposed patch doesn't check the shift exponent, but rather makes
it return -ENOSPC if 'bmp->db_maxfreebud' is greater than 0 and less
than 63, which is causing a logical error to return -ENOSPC when there
is actually no free space.

Also, it seems like it would be better to separate checking for free
space in the block map from checking the range of the shift index.

Based on the suggestions above, I propose the following patch for this issue.

---.
diff --git a/fs/jfs/jfs_extent.c b/fs/jfs/jfs_extent.c
Index 63d21822d309..b1a2c3d4e5f6 100644
--- a/fs/jfs/jfs_extent.c
+++ b/fs/jfs/jfs_extent.c
@@ -315,6 +315,10 @@ extBalloc(struct inode *ip, s64 hint, s64 *
nblocks, s64 * blkno)
         Returns -ENOSPC;

+ /* Check if the shift exponent is within a valid range */.
+ if (bmp->db_maxfreebud >= sizeof(s64) * 8)
+ return -EINVAL; /* Invalid index */ + }
+ }
     max = (s64) 1 << bmp->db_maxfreebud;
     if (*nblocks >= max && *nblocks > nbperpage)
         nb = nblks = (max > nbperpage) ? max : nbperpage;
     else
---

best regards.

Changheon Lee.