On Wed, Dec 11, 2019 at 10:50:11AM +0800, Herbert Xu wrote:
> On Tue, Dec 10, 2019 at 06:26:13PM -0800, Eric Biggers wrote:
> >
> > Sorry, I didn't notice you had already sent another patch for this. I think
> > this patch is okay, except that it's broken because it doesn't actually do
> > anything with the 'r' variable in crypto_alg_tested(). I suggest just removing
>
> Oops.
>
> > that variable and doing:
> >
> > if (best && crypto_mod_get(alg))
> > larval->adult = alg;
> > else
> > larval->adult = ERR_PTR(-EAGAIN);
>
> OK I have made this change.
>
> > Also, it would be nice to also add a function comment for crypto_alg_tested(),
> > like I had in my original patch. It's hard to understand this code.
>
> Your original comments no longer apply but if you wish to make
> another patch to add more comments that would certainly be welcome.
>
> Thanks,
>
> ---8<---
> When CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y, the first lookup of an
> algorithm that needs to be instantiated using a template will always get
> the generic implementation, even when an accelerated one is available.
>
> This happens because the extra self-tests for the accelerated
> implementation allocate the generic implementation for comparison
> purposes, and then crypto_alg_tested() for the generic implementation
> "fulfills" the original request (i.e. sets crypto_larval::adult).
>
> This patch fixes this by only fulfilling the original request if
> we are currently the best outstanding larval as judged by the
> priority. If we're not the best then we will ask all waiters on
> that larval request to retry the lookup.
>
> Note that this patch introduces a behaviour change when the module
> providing the new algorithm is unregistered during the process.
> Previously we would have failed with ENOENT, after the patch we
> will instead redo the lookup.
>
> Fixes: 9a8a6b3f0950 ("crypto: testmgr - fuzz hashes against...")
> Fixes: d435e10e67be ("crypto: testmgr - fuzz skciphers against...")
> Fixes: 40153b10d91c ("crypto: testmgr - fuzz AEADs against...")
> Reported-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Looks good, thanks.
Reviewed-by: Eric Biggers <ebiggers@xxxxxxxxxx>
- Eric
