> -----Original Message----- > From: Eric Biggers <ebiggers@xxxxxxxxxx> > Sent: Friday, August 19, 2022 6:15 PM > To: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> > Cc: Elliott, Robert (Servers) <elliott@xxxxxxx>; > tim.c.chen@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; linux- > crypto@xxxxxxxxxxxxxxx; Kani, Toshi <toshi.kani@xxxxxxx>; Wright, Randy > (HPE Servers Linux) <rwright@xxxxxxx> > Subject: Re: [PATCH] crypto: testmgr - don't generate WARN for missing > modules > > On Fri, Aug 19, 2022 at 07:05:41PM +0800, Herbert Xu wrote: > > Robert Elliott <elliott@xxxxxxx> 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@xxxxxxx> > > > --- > > > 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 Per Stephan, it sounds like this was a hacky way to get some/most of the modules loaded. It'd be good if there was a way to run all registered tests on all available modules, not just the ones that someone remembered to put in tcrypt.c. I do worry this WARN() isn't really helpful even for real self-test failures - it's dumping the call trace to alg_test(), not the trace to whatever crypto function alg_test called that is failing. With Linus always expressing concern with too many BUG and WARN calls, it might be better as just pr_warn() or pr_err().
