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

[Eric Blake]

On 11/07/2017 05:32 AM, Longpeng(Mike) wrote:
> From: Longpeng <address@hidden>
> 
> Test-crypto-hash calls qcrypto_hash_bytesv/digest/base64 with
> errp=NULL, this will cause a NULL pointer deference if afalg_driver

s/deference/dereference/

(I called out two typo fixes against v1, but you only fixed one of them).

> 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
> 
> Because the error message is threw away immediately, so we should

s/threw/thrown/
s/so //

> just pass NULL to hash_bytesv(). There is also the same problem in
> afalg-backend cipher & hmac, let's fix them together.
> 
> Reported-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Longpeng <address@hidden>
> ---

> +++ b/crypto/hash.c
> @@ -51,16 +51,10 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
>  
>      ret = qcrypto_hash_afalg_driver.hash_bytesv(alg, iov, niov,
>                                                  result, resultlen,
> -                                                errp);
> +                                                NULL);
>      if (ret == 0) {
>          return ret;
>      }
> -
> -    /*
> -     * TODO:
> -     * Maybe we should treat some afalg errors as fatal
> -     */

The comment is probably still worth keeping (maybe hoisted above the
call to hash_bytesv(, NULL), though).

As moving (instead of deleting) a comment is trivial, I'm okay if a
maintainer does that while applying:

Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature