* [PATCH v2] crypto: nx: fix nx_crypto_ctx_exit argument
2026-05-22 18:44 ` Eric Biggers
@ 2026-05-23 4:08 ` Sam James
2026-05-23 6:30 ` [PATCH] " Simon Richter
1 sibling, 0 replies; 4+ messages in thread
From: Sam James @ 2026-05-23 4:08 UTC (permalink / raw)
To: Breno Leitão, Nayna Jain, Paulo Flabiano Smorigo,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Herbert Xu, David S. Miller,
Ard Biesheuvel, Eric Biggers
Cc: Sam James, Eric Biggers, stable, Calvin Buckley, Brad Spengler,
linux-crypto, linuxppc-dev, linux-kernel
nx_crypto_ctx_shash_exit calls nx_crypto_ctx_exit with crypto_shash_ctx(...)
but crypto_shash_ctx gives a nx_crypto_ctx *, not a crypto_tfm *.
Fix the type in nx_crypto_ctx_exit and drop the bogus crypto_tfm_ctx
call.
This fixes the following oops:
BUG: Unable to handle kernel data access at 0xc0403effffffffc8
Faulting instruction address: 0xc000000000396cb4
Oops: Kernel access of bad area, sig: 11 [#15]
Call Trace:
nx_crypto_ctx_shash_exit+0x24/0x60
crypto_shash_exit_tfm+0x28/0x40
crypto_destroy_tfm+0x98/0x140
crypto_exit_ahash_using_shash+0x20/0x40
crypto_destroy_tfm+0x98/0x140
hash_release+0x1c/0x30
alg_sock_destruct+0x38/0x60
__sk_destruct+0x48/0x2b0
af_alg_release+0x58/0xb0
__sock_release+0x68/0x150
sock_close+0x20/0x40
__fput+0x110/0x3a0
sys_close+0x48/0xa0
system_call_exception+0x140/0x2d0
system_call_common+0xf4/0x258
.. which came from hardlink(1) opportunistically using AF_ALG.
The same problem exists with nx_crypto_ctx_skcipher_exit getting a context
it wasn't expecting, but apparently nobody hit that for years.
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: stable@vger.kernel.org
Fixes: bfd9efddf990 ("crypto: nx - convert AES-ECB to skcipher API")
Fixes: 9420e628e7d8 ("crypto: nx - Use API partial block handling")
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Reported-by: Calvin Buckley <calvin@cmpct.info>
Tested-by: Calvin Buckley <calvin@cmpct.info>
Suggested-by: Brad Spengler <brad.spengler@opensrcsec.com>
Signed-off-by: Sam James <sam@gentoo.org>
---
v2: Add stable cc, fix doc for tfm param.
drivers/crypto/nx/nx.c | 6 ++----
drivers/crypto/nx/nx.h | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index 78135fb13f5c..f4bc947086f8 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -714,15 +714,13 @@ int nx_crypto_ctx_aes_xcbc_init(struct crypto_shash *tfm)
/**
* nx_crypto_ctx_exit - destroy a crypto api context
*
- * @tfm: the crypto transform pointer for the context
+ * @tfm: the crypto api context
*
* As crypto API contexts are destroyed, this exit hook is called to free the
* memory associated with it.
*/
-void nx_crypto_ctx_exit(struct crypto_tfm *tfm)
+void nx_crypto_ctx_exit(struct nx_crypto_ctx *nx_ctx)
{
- struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(tfm);
-
kfree_sensitive(nx_ctx->kmem);
nx_ctx->csbcpb = NULL;
nx_ctx->csbcpb_aead = NULL;
diff --git a/drivers/crypto/nx/nx.h b/drivers/crypto/nx/nx.h
index 36974f08490a..6dfabfbf8192 100644
--- a/drivers/crypto/nx/nx.h
+++ b/drivers/crypto/nx/nx.h
@@ -153,7 +153,7 @@ int nx_crypto_ctx_aes_ctr_init(struct crypto_skcipher *tfm);
int nx_crypto_ctx_aes_cbc_init(struct crypto_skcipher *tfm);
int nx_crypto_ctx_aes_ecb_init(struct crypto_skcipher *tfm);
int nx_crypto_ctx_sha_init(struct crypto_shash *tfm);
-void nx_crypto_ctx_exit(struct crypto_tfm *tfm);
+void nx_crypto_ctx_exit(struct nx_crypto_ctx *nx_ctx);
void nx_crypto_ctx_skcipher_exit(struct crypto_skcipher *tfm);
void nx_crypto_ctx_aead_exit(struct crypto_aead *tfm);
void nx_crypto_ctx_shash_exit(struct crypto_shash *tfm);
base-commit: 758c807bb943138f887d42d986b645e12446ba9c
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] crypto: nx: fix nx_crypto_ctx_exit argument
2026-05-22 18:44 ` Eric Biggers
2026-05-23 4:08 ` [PATCH v2] " Sam James
@ 2026-05-23 6:30 ` Simon Richter
1 sibling, 0 replies; 4+ messages in thread
From: Simon Richter @ 2026-05-23 6:30 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-crypto, linuxppc-dev
[-- Attachment #1.1: Type: text/plain, Size: 3739 bytes --]
Hi,
On 5/23/26 03:44, Eric Biggers wrote:
> Otherwise this looks good. Really there's a good chance this driver is
> no longer useful (if it ever was) and should just be deleted, but that
> would be a separate effort.
I happen to have one (well, two) of these, so this is relevant to my
interests.
tl;dr: the crypto drivers are most likely unused, the hardware is great,
but the crypto subsystem cannot use it efficiently.
Below drivers/crypto/nx, there are three drivers in a trenchcoat:
- an NX crypto driver that is not endian safe, can therefore only be
used on big endian systems, and that implements a bunch of AES modes
plus SHA256/SHA512, all of them synchronous.
- an scomp driver with an IBM specific compression algorithm
- a gzip driver that does not integrate with the crypto subsystem and
provides its own userspace interface.
The "big endian only" thing is a massive restriction, this is how IBM
separates enterprise and hobbyist customers, so if there are users of
this module, then they both have enterprise support contracts.
The gzip mode is really useful, with 4 GB of random data I get
$ time ./nx_gzip test.bin
real 0m2.989s
user 0m1.317s
sys 0m1.665s
$ time gzip -9k test.bin
real 2m57.468s
user 2m55.325s
sys 0m1.682s
so 3 GB/s vs 22 MB/s. Even if I had a workload where I could use all the
CPU cores in parallel, offloading is still faster, 120W cheaper and
leaves the CPU free as a bonus, so I think that's a no-brainer.
The "842" compression is mainly designed to be fast, the marketing
material claims > 25 GB/s, which makes sense, this unit sits on a 128
bit wide bus clocked at 2 GHz, and the algorithm is designed around
that. On the other hand it is fairly niche.
I couldn't find numbers for the AES and SHA units, I'd expect them to be
in the same ballpark, but I cannot measure them easily. CPU is ~500 MB/s
for SHA1 and SHA512, ~300 MB/s for SHA256, that should be easy to beat
(even a primitive 2-way SHA256 would be at 4 GB/s, and I doubt IBM left
it at that).
POWER11 introduces new opcodes, which will shake things up, but these
machines are on a fairly long replacement cycle.
The main problem with getting the advertised performance is feeding
requests fast enough. Large requests are easy, but the optimum strategy
for feeding small requests is just to start submitting, poll old
requests for completion inbetween, and start requesting interrupts only
if nothing is complete and it looks like the unit will be busy for a while.
That's not what is currently implemented, and I doubt it could be
implemented with the current kernel interfaces, so getting decent
performance inside the kernel would require some redesign.
I suppose that also explains the synchronous implementation: we are
submitting the request and polling for completion, so overhead is fairly
minimal and should break even at a few hundred bytes, but obviously that
is not the ideal way to run this thing.
The endianness issues are trivial to fix (really just needs a sprinkle
of cpu_to_beXX/beXX_to_cpu when putting the job control blocks together,
like nx-842 does); if you have a definition of what you would consider a
"real world" workload for AES I could run that to gather some numbers.
So far however, no one bothered fixing this, and I'm pretty meh about it
myself since I don't have SHA/AES workloads in the kernel, only in
userspace.
Other than that, if you decide to remove the driver from the crypto
subsystem, then nx-gzip should be kept (and probably moved somewhere
else), because it is not a crypto driver, it just shares a bunch of
headers with them.
Simon
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread