linux-kernel - Re: [PATCH] maple_tree: Fix potential out-of-bounds access in mas_wr_end

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: <1570feed-489c-82f7-8d6b-9f53e9ebb87e@gmail.com>
Date:   Sat, 6 May 2023 09:48:19 +0800
From:   Peng Zhang <perlyzhang@...il.com>
To:     "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, maple-tree@...ts.infradead.org,
        Peng Zhang <zhangpeng.00@...edance.com>
Subject: Re: [PATCH] maple_tree: Fix potential out-of-bounds access in
 mas_wr_end_piv()



在 2023/5/6 09:14, Liam R. Howlett 写道:
> * Peng Zhang <zhangpeng.00@...edance.com> [230503 23:14]:
>> Access to the pivots array may be out of bounds. Fix it by changing the
>> code to ensure that the index of the pivots does not go out of bounds.
>> It is difficult to assess user-visible impact.
> 
> This is indeed an issue.  There isn't any user-visible impact for
> current node types, since the overflow will access the slots and be
> corrected in the next if clause, but it's certainly better to fix this.
> 
> The commit message is also not as descriptive as necessary, perhaps
> something like:
> 
> Check the write offset end bounds before using it as the offset into the
> pivot array.  This avoids a possible out-of-bounds access on the pivot
> array if the write extends to the last slot in the node, in which case
> the node maximum should be used as the end pivot.
> 
> Reviewed-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> 
>>
>> Fixes: 54a611b60590 ("Maple Tree: add new data structure")
> 
> Cc stable ?
I don't know if it should be cc stable since Andrew says it
always needs to describe user-visible impact.
> 
>> Signed-off-by: Peng Zhang <zhangpeng.00@...edance.com>
>> ---
>>   lib/maple_tree.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
>> index 110a36479dced..5a49327444d76 100644
>> --- a/lib/maple_tree.c
>> +++ b/lib/maple_tree.c
>> @@ -4263,11 +4263,13 @@ static inline bool mas_wr_slot_store(struct ma_wr_state *wr_mas)
>>   
>>   static inline void mas_wr_end_piv(struct ma_wr_state *wr_mas)
>>   {
>> -	while ((wr_mas->mas->last > wr_mas->end_piv) &&
>> -	       (wr_mas->offset_end < wr_mas->node_end))
>> -		wr_mas->end_piv = wr_mas->pivots[++wr_mas->offset_end];
>> +	while ((wr_mas->offset_end < wr_mas->node_end) &&
>> +	       (wr_mas->mas->last > wr_mas->pivots[wr_mas->offset_end]))
>> +		wr_mas->offset_end++;
>>   
>> -	if (wr_mas->mas->last > wr_mas->end_piv)
>> +	if (wr_mas->offset_end < wr_mas->node_end)
>> +		wr_mas->end_piv = wr_mas->pivots[wr_mas->offset_end];
>> +	else
>>   		wr_mas->end_piv = wr_mas->mas->max;
>>   }
>>   
>> @@ -4424,7 +4426,6 @@ static inline void *mas_wr_store_entry(struct ma_wr_state *wr_mas)
>>   	}
>>   
>>   	/* At this point, we are at the leaf node that needs to be altered. */
>> -	wr_mas->end_piv = wr_mas->r_max;
>>   	mas_wr_end_piv(wr_mas);
>>   
>>   	if (!wr_mas->entry)
>> -- 
>> 2.20.1
>>