[bug report] evm: Check also if *tfm is an error pointer in init_desc()

[Dan Carpenter]

Hello Roberto Sassu,

The patch 53de3b080d5e: "evm: Check also if *tfm is an error pointer
in init_desc()" from Apr 27, 2020, leads to the following static
checker warning:

	security/integrity/evm/evm_crypto.c:119 init_desc()
	error: '*tfm' dereferencing possible ERR_PTR()

security/integrity/evm/evm_crypto.c
    89  
    90                  tfm = &evm_tfm[hash_algo];
    91                  algo = hash_algo_name[hash_algo];
    92          }
    93  
    94          if (IS_ERR_OR_NULL(*tfm)) {

This used to be a "if (!*tfm)" check.

    95                  mutex_lock(&mutex);
    96                  if (*tfm)
    97                          goto out;

Then we test again with the lock held.  But in the new code if "*tfm"
is an error pointer then we jump directly to the unlock and crash on the
next line.  I can't see how the commit would fix anything.

    98                  *tfm = crypto_alloc_shash(algo, 0, CRYPTO_NOLOAD);
    99                  if (IS_ERR(*tfm)) {
   100                          rc = PTR_ERR(*tfm);
   101                          pr_err("Can not allocate %s (reason: %ld)\n", algo, rc);
   102                          *tfm = NULL;
   103                          mutex_unlock(&mutex);
   104                          return ERR_PTR(rc);
   105                  }
   106                  if (type == EVM_XATTR_HMAC) {
   107                          rc = crypto_shash_setkey(*tfm, evmkey, evmkey_len);
   108                          if (rc) {
   109                                  crypto_free_shash(*tfm);
   110                                  *tfm = NULL;
   111                                  mutex_unlock(&mutex);
   112                                  return ERR_PTR(rc);
   113                          }
   114                  }
   115  out:
   116                  mutex_unlock(&mutex);
   117          }
   118  
   119          desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(*tfm),
                                                                     ^^^^
I don't understand how using *tfm outside of a lock is safe at all
anyway.

   120                          GFP_KERNEL);
   121          if (!desc)
   122                  return ERR_PTR(-ENOMEM);
   123  

regards,
dan carpenter