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

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

[Ard Biesheuvel]

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;
-- 
2.30.1

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

[Eric Biggers]

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