[linux-next:master] [crypto] 698de82278: WARNING:at_crypto/testmgr.c:#alg_test

[linux-next:master] [crypto] 698de82278: WARNING:at_crypto/testmgr.c:#alg_test

[kernel test robot]

Hello,

kernel test robot noticed "WARNING:at_crypto/testmgr.c:#alg_test" on:

commit: 698de822780fbae79b11e5d749863c1aa64a0a55 ("crypto: testmgr - make it easier to enable the full set of tests")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master bdd609656ff5573db9ba1d26496a528bdd297cf2]

in testcase: boot

config: i386-randconfig-002-20250513
compiler: clang-20
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+---------------------------------------+------------+------------+
|                                       | 40b9969796 | 698de82278 |
+---------------------------------------+------------+------------+
| WARNING:at_crypto/testmgr.c:#alg_test | 0          | 12         |
| EIP:alg_test                          | 0          | 12         |
+---------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202505151503.d8a6cf10-lkp@intel.com


[   16.077514][  T327] ------------[ cut here ]------------
[   16.079451][  T327] alg: self-tests for lrw(twofish) using lrw(ecb(twofish-asm)) failed (rc=-22)
[ 16.079491][ T327] WARNING: CPU: 0 PID: 327 at crypto/testmgr.c:5811 alg_test (kbuild/obj/consumer/i386-randconfig-002-20250513/crypto/testmgr.c:5809) 
[   16.081769][  T327] Modules linked in:
[   16.082303][  T327] CPU: 0 UID: 0 PID: 327 Comm: cryptomgr_test Tainted: G S                  6.15.0-rc5-00343-g698de822780f #1 PREEMPTLAZY  36427eb4e2823c0186d3f8e85f01fa19b8e4ee5b
[   16.084257][  T327] Tainted: [S]=CPU_OUT_OF_SPEC
[   16.084897][  T327] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 16.086245][ T327] EIP: alg_test (kbuild/obj/consumer/i386-randconfig-002-20250513/crypto/testmgr.c:5809) 
[ 16.086824][ T327] Code: 89 c7 e8 3d a7 79 ff 83 c4 10 b8 fe ff ff ff 83 ff fe 0f 84 54 ff ff ff 89 f9 51 56 53 68 a4 c9 6f 56 e8 f5 90 79 ff 83 c4 10 <0f> 0b 89 f8 e9 39 ff ff ff 8d 83 74 0d ec 55 8b 78 10 8b 58 14 89
All code
========
   0:	89 c7                	mov    %eax,%edi
   2:	e8 3d a7 79 ff       	call   0xffffffffff79a744
   7:	83 c4 10             	add    $0x10,%esp
   a:	b8 fe ff ff ff       	mov    $0xfffffffe,%eax
   f:	83 ff fe             	cmp    $0xfffffffe,%edi
  12:	0f 84 54 ff ff ff    	je     0xffffffffffffff6c
  18:	89 f9                	mov    %edi,%ecx
  1a:	51                   	push   %rcx
  1b:	56                   	push   %rsi
  1c:	53                   	push   %rbx
  1d:	68 a4 c9 6f 56       	push   $0x566fc9a4
  22:	e8 f5 90 79 ff       	call   0xffffffffff79911c
  27:	83 c4 10             	add    $0x10,%esp
  2a:*	0f 0b                	ud2		<-- trapping instruction
  2c:	89 f8                	mov    %edi,%eax
  2e:	e9 39 ff ff ff       	jmp    0xffffffffffffff6c
  33:	8d 83 74 0d ec 55    	lea    0x55ec0d74(%rbx),%eax
  39:	8b 78 10             	mov    0x10(%rax),%edi
  3c:	8b 58 14             	mov    0x14(%rax),%ebx
  3f:	89                   	.byte 0x89

Code starting with the faulting instruction
===========================================
   0:	0f 0b                	ud2
   2:	89 f8                	mov    %edi,%eax
   4:	e9 39 ff ff ff       	jmp    0xffffffffffffff42
   9:	8d 83 74 0d ec 55    	lea    0x55ec0d74(%rbx),%eax
   f:	8b 78 10             	mov    0x10(%rax),%edi
  12:	8b 58 14             	mov    0x14(%rax),%ebx
  15:	89                   	.byte 0x89
