linux-kernel - [PATCH] crypto: api - Allow delayed algorithm destruction

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: <Z_XpfyPaoZ6Y8u6z@gondor.apana.org.au>
Date: Wed, 9 Apr 2025 11:29:03 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Horia Geantă <horia.geanta@....com>,
	Pankaj Gupta <pankaj.gupta@....com>,
	Gaurav Jain <gaurav.jain@....com>, linux-crypto@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	linux-kernel@...r.kernel.org,
	Valentin Ciocoi Radulescu <valentin.ciocoi@....com>
Subject: [PATCH] crypto: api - Allow delayed algorithm destruction

On Wed, Apr 09, 2025 at 10:58:04AM +0800, Herbert Xu wrote:
>
> What I'll do is make the crypto_unregister call wait for the users
> to go away.  That matches how the network device unregistration works
> and hopefully should solve this problem.  But keep your eyes for
> dead locks that used to plague netdev unregistration :)

That was a dumb idea.  All it would do is make the shutdown hang.
So here is a different tack.  Let the algorithms stick around,
by allocating them dynamically instead.  Then we could simply
kfree them when the user finally disappears (if ever).

Note to make this work, caam needs to be modified to allocate the
algorithms dynamically (kmemdup should work), and add a cra_destroy
function to kfree the memory.

---8<---
The current algorithm unregistration mechanism originated from
software crypto.  The code relies on module reference counts to
stop in-use algorithms from being unregistered.  Therefore if
the unregistration function is reached, it is assumed that the
module reference count has hit zero and thus the algorithm reference
count should be exactly 1.

This is completely broken for hardware devices, which can be
unplugged at random.

Fix this by allowing algorithms to be destroyed later if a destroy
callback is provided.

Reported-by: Sean Anderson <sean.anderson@...ux.dev>
Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index 5b8a4c787387..f368c0dc0d6d 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -481,10 +481,10 @@ void crypto_unregister_alg(struct crypto_alg *alg)
 	if (WARN(ret, "Algorithm %s is not registered", alg->cra_driver_name))
 		return;
 
-	if (WARN_ON(refcount_read(&alg->cra_refcnt) != 1))
-		return;
-
-	if (alg->cra_type && alg->cra_type->destroy)
+	if (alg->cra_destroy)
+		crypto_alg_put(alg);
+	else if (!WARN_ON(refcount_read(&alg->cra_refcnt) != 1) &&
+		 alg->cra_type && alg->cra_type->destroy)
 		alg->cra_type->destroy(alg);
 
 	crypto_remove_final(&list);
-- 
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