Should the return result of CRYPTO_UP_REF() / CRYPTO_DOWN_REF() be checked?

[Matt Caswell]

On 10/02/2020 00:15, SHANE LONTIS wrote:
> With the new architecture changes there are quite a few new calls to
> 
> CRYPTO_UP_REF()
> CRYPTO_DOWN_REF()
> 
> These methods return an int that is not being checked in lots of places.
> 
> This return value only seems to affect fallback code that calls CRYPTO_atomic_add (which can return 0 on lock or unlock failure)
> 
> SO the question is should we be checking this return value?

Yes, I think we should be.

Matt

> 
> 
> Note that not checking has resulted in a few assumptions in other code…
> e.g the following function returns void.
>  
> /crypto/evp/keymgmt_lib.c: 165 in evp_keymgmt_util_cache_pkey()
> 159     }
> 160     
> 161     void evp_keymgmt_util_cache_pkey(EVP_PKEY *pk, size_t index,
> 162                                      EVP_KEYMGMT *keymgmt, void *keydata)
> 163     {
> 164         if (keydata != NULL) {
>>>>    CID 1458170:  Error handling issues  (CHECKED_RETURN)
>>>>    Calling "EVP_KEYMGMT_up_ref" without checking return value (as is done elsewhere 4 out of 5 times).
> 165             EVP_KEYMGMT_up_ref(keymgmt);
> 
> NOTE: EVP_KEYMGMT_up_ref() just does an CRYPTO_UP_REF() call and always returns 1.
> 
>