Re: [v5 PATCH] crypto: api - fix unexpectedly getting generic implementation

[Eric Biggers <ebiggers@kernel.org>]

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@google.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Looks good, thanks.

Reviewed-by: Eric Biggers <ebiggers@google.com>

- Eric