netdev - Re: [PATCH] rhashtable: Fix potential deadlock by moving schedule

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: <Z4DoFYQ3ytB-wS3-@gondor.apana.org.au>
Date: Fri, 10 Jan 2025 17:27:49 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Breno Leitao <leitao@...ian.org>
Cc: Michael Kelley <mhklinux@...look.com>,
	"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: [PATCH] rhashtable: Fix potential deadlock by moving
 schedule_work outside lock

On Thu, Jan 09, 2025 at 02:15:17AM -0800, Breno Leitao wrote:
>
> I would suggest we revert this patch until we investigate further. I'll
> prepare and send a revert patch shortly.

Sorry, I think it was my addition that broke things.  The condition
for checking whether an entry is inserted is incorrect, thus resulting
in an underflow of the number of entries after entry removal.

Please test this patch:

---8<---
The function rhashtable_insert_one only returns NULL iff the
insertion was successful, so that alone should be tested before
increment nelems.  Testing the variable data is redundant, and
buggy because we will have overwritten the original value of data
by this point.

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..e196b6f0e35a 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -621,7 +621,7 @@ static void *rhashtable_try_insert(struct rhashtable *ht, const void *key,
 
 			rht_unlock(tbl, bkt, flags);
 
-			if (PTR_ERR(data) == -ENOENT && !new_tbl) {
+			if (!new_tbl) {
 				atomic_inc(&ht->nelems);
 				if (rht_grow_above_75(ht, tbl))
 					schedule_work(&ht->run_work);
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt