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

Peng Zhang
2023-05-06T01:48:26Z

在 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
>>