Re: [PATCH] crypto: testmgr - don't generate WARN for missing modules

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Aug 19, 2022 at 07:05:41PM +0800, Herbert Xu wrote:
> Robert Elliott <elliott@hpe.com> wrote:
> > This userspace command:
> >    modprobe tcrypt
> > or
> >    modprobe tcrypt mode=0
> > 
> > runs all the tcrypt test cases numbered <200 (i.e., all the
> > test cases calling tcrypt_test() and returning return values).
> > 
> > Tests are sparsely numbered from 0 to 1000. For example:
> >    modprobe tcrypt mode=12
> > tests sha512, and
> >    modprobe tcrypt mode=152
> > tests rfc4543(gcm(aes))) - AES-GCM as GMAC
> > 
> > The test manager generates WARNING crashdumps every time it attempts
> > a test using an algorithm that is not available (not built-in to the
> > kernel or available as a module):
> > 
> >    alg: skcipher: failed to allocate transform for ecb(arc4): -2
> >    ------------[ cut here ]-----------
> >    alg: self-tests for ecb(arc4) (ecb(arc4)) failed (rc=-2)
> >    WARNING: CPU: 9 PID: 4618 at crypto/testmgr.c:5777
> > alg_test+0x30b/0x510
> >    [50 more lines....]
> > 
> >    ---[ end trace 0000000000000000 ]---
> > 
> > If the kernel is compiled with CRYPTO_USER_API_ENABLE_OBSOLETE
> > disabled (the default), then these algorithms are not compiled into
> > the kernel or made into modules and trigger WARNINGs:
> >    arc4 tea xtea khazad anubis xeta seed
> > 
> > Additionally, any other algorithms that are not enabled in .config
> > will generate WARNINGs. In RHEL 9.0, for example, the default
> > selection of algorithms leads to 16 WARNING dumps.
> > 
> > One attempt to fix this was by modifying tcrypt_test() to check
> > crypto_has_alg() and immediately return 0 if crypto_has_alg() fails,
> > rather than proceed and return a non-zero error value that causes
> > the caller (alg_test() in crypto/testmgr.c) to invoke WARN().
> > That knocks out too many algorithms, though; some combinations
> > like ctr(des3_ede) would work.
> > 
> > Instead, change the condition on the WARN to ignore a return
> > value is ENOENT, which is the value returned when the algorithm
> > or combination of algorithms doesn't exist. Add a pr_warn to
> > communicate that information in case the WARN is skipped.
> > 
> > This approach allows algorithm tests to work that are combinations,
> > not provided by one driver, like ctr(blowfish).
> > 
> > Result - no more WARNINGs:
> > modprobe tcrypt
> > [  115.541765] tcrypt: testing md5
> > [  115.556415] tcrypt: testing sha1
> > [  115.570463] tcrypt: testing ecb(des)
> > [  115.585303] cryptomgr: alg: skcipher: failed to allocate transform for ecb(des): -2
> > [  115.593037] cryptomgr: alg: self-tests for ecb(des) using ecb(des) failed (rc=-2)
> > [  115.593038] tcrypt: testing cbc(des)
> > [  115.610641] cryptomgr: alg: skcipher: failed to allocate transform for cbc(des): -2
> > [  115.618359] cryptomgr: alg: self-tests for cbc(des) using cbc(des) failed (rc=-2)
> > ...
> > 
> > Signed-off-by: Robert Elliott <elliott@hpe.com>
> > ---
> > crypto/testmgr.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> 
> Patch applied.  Thanks.

I thought the conclusion from the discussion was that this should instead be
solved by a tcrypt change?  Either dropping the enumerative testing support from
tcrypt, or making tcrypt just try to allocate the algorithms (relying on the
registration-time self-tests) rather than call alg_test() directly.

- Eric