linux-kernel - Re: [PATCH] jfs: fix array-index-out-of-bounds in dbFindLeaf

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 for Android: free password hash cracker in your pocket

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

Message-ID: <e62dbf53-9d74-45e5-b44e-53140672b8b5@oracle.com>
Date:   Tue, 29 Aug 2023 13:38:04 -0500
From:   Dave Kleikamp <dave.kleikamp@...cle.com>
To:     Manas Ghandat <ghandatmanas@...il.com>, shaggy@...nel.org,
        liushixin2@...wei.com
Cc:     linux-kernel@...r.kernel.org, jfs-discussion@...ts.sourceforge.net,
        Linux-kernel-mentees@...ts.linuxfoundation.org,
        syzbot+aea1ad91e854d0a83e04@...kaller.appspotmail.com
Subject: Re: [PATCH] jfs: fix array-index-out-of-bounds in dbFindLeaf

On 8/29/23 11:52AM, Manas Ghandat wrote:
> Currently while searching for dmtree_t for sufficient free blocks there
> is an array out of bounds while getting element in tp->dm_stree. Added
> the required bound check.
> 
> Ps: After I added the check I am getting the following log
> 
> [   22.661748][ T4425] ERROR: (device loop0): dbAllocAny: unable to allocate blocks
> [   22.661748][ T4425]
> [   22.665536][ T4425] ERROR: (device loop0): remounting filesystem as read-only
> [   22.667856][ T4425] jfs_mkdir: dtInsert returned -EIO
> [   22.669750][ T4425] ERROR: (device loop0): txAbort:
> 
> I was wondering if these checks are significant of not?

This won't work. dbFindLeaf() can be called from dbFindCtl() with struct 
dmapctl whose stree index can be as high as CTLTREESIZE which is larger 
than TREESIZE. A check against CTLTREESIZE might be better than nothing 
at all but won't necessarily detect an overflow. Currently, dbFindLeaf 
doesn't have anything to tell it which tree it is working on.

We could pass in the treesize as an argument to dbFindCtl() if we can't 
come up with something simpler.

Shaggy

> 
> Signed-off-by: Manas Ghandat <ghandatmanas@...il.com>
> Reported-by: syzbot+aea1ad91e854d0a83e04@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=aea1ad91e854d0a83e04
> ---
>   fs/jfs/jfs_dmap.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index a14a0f18a4c4..5af17b2287be 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -2948,6 +2948,10 @@ static int dbFindLeaf(dmtree_t * tp, int l2nb, int *leafidx)
>   			/* sufficient free space found.  move to the next
>   			 * level (or quit if this is the last level).
>   			 */
> +
> +			if (x + n > TREESIZE)
> +				return -ENOSPC;
> +
>   			if (l2nb <= tp->dmt_stree[x + n])
>   				break;
>   		}