netdev - RE: [v2 PATCH] rhashtable: Fix rhashtable_try

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:
 <SN6PR02MB41570F1F5F4F579B4E8F0CD3D41C2@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Fri, 10 Jan 2025 18:22:40 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
CC: Breno Leitao <leitao@...ian.org>, "saeedm@...dia.com" <saeedm@...dia.com>,
	"tariqt@...dia.com" <tariqt@...dia.com>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Graf <tgraf@...g.ch>, Tejun Heo <tj@...nel.org>, Hao Luo
	<haoluo@...gle.com>, Josh Don <joshdon@...gle.com>, Barret Rhoden
	<brho@...gle.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [v2 PATCH] rhashtable: Fix rhashtable_try_insert test

From: Herbert Xu <herbert@...dor.apana.org.au> Sent: Friday, January 10, 2025 9:24 AM
> 
> On Fri, Jan 10, 2025 at 04:59:28PM +0000, Michael Kelley wrote:
> >
> > This patch fixes the problem I saw with VMs in the Azure cloud.  Thanks!
> 
> Sorry, but the test on data is needed after all (it was just
> buggy).  Otherwise we will break rhlist.  So please test this
> patch instead.
> 
> ---8<---
> The test on whether rhashtable_insert_one did an insertion relies
> on the value returned by rhashtable_lookup_one.  Unfortunately that
> value is overwritten after rhashtable_insert_one returns.  Fix this
> by saving the old value.
> 
> Also simplify the test as only data == NULL matters.
> 
> Reported-by: Michael Kelley <mhklinux@...look.com>
> Fixes: e1d3422c95f0 ("rhashtable: Fix potential deadlock by moving schedule_work
> outside lock")
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
> 
> diff --git a/lib/rhashtable.c b/lib/rhashtable.c
> index bf956b85455a..e36b36f3146d 100644
> --- a/lib/rhashtable.c
> +++ b/lib/rhashtable.c
> @@ -611,17 +611,20 @@ static void *rhashtable_try_insert(struct rhashtable *ht,
> const void *key,
>  			new_tbl = rht_dereference_rcu(tbl->future_tbl, ht);
>  			data = ERR_PTR(-EAGAIN);
>  		} else {
> +			void *odata;
> +
>  			flags = rht_lock(tbl, bkt);
>  			data = rhashtable_lookup_one(ht, bkt, tbl,
>  						     hash, key, obj);
>  			new_tbl = rhashtable_insert_one(ht, bkt, tbl,
>  							hash, obj, data);
> +			odata = data;
>  			if (PTR_ERR(new_tbl) != -EEXIST)
>  				data = ERR_CAST(new_tbl);
> 
>  			rht_unlock(tbl, bkt, flags);
> 
> -			if (PTR_ERR(data) == -ENOENT && !new_tbl) {
> +			if (odata && !new_tbl) {
>  				atomic_inc(&ht->nelems);
>  				if (rht_grow_above_75(ht, tbl))
>  					schedule_work(&ht->run_work);

This patch passes my tests. I'm doing a narrow test to verify that
the boot failure when opening the Mellanox NIC is no longer occurring.
I also unloaded/reloaded the mlx5 driver a couple of times. For good
measure, I then did a full Linux kernel build, and all is good. My testing
does not broadly verify correct operation of rhashtable except as it
gets exercised implicitly by these basic tests.

Michael