Re: [PATCH] crypto: api - check for ERR pointers in crypto_destroy_tfm()

[Eric Biggers <ebiggers@kernel.org>]

On Sun, Feb 28, 2021 at 01:28:24PM +0100, Ard Biesheuvel wrote:
> Given that crypto_alloc_tfm() may return ERR pointers, and to avoid
> crashes on obscure error paths where such pointers are presented to
> crypto_destroy_tfm() (such as [0]), add an ERR_PTR check there
> before dereferencing the second argument as a struct crypto_tfm
> pointer.
> 
> [0] https://lore.kernel.org/linux-crypto/000000000000de949705bc59e0f6@google.com/
> 
> Reported-by: syzbot+12cf5fbfdeba210a89dd@syzkaller.appspotmail.com
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  crypto/api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/crypto/api.c b/crypto/api.c
> index ed08cbd5b9d3..c4eda56cff89 100644
> --- a/crypto/api.c
> +++ b/crypto/api.c
> @@ -562,7 +562,7 @@ void crypto_destroy_tfm(void *mem, struct crypto_tfm *tfm)
>  {
>  	struct crypto_alg *alg;
>  
> -	if (unlikely(!mem))
> +	if (IS_ERR_OR_NULL(mem))
>  		return;
>  
>  	alg = tfm->__crt_alg;

Could you update the comments for the functions which call crypto_destroy_tfm()
(crypto_free_aead(), crypto_free_skcipher(), etc.) to mention that they do
nothing when passed NULL or an ERR_PTR()?

- Eric