Re: [PATCH 1/2] maple_tree: Disable mas_wr_append() when other readers are possible

[Liam R. Howlett]

I forgot the fixes tag, I will include that in v2.

* Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> [230817 15:15]:
> The current implementation of append may cause duplicate data and/or
> incorrect ranges to be returned to a reader during an update.  Although
> this has not been reported or seen, disable the append write operation
> while the tree is in rcu mode out of an abundance of caution.
> 
> During the analysis of the mas_next_slot() the following was
> artificially created by separating the writer and reader code:
> 
> Writer:                                 reader:
> mas_wr_append
>     set end pivot
>     updates end metata
>     Detects write to last slot
>     last slot write is to start of slot
>     store current contents in slot
>     overwrite old end pivot
>                                         mas_next_slot():
>                                                 read end metadata
>                                                 read old end pivot
>                                                 return with incorrect range
>     store new value
> 
> Alternatively:
> 
> Writer:                                 reader:
> mas_wr_append
>     set end pivot
>     updates end metata
>     Detects write to last slot
>     last lost write to end of slot
>     store value
>                                         mas_next_slot():
>                                                 read end metadata
>                                                 read old end pivot
>                                                 read new end pivot
>                                                 return with incorrect range
>     set old end pivot
> 
> There may be other accesses that are not safe since we are now updating
> both metadata and pointers, so disabling append if there could be rcu
> readers is the safest action.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> ---
>  lib/maple_tree.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index ffb9d15bd815..05d5db255c39 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4107,6 +4107,10 @@ static inline unsigned char mas_wr_new_end(struct ma_wr_state *wr_mas)
>   * mas_wr_append: Attempt to append
>   * @wr_mas: the maple write state
>   *
> + * This is currently unsafe in rcu mode since the end of the node may be cached
> + * by readers while the node contents may be updated which could result in
> + * inaccurate information.
> + *
>   * Return: True if appended, false otherwise
>   */
>  static inline bool mas_wr_append(struct ma_wr_state *wr_mas,
> @@ -4116,6 +4120,9 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas,
>  	struct ma_state *mas = wr_mas->mas;
>  	unsigned char node_pivots = mt_pivots[wr_mas->type];
>  
> +	if (mt_in_rcu(mas->tree))
> +		return false;
> +
>  	if (mas->offset != wr_mas->node_end)
>  		return false;
>  
> -- 
> 2.39.2
>