On Sun, Oct 06, 2024 at 08:53:28AM +0800, Herbert Xu wrote:
> On Sat, Oct 05, 2024 at 03:24:48PM -0700, Eric Biggers wrote:
> >
> > The tests are still failing on upstream:
> >
> > [ 0.343845] alg: self-tests for rfc4106(gcm(aes)) using rfc4106(gcm_base(ctr(aes-generic),ghash-generic)) failed (rc=-2)
>
> You're right. I only disabled the warnings at the point of
> allocation, the overall self-test warning is still there. Let
> me get rid of them too.
>
> > Besides the test failures, it looks like there's no longer any guarantee that
> > algorithms are actually available now that their module is loaded.
>
> That would indeed be a bug. But I haven't seen it in practice.
> Although the s390 folks were reporting some weird errors with
> dm-crypt, they have recently disappeared.
>
> If you do see an actual failure please report it and then I'll
> consider reverting it until it's fixed.
>
> > E.g. consider if someone does 'modprobe aesni-intel' and then immediately
> > creates a dm-crypt device. Now it sounds like the AES-NI algorithms might not
> > have finished being tested yet and the generic algorithms can be used instead,
> > resulting in a performance regression.
>
> That is not the case. After modprobe returns, the algorithm is
> guaranteed to have been registered. Yes it is untested, but that
> should not be a problem because a test larval will have been created
> and all users looking for that algorithm will be waiting on that
> test larval.
I'm not sure about that, since the code that looks up algorithms only looks for
algorithms that already have the CRYPTO_ALG_TESTED flag.
> > I understand that you want to try to fix the edge cases in "fallback" ciphers.
> > But "fallback" ciphers have always seemed like a bad design due to how they use
> > the crypto API recursively. I think the algorithms that use them should
> > generally be migrated off of them, e.g. as I did in commit f235bc11cc95
> > ("crypto: arm/aes-neonbs - go back to using aes-arm directly"). That fixed the
> > problem in aes-neonbs that seems to have triggered this work in the first place.
>
> Yes getting rid of fallbacks is nice, but this it not the reason why
> we're making self-test asynchronous. The primary issue with synchronous
> self-tests is the modprobe dead-lock.
That problem is caused by the use of fallback ciphers, though.
- Eric