[   16.089318][  T327] EAX: 0000004c EBX: bad01080 ECX: 00000000 EDX: 00000000
[   16.090193][  T327] ESI: bad01000 EDI: ffffffea EBP: ba399f54 ESP: ba399eb0
[   16.091085][  T327] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010282
[   16.092098][  T327] CR0: 80050033 CR2: ffd9a000 CR3: 16f05000 CR4: 00040690
[   16.092992][  T327] Call Trace:
[ 16.093406][ T327] ? lock_acquire (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/locking/lockdep.c:5866) 
[ 16.094001][ T327] ? __kthread_parkme (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/kthread.c:290) 
[ 16.094662][ T327] ? trace_hardirqs_on (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/trace/trace_preemptirq.c:80) 
[ 16.095313][ T327] cryptomgr_test (kbuild/obj/consumer/i386-randconfig-002-20250513/crypto/algboss.c:179) 
[ 16.095890][ T327] kthread (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/kthread.c:466) 
[ 16.096412][ T327] ? crypto_alg_put (kbuild/obj/consumer/i386-randconfig-002-20250513/crypto/algboss.c:174) 
[ 16.097000][ T327] ? kthread_blkcg (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/kthread.c:413) 
[ 16.097584][ T327] ? kthread_blkcg (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/kthread.c:413) 
[ 16.098186][ T327] ret_from_fork (kbuild/obj/consumer/i386-randconfig-002-20250513/arch/x86/kernel/process.c:159) 
[ 16.098745][ T327] ret_from_fork_asm (kbuild/obj/consumer/i386-randconfig-002-20250513/arch/x86/entry/entry_32.S:737) 
[ 16.099349][ T327] entry_INT80_32 (kbuild/obj/consumer/i386-randconfig-002-20250513/arch/x86/entry/entry_32.S:945) 
[   16.099958][  T327] irq event stamp: 30685
[ 16.100499][ T327] hardirqs last enabled at (30693): __console_unlock (kbuild/obj/consumer/i386-randconfig-002-20250513/arch/x86/include/asm/irqflags.h:19 kbuild/obj/consumer/i386-randconfig-002-20250513/arch/x86/include/asm/irqflags.h:109 kbuild/obj/consumer/i386-randconfig-002-20250513/arch/x86/include/asm/irqflags.h:151 kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/printk/printk.c:344 kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/printk/printk.c:2885) 
[ 16.101680][ T327] hardirqs last disabled at (30702): __console_unlock (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/printk/printk.c:342) 
[ 16.102859][ T327] softirqs last enabled at (22314): __do_softirq (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/softirq.c:614) 
[ 16.103928][ T327] softirqs last disabled at (22303): __do_softirq (kbuild/obj/consumer/i386-randconfig-002-20250513/kernel/softirq.c:614) 
[   16.105018][  T327] ---[ end trace 0000000000000000 ]---
[   16.105779][    T1] alg: skcipher: failed to allocate transform for lrw(twofish): -80
[   16.106818][    T1] alg: self-tests for lrw(twofish) using lrw(twofish) failed (rc=-80)


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250515/202505151503.d8a6cf10-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH] crypto: lrw - Only add ecb if it is not already there

[Herbert Xu]

On Thu, May 15, 2025 at 03:41:05PM +0800, kernel test robot wrote:
>
> [   16.077514][  T327] ------------[ cut here ]------------
> [   16.079451][  T327] alg: self-tests for lrw(twofish) using lrw(ecb(twofish-asm)) failed (rc=-22)

The crucial line actually got cut off:

alg: skcipher: error allocating lrw(ecb(twofish-generic)) (generic impl of lrw(twofish)): -22

The bug is in lrw, which unconditionally adds ecb() around its
parameter, so we end up with ecb(ecb(twofish-generic)), which is
then correctly rejected by the ecb template when it tries to
create the inner ecb(twofish-generic) as a simple cipher.

---8<---
Only add ecb to the cipher name if it isn't already ecb.

Also use memcmp instead of strncmp since these strings are all
stored in an array of length CRYPTO_MAX_ALG_NAME.

Fixes: 700cb3f5fe75 ("crypto: lrw - Convert to skcipher")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202505151503.d8a6cf10-lkp@intel.com
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/lrw.c b/crypto/lrw.c
index e7f0368f8c97..dd403b800513 100644
--- a/crypto/lrw.c
+++ b/crypto/lrw.c
@@ -322,7 +322,7 @@ static int lrw_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = crypto_grab_skcipher(spawn, skcipher_crypto_instance(inst),
 				   cipher_name, 0, mask);
