linux-kernel - Re: [PATCH] crypto: caam/qi

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_XiPLmSVs8PGTZD@gondor.apana.org.au>
Date: Wed, 9 Apr 2025 10:58:04 +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: Re: [PATCH] crypto: caam/qi - Fix drv_ctx refcount bug

On Tue, Apr 08, 2025 at 11:44:27AM -0400, Sean Anderson wrote:
>
> However, note that the following error is still present:
> 
> [    3.294978] alg: sig: sign test failed: invalid output

That just looks like a bug in the driver.  I'll leave it in the
caam maintainers' capable hands :)
 
> as well as another error on reboot (present before but I forgot to post it):
> 
> [   98.514208] ------------[ cut here ]------------
> [   98.518823] WARNING: CPU: 3 PID: 1 at crypto/algapi.c:464 crypto_unregister_alg (crypto/algapi.c:464 (discriminator 1)) 
> [   98.527100] Modules linked in:
> [   98.530153] CPU: 3 UID: 0 PID: 1 Comm: systemd-shutdow Tainted: G        W          6.14.0-seco+ #163 NONE
> [   98.539899] Tainted: [W]=WARN
> [   98.542859] Hardware name: LS1046A RDB Board (DT)
> [   98.547559] pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [   98.554520] pc : crypto_unregister_alg (crypto/algapi.c:464 (discriminator 1)) 
> [   98.559224] lr : crypto_unregister_alg (include/linux/atomic/atomic-arch-fallback.h:457 include/linux/atomic/atomic-instrumented.h:33 include/linux/refcount.h:136 crypto/algapi.c:464) 

Yes that's a long-standing design fault in the crypto_unregister
mechanism for drivers.

The unregister mechanism was designed for software crypto, so it
relied on module refcounts to prevent unregistering something
that's still in use.  For hardware that follows the device model
where things get unplugged at random, this obviously doesn't work.

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 :)

Cheers,
-- 
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