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

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] [day] [month] [year] [list]

Message-ID: <a5d8f41d-abe3-4a76-994f-6047148f5554@oracle.com>
Date: Mon, 2 Dec 2024 14:53:58 -0600
From: Dave Kleikamp <dave.kleikamp@...cle.com>
To: Matt Jan <zoo868e@...il.com>, jfs-discussion@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org, Shuan Khan <skhan@...uxfoundation.org>
Cc: syzbot+9e90a1c5eedb9dc4c6cc@...kaller.appspotmail.com
Subject: Re: [PATCH v4] jfs: UBSAN: shift-out-of-bounds in dbFindBits

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@...kaller.appspotmail.com
> Signed-off-by: Matt Jan <zoo868e@...il.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