Re: [PATCH] maple_tree: Fix potential out-of-bounds access in mas_wr_end_piv()

[Peng Zhang <perlyzhang@gmail.com>]

在 2023/5/6 09:14, Liam R. Howlett 写道:
> * Peng Zhang <zhangpeng.00@bytedance.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@oracle.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@bytedance.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
>>