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

[Eric Biggers <ebiggers@kernel.org>]

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