linux-kernel - Re: [PATCH 1/2] maple_tree: check for MA_STATE_BULK on setting wr

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: <kpz4acqtffzx2vabwdzykhxdypfaght57frbv6kl4ropkgih7q@afml7mwgftj6>
Date: Mon, 14 Oct 2024 22:03:46 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Sidhartha Kumar <sidhartha.kumar@...cle.com>
Cc: linux-kernel@...r.kernel.org, maple-tree@...ts.infradead.org,
        linux-mm@...ck.org, akpm@...ux-foundation.org, willy@...radead.org,
        richard.weiyang@...il.com
Subject: Re: [PATCH 1/2] maple_tree: check for MA_STATE_BULK on setting
 wr_rebalance

* Sidhartha Kumar <sidhartha.kumar@...cle.com> [241011 17:44]:
> It is possible for a bulk operation (MA_STATE_BULK is set) to enter the
> new_end < mt_min_slots[type] case and set wr_rebalance as a store type.
> This is incorrect as bulk stores do not rebalance per write, but rather
> after the all of the writes are done through the mas_bulk_rebalance()
> path. Therefore, add a check to make sure MA_STATE_BULK is not set
> before we return wr_rebalance as the store type.
> 
> Also add a test to make sure wr_rebalance is never the store type when
> doing bulk operations via mas_expected_entries()
> 
> Fixes: 5d659bbb52a2 ("maple_tree: introduce mas_wr_store_type()")
> Suggested-by: Liam Howlett <liam.howlett@...cle.com>
> Signed-off-by: Sidhartha <sidhartha.kumar@...cle.com>

Reviewed-by: Liam Howlett <liam.howlett@...cle.com>

> ---
> 
> Hi Andrew, this commit is a hotfix for this rc however it has no userspace
> effects as there are no users of the bulk instertion mode.
> 
>  lib/maple_tree.c                 |  2 +-
>  tools/testing/radix-tree/maple.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 4b423330d83c..b3b1d4b8126b 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4228,7 +4228,7 @@ static inline void mas_wr_store_type(struct ma_wr_state *wr_mas)
>  
>  	/* Potential spanning rebalance collapsing a node */
>  	if (new_end < mt_min_slots[wr_mas->type]) {
> -		if (!mte_is_root(mas->node)) {
> +		if (!mte_is_root(mas->node) && !(mas->mas_flags & MA_STATE_BULK)) {
>  			mas->store_type = wr_rebalance;
>  			return;
>  		}
> diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c
> index 5fde09999be4..551ae6898c1d 100644
> --- a/tools/testing/radix-tree/maple.c
> +++ b/tools/testing/radix-tree/maple.c
> @@ -36317,6 +36317,28 @@ static inline int check_vma_modification(struct maple_tree *mt)
>  	return 0;
>  }
>  
> +/*
> + * test to check that bulk stores do not use wr_rebalance as the store
> + * type.
> + */
> +static inline void check_bulk_rebalance(struct maple_tree *mt)
> +{
> +	MA_STATE(mas, mt, ULONG_MAX, ULONG_MAX);
> +	int max = 10;
> +
> +	build_full_tree(mt, 0, 2);
> +
> +	/* erase every entry in the tree */
> +	do {
> +		/* set up bulk store mode */
> +		mas_expected_entries(&mas, max);
> +		mas_erase(&mas);
> +		MT_BUG_ON(mt, mas.store_type == wr_rebalance);
> +	} while (mas_prev(&mas, 0) != NULL);
> +
> +	mas_destroy(&mas);
> +}
> +
>  void farmer_tests(void)
>  {
>  	struct maple_node *node;
> @@ -36328,6 +36350,10 @@ void farmer_tests(void)
>  	check_vma_modification(&tree);
>  	mtree_destroy(&tree);
>  
> +	mt_init(&tree);
> +	check_bulk_rebalance(&tree);
> +	mtree_destroy(&tree);
> +
>  	tree.ma_root = xa_mk_value(0);
>  	mt_dump(&tree, mt_dump_dec);
>  
> -- 
> 2.43.0
>