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

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

Message-ID: <f9475918-2186-49b8-b801-6f0f9e75f4fa@oracle.com>
Date:   Fri, 1 Sep 2023 12:08:09 -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/31/23 10:19AM, Manas Ghandat wrote:
> I was wondering if we could implement a get_tree_size macro wherein  we 
> could find the tree size so that we can do the comparison. SInce the 
> tp->dmt_stree is an array we can get its size and fix the out of bounds. 
> Would this thing work?

dmtree_t is a union of two nearly identical structures that both contain 
an stree. The only real difference in the structures is the size of the 
stree, so dbFindLeaf doesn't really know which is being used by the caller.

> 
> On 30/08/23 00:08, Dave Kleikamp wrote:
>> 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;
>>>           }