[bug report] security: keys: trusted: use ASN.1 TPM2 key format for the blobs

[bug report] security: keys: trusted: use ASN.1 TPM2 key format for the blobs

[Dan Carpenter]

Hello James Bottomley,

The patch f2219745250f: "security: keys: trusted: use ASN.1 TPM2 key
format for the blobs" from Jan 27, 2021, leads to the following
static checker warning:

security/keys/trusted-keys/trusted_tpm2.c:344 tpm2_seal_trusted() warn: inconsistent returns '&chip->ops_sem'.
  Locked on  : 339
  Unlocked on: 258,344
security/keys/trusted-keys/trusted_tpm2.c:344 tpm2_seal_trusted() warn: inconsistent returns '&chip->tpm_mutex'.
  Locked on  : 339
  Unlocked on: 258,344

security/keys/trusted-keys/trusted_tpm2.c
   324  
   325          blob_len = tpm2_key_encode(payload, options,
   326                                     &buf.data[TPM_HEADER_SIZE + 4],
   327                                     blob_len);
   328  
   329  out:
   330          tpm_buf_destroy(&buf);
   331  
   332          if (rc > 0) {
   333                  if (tpm2_rc_value(rc) == TPM2_RC_HASH)
   334                          rc = -EINVAL;
   335                  else
   336                          rc = -EPERM;
   337          }
   338          if (blob_len < 0)
   339                  return blob_len;
                        ^^^^^^^^^^^^^^^^
Need to call tpm_put_ops() or something before returning.

   340  
   341          payload->blob_len = blob_len;
   342  
   343          tpm_put_ops(chip);
   344          return rc;
   345  }

regards,
dan carpenter

Re: [bug report] security: keys: trusted: use ASN.1 TPM2 key format for the blobs

[James Bottomley]

On Thu, 2021-04-29 at 09:45 +0300, Dan Carpenter wrote:
> Hello James Bottomley,
> 
> The patch f2219745250f: "security: keys: trusted: use ASN.1 TPM2 key
> format for the blobs" from Jan 27, 2021, leads to the following
> static checker warning:
> 
> security/keys/trusted-keys/trusted_tpm2.c:344 tpm2_seal_trusted()
> warn: inconsistent returns '&chip->ops_sem'.
>   Locked on  : 339
>   Unlocked on: 258,344
> security/keys/trusted-keys/trusted_tpm2.c:344 tpm2_seal_trusted()
> warn: inconsistent returns '&chip->tpm_mutex'.
>   Locked on  : 339
>   Unlocked on: 258,344
> 
> security/keys/trusted-keys/trusted_tpm2.c
>    324  
>    325          blob_len = tpm2_key_encode(payload, options,
>    326                                     &buf.data[TPM_HEADER_SIZE
> + 4],
>    327                                     blob_len);
>    328  
>    329  out:
>    330          tpm_buf_destroy(&buf);
>    331  
>    332          if (rc > 0) {
>    333                  if (tpm2_rc_value(rc) == TPM2_RC_HASH)
>    334                          rc = -EINVAL;
>    335                  else
>    336                          rc = -EPERM;
>    337          }
>    338          if (blob_len < 0)
>    339                  return blob_len;
>                         ^^^^^^^^^^^^^^^^
> Need to call tpm_put_ops() or something before returning.
> 
>    340  
>    341          payload->blob_len = blob_len;
>    342  
>    343          tpm_put_ops(chip);
>    344          return rc;
>    345  }

Yes, you're right.  The best fix seems to be the below so as not to add
more error legs with puts.

James

---

diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index d225ad140960..cbf2a932577b 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -336,9 +336,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 			rc = -EPERM;
 	}
 	if (blob_len < 0)
-		return blob_len;
-
-	payload->blob_len = blob_len;
+		rc = blob_len;
+	else
+		payload->blob_len = blob_len;
 
 	tpm_put_ops(chip);
 	return rc;