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

[Dave Kleikamp]

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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Matt Jan <zoo868e@xxxxxxxxx>
> ---
> 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