Re: [Qemu-devel] [PATCH] crypto: afalg: fix a NULL pointer dereference

[Eric Blake]

On 11/06/2017 12:21 AM, Longpeng(Mike) wrote:
> Test-crypto-hash calls qcrypto_hash_bytesv/digest/base64 with
> errp=NULL, this will cause a NULL poniter deference if afalg_driver

s/poniter deference/pointer dereference/

> doesn't support requested algos:
>     ret = qcrypto_hash_afalg_driver.hash_bytesv(alg, iov, niov,
>                                                 result, resultlen,
>                                                 errp);
>     if (ret == 0) {
>         return ret;
>     }
> 
>     error_free(*errp);  // <--- here
> 
> So we must check 'errp & *errp' before dereference.

No, if we are going to blindly ignore the error from the hash_bytesv()
call, then we should pass NULL rather than errp.

> 
> Signed-off-by: Longpeng(Mike) <address@hidden>
> ---
>  crypto/hash.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/hash.c b/crypto/hash.c
> index ac59c63..c464c78 100644
> --- a/crypto/hash.c
> +++ b/crypto/hash.c
> @@ -60,7 +60,9 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
>       * TODO:
>       * Maybe we should treat some afalg errors as fatal
>       */
> -    error_free(*errp);
> +    if (errp && *errp) {
> +        error_free(*errp);
> +    }

Any code that uses 'if (errp && *errp)' is suspicious; I think you need
a v2 that doesn't try to set errp in the first place, rather than
cleaning it up to ignore it after the fact.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature