Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ae6ce935-96eb-3d40-f252-54fff7eb0ea9@gmail.com>
Date:   Thu, 31 Aug 2023 20:49:09 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] jfs: fix array-index-out-of-bounds in dbFindLeaf
Content-Language: en-US
To:     Dave Kleikamp <dave.kleikamp@oracle.com>, shaggy@kernel.org,
        liushixin2@huawei.com
Cc:     linux-kernel@vger.kernel.org, jfs-discussion@lists.sourceforge.net,
        Linux-kernel-mentees@lists.linuxfoundation.org,
        syzbot+aea1ad91e854d0a83e04@syzkaller.appspotmail.com
References: <20230829165244.460154-1-ghandatmanas@gmail.com>
 <e62dbf53-9d74-45e5-b44e-53140672b8b5@oracle.com>
From:   Manas Ghandat <ghandatmanas@gmail.com>
In-Reply-To: <e62dbf53-9d74-45e5-b44e-53140672b8b5@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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?

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@gmail.com>
>> Reported-by: syzbot+aea1ad91e854d0a83e04@syzkaller.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;
>>           }