Re: [PATCH] crypto: algapi - Remove test larvals to fix error paths

[Herbert Xu <herbert@gondor.apana.org.au>]

On Wed, Jan 26, 2022 at 03:53:22PM +0100, Philipp Zabel wrote:
> If crypto_unregister_alg() is called with an algorithm that still has a
> pending test larval, the algorithm will have a reference count of 2 and
> crypto_unregister_alg() will trigger a BUG(). This can happen during
> cleanup if the error path is taken for a built-in algorithm, before
> crypto_algapi_init() was called.
> 
> Kill test larvals for untested algorithms during removal to fix the
> reference count.
> 
> Fixes: adad556efcdd ("crypto: api - Fix built-in testing dependency failures")
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  crypto/algapi.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

Yes this is definitely a bug.

However, I think simply fixing this for test larvals is not enough.
After all, another thread could come through the middle and take
a reference to our newly minted algorithm before a subsequent
algorithm registration failure and thus would trigger exactly
the same crash.

So we need something a bit more general.  Previously we relied
on module reference counts to stop the unregistering of live
algorithms.  But that is clearly not of any use in this case.

It also fails for hardware devices than can be unplugged.

One solution would be to simply allow the unregistration to
continue and only free the data structures after all references
have gone away.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt