[v4] jfs: UBSAN: shift-out-of-bounds in dbFindBits

[PATCH v4] jfs: UBSAN: shift-out-of-bounds in dbFindBits

Posted by Matt Jan 1 year, 3 months ago

Ensure l2nb is less than BUDMIN by performing a sanity check in the caller.
Return -EIO if the check fails.

#syz test

Reported-by: syzbot+9e90a1c5eedb9dc4c6cc@syzkaller.appspotmail.com
Signed-off-by: Matt Jan <zoo868e@gmail.com>
---
Changes in v4: Thanks to Shaggy for the review. We now perform a sanity check instead of continuing as if nothing is wrong.
Changes in v3: Return the result earlier instead of assert it
Changes in v2: Test if the patch resolve the issue through syzbot and reference the reporter

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

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index 974ecf5e0d95..89c22a18314f 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -1217,7 +1217,7 @@ dbAllocNear(struct bmap * bmp,
 	int word, lword, rc;
 	s8 *leaf;
 
-	if (dp->tree.leafidx != cpu_to_le32(LEAFIND)) {
+	if (dp->tree.leafidx != cpu_to_le32(LEAFIND) || l2nb >= L2DBWORD) {
 		jfs_error(bmp->db_ipbmap->i_sb, "Corrupt dmap page\n");
 		return -EIO;
 	}
@@ -1969,7 +1969,7 @@ dbAllocDmapLev(struct bmap * bmp,
 	if (dbFindLeaf((dmtree_t *) &dp->tree, l2nb, &leafidx, false))
 		return -ENOSPC;
 
-	if (leafidx < 0)
+	if (leafidx < 0 || l2nb >= L2DBWORD)
 		return -EIO;
 
 	/* determine the block number within the file system corresponding
-- 
2.25.1

Re: [PATCH v4] jfs: UBSAN: shift-out-of-bounds in dbFindBits

Posted by Dave Kleikamp 1 year, 2 months ago

On 11/1/24 4:59AM, Matt Jan wrote:
> Ensure l2nb is less than BUDMIN by performing a sanity check in the caller.
> Return -EIO if the check fails.

Sorry for the delay again, but I'm still not okay with this patch.

It's possible for l2nb to be greater than L2DBWORD if and only if the 
entire dmap page represents free space.

In dbAllocNear, there is a test:
	if (leaf[word] < l2nb)
before dbFindbits is called. This will prevent the problem in dbFindbits 
from this path. The problem still remains in dbAllocDmapLev since there 
is no similar check.

> 
> #syz test
> 
> Reported-by: syzbot+9e90a1c5eedb9dc4c6cc@syzkaller.appspotmail.com
> Signed-off-by: Matt Jan <zoo868e@gmail.com>
> ---
> Changes in v4: Thanks to Shaggy for the review. We now perform a sanity check instead of continuing as if nothing is wrong.
> Changes in v3: Return the result earlier instead of assert it
> Changes in v2: Test if the patch resolve the issue through syzbot and reference the reporter
> 
>   fs/jfs/jfs_dmap.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index 974ecf5e0d95..89c22a18314f 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -1217,7 +1217,7 @@ dbAllocNear(struct bmap * bmp,
>   	int word, lword, rc;
>   	s8 *leaf;
>   
> -	if (dp->tree.leafidx != cpu_to_le32(LEAFIND)) {
> +	if (dp->tree.leafidx != cpu_to_le32(LEAFIND) || l2nb >= L2DBWORD) {
>   		jfs_error(bmp->db_ipbmap->i_sb, "Corrupt dmap page\n");
>   		return -EIO;
>   	}
> @@ -1969,7 +1969,7 @@ dbAllocDmapLev(struct bmap * bmp,
>   	if (dbFindLeaf((dmtree_t *) &dp->tree, l2nb, &leafidx, false))
>   		return -ENOSPC;
>   
> -	if (leafidx < 0)
> +	if (leafidx < 0 || l2nb >= L2DBWORD)
>   		return -EIO;
>   
>   	/* determine the block number within the file system corresponding

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

Posted by syzbot 1 year, 3 months ago

Hello,

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

Reported-by: syzbot+9e90a1c5eedb9dc4c6cc@syzkaller.appspotmail.com
Tested-by: syzbot+9e90a1c5eedb9dc4c6cc@syzkaller.appspotmail.com

Tested on:

commit:         6c52d4da Merge tag 'for-linus' of git://git.kernel.org..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=116eb2a7980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=35698c25466f388c
dashboard link: https://syzkaller.appspot.com/bug?extid=9e90a1c5eedb9dc4c6cc
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1293e6f7980000

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