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

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] [day] [month] [year] [list]

Message-ID: <20230817235654.z7e7fi2gofnsxniw@revolver>
Date:   Thu, 17 Aug 2023 19:56:54 -0400
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     maple-tree@...ts.infradead.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/2] maple_tree: Disable mas_wr_append() when other
 readers are possible

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

* Liam R. Howlett <Liam.Howlett@...cle.com> [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@...r.kernel.org
> Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> ---
>  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
>