-	if (err == -ENOENT) {
+	if (err == -ENOENT && memcmp(cipher_name, "ecb(", 4)) {
 		err = -ENAMETOOLONG;
 		if (snprintf(ecb_name, CRYPTO_MAX_ALG_NAME, "ecb(%s)",
 			     cipher_name) >= CRYPTO_MAX_ALG_NAME)
@@ -356,7 +356,7 @@ static int lrw_create(struct crypto_template *tmpl, struct rtattr **tb)
 	/* Alas we screwed up the naming so we have to mangle the
 	 * cipher name.
 	 */
-	if (!strncmp(cipher_name, "ecb(", 4)) {
+	if (!memcmp(cipher_name, "ecb(", 4)) {
 		int len;
 
 		len = strscpy(ecb_name, cipher_name + 4, sizeof(ecb_name));
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: lrw - Only add ecb if it is not already there

[Eric Biggers]

On Thu, May 15, 2025 at 04:28:08PM +0800, Herbert Xu wrote:
> On Thu, May 15, 2025 at 03:41:05PM +0800, kernel test robot wrote:
> >
> > [   16.077514][  T327] ------------[ cut here ]------------
> > [   16.079451][  T327] alg: self-tests for lrw(twofish) using lrw(ecb(twofish-asm)) failed (rc=-22)
> 
> The crucial line actually got cut off:
> 
> alg: skcipher: error allocating lrw(ecb(twofish-generic)) (generic impl of lrw(twofish)): -22
> 
> The bug is in lrw, which unconditionally adds ecb() around its
> parameter, so we end up with ecb(ecb(twofish-generic)), which is
> then correctly rejected by the ecb template when it tries to
> create the inner ecb(twofish-generic) as a simple cipher.

Please include this explanation in the commit message itself.

Also adding ecb wasn't unconditional, but only if it wasn't found with one ecb.

> Fixes: 700cb3f5fe75 ("crypto: lrw - Convert to skcipher")

It didn't actually make a difference until 795f85fca229 ("crypto: algboss - Pass
instance creation error up") though, right?  Before then, if "ecb(...)" gave
ENOENT then "ecb(ecb(...))" gave ENOENT too.

As I said in
https://lore.kernel.org/linux-crypto/20240924222839.GC1585@sol.localdomain/,
that commit (which had no explanation) just seems wrong.  We should have simply
stuck with ENOENT.

But as usual my concern just got ignored and it got pushed out anyway.

- Eric

Re: [PATCH] crypto: lrw - Only add ecb if it is not already there

[Herbert Xu]

On Thu, May 15, 2025 at 01:04:55PM -0700, Eric Biggers wrote:
>
> It didn't actually make a difference until 795f85fca229 ("crypto: algboss - Pass
> instance creation error up") though, right?  Before then, if "ecb(...)" gave
> ENOENT then "ecb(ecb(...))" gave ENOENT too.

I would consider this a success, it actually caught lrw doing
something silly by trying to allocate "ecb(ecb(XXX))".  Sure we
can sweep all of this under the rug with ENOENT but it might end
up blowing up in a much worse place.

Also I don't think it's true that ecb(ecb(XXX)) will always give an
ENOENT if ecb(XXX) gives an ENOENT.  The error is actually originating
from the lskcipher code, which tries to construct ecb from either an
lskcipher, or a simple cipher.  It is the latter that triggers the
EINVAL error since the ecb template cannot create a simple cipher.

> As I said in
> https://lore.kernel.org/linux-crypto/20240924222839.GC1585@sol.localdomain/,
> that commit (which had no explanation) just seems wrong.  We should have simply
> stuck with ENOENT.
> 
> But as usual my concern just got ignored and it got pushed out anyway.

I don't ignore all your concerns, just the ones that I think are
unwarranted :)

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: lrw - Only add ecb if it is not already there

[Eric Biggers]

On Fri, May 16, 2025 at 05:15:20PM +0800, Herbert Xu wrote:
> On Thu, May 15, 2025 at 01:04:55PM -0700, Eric Biggers wrote:
> >
> > It didn't actually make a difference until 795f85fca229 ("crypto: algboss - Pass
> > instance creation error up") though, right?  Before then, if "ecb(...)" gave
> > ENOENT then "ecb(ecb(...))" gave ENOENT too.
> 
> I would consider this a success, it actually caught lrw doing
> something silly by trying to allocate "ecb(ecb(XXX))".  Sure we
> can sweep all of this under the rug with ENOENT but it might end
> up blowing up in a much worse place.
> 
> Also I don't think it's true that ecb(ecb(XXX)) will always give an
> ENOENT if ecb(XXX) gives an ENOENT.  The error is actually originating
> from the lskcipher code, which tries to construct ecb from either an
> lskcipher, or a simple cipher.  It is the latter that triggers the
> EINVAL error since the ecb template cannot create a simple cipher.
> 
> > As I said in
> > https://lore.kernel.org/linux-crypto/20240924222839.GC1585@sol.localdomain/,
> > that commit (which had no explanation) just seems wrong.  We should have simply
> > stuck with ENOENT.
> > 
> > But as usual my concern just got ignored and it got pushed out anyway.
> 
> I don't ignore all your concerns, just the ones that I think are
> unwarranted :)

I don't think this one is unwarranted.  We should keep the error codes simple
and just use ENOENT for algorithms that don't exist.  This also affected AF_ALG.

- Eric