Re: [Qemu-devel] [PATCH] crypto: assert that qcrypto_hash_digest_len is in range

[Eric Blake]

On 05/20/2016 03:09 AM, Paolo Bonzini wrote:
> Otherwise unintended results could happen.  For example,
> Coverity reports a division by zero in qcrypto_afsplit_hash.
> While this cannot really happen, it shows that the contract
> of qcrypto_hash_digest_len can be improved.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  crypto/hash.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Eric Blake <address@hidden>

> 
> diff --git a/crypto/hash.c b/crypto/hash.c
> index b90af34..2907bff 100644
> --- a/crypto/hash.c
> +++ b/crypto/hash.c
> @@ -36,9 +36,7 @@ static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG__MAX] 
> = {
>  
>  size_t qcrypto_hash_digest_len(QCryptoHashAlgorithm alg)
>  {
> -    if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_size)) {
> -        return 0;
> -    }
> +    assert(alg < G_N_ELEMENTS(qcrypto_hash_alg_size));
>      return qcrypto_hash_alg_size[alg];

The assertion doesn't protect us if QCryptoHashAlgorithm gains another
member but we forget to update qcrypto_hash_alg_size[] to match.  Do you
want an additional assertion that you are returning a non-zero value?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature