linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] trusted-keys: match tpm_get_ops on all return paths
@ 2021-04-29 18:37 Ben Boeckel
  2021-04-29 18:37 ` [PATCH 1/1] " Ben Boeckel
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Boeckel @ 2021-04-29 18:37 UTC (permalink / raw)
  To: keyrings
  Cc: Ben Boeckel, James Bottomley, linux-integrity, linux-kernel,
	linux-security-module

From: Ben Boeckel <mathstuf@gmail.com>

Bug report thread Message-Id: <YIpV9pcyM9/rWqEt@mwanda>

Ben Boeckel (1):
  trusted-keys: match tpm_get_ops on all return paths

 security/keys/trusted-keys/trusted_tpm2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: 3644286f6cbcea86f6fa4d308e7ac06bf2a3715a
-- 
2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] trusted-keys: match tpm_get_ops on all return paths
  2021-04-29 18:37 [PATCH 0/1] trusted-keys: match tpm_get_ops on all return paths Ben Boeckel
@ 2021-04-29 18:37 ` Ben Boeckel
  2021-04-29 18:50   ` James Bottomley
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Boeckel @ 2021-04-29 18:37 UTC (permalink / raw)
  To: keyrings
  Cc: Ben Boeckel, James Bottomley, linux-integrity, linux-kernel,
	linux-security-module, Dan Carpenter

From: Ben Boeckel <mathstuf@gmail.com>

The `tpm_get_ops` call at the beginning of the function is not paired
with a `tpm_put_ops` on this return path.

Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 security/keys/trusted-keys/trusted_tpm2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 617fabd4d913..25c2c4d564de 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -335,8 +335,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		else
 			rc = -EPERM;
 	}
-	if (blob_len < 0)
+	if (blob_len < 0) {
+		tpm_put_ops(chip);
 		return blob_len;
+	}
 
 	payload->blob_len = blob_len;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] trusted-keys: match tpm_get_ops on all return paths
  2021-04-29 18:37 ` [PATCH 1/1] " Ben Boeckel
@ 2021-04-29 18:50   ` James Bottomley
  2021-04-29 19:03     ` Ben Boeckel
  0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2021-04-29 18:50 UTC (permalink / raw)
  To: Ben Boeckel, keyrings
  Cc: Ben Boeckel, linux-integrity, linux-kernel,
	linux-security-module, Dan Carpenter

On Thu, 2021-04-29 at 14:37 -0400, Ben Boeckel wrote:
> From: Ben Boeckel <mathstuf@gmail.com>
> 
> The `tpm_get_ops` call at the beginning of the function is not paired
> with a `tpm_put_ops` on this return path.
> 
> Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key
> format for the blobs")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
> ---
>  security/keys/trusted-keys/trusted_tpm2.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c
> b/security/keys/trusted-keys/trusted_tpm2.c
> index 617fabd4d913..25c2c4d564de 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -335,8 +335,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		else
>  			rc = -EPERM;
>  	}
> -	if (blob_len < 0)
> +	if (blob_len < 0) {
> +		tpm_put_ops(chip);
>  		return blob_len;
> +	}
>  
>  	payload->blob_len = blob_len;
>  

Actually, I think this is a better fix to avoid multiple put and
returns.

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;


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] trusted-keys: match tpm_get_ops on all return paths
  2021-04-29 18:50   ` James Bottomley
@ 2021-04-29 19:03     ` Ben Boeckel
  2021-04-29 19:08       ` James Bottomley
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Boeckel @ 2021-04-29 19:03 UTC (permalink / raw)
  To: James Bottomley
  Cc: keyrings, Ben Boeckel, linux-integrity, linux-kernel,
	linux-security-module, Dan Carpenter

On Thu, Apr 29, 2021 at 11:50:50 -0700, James Bottomley wrote:
> Actually, I think this is a better fix to avoid multiple put and
> returns.
> 
> 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;

Ah, that does look better. I had first added a new label, but that
didn't seem like an improvement in readability. I grabbed this pattern
from an early return earlier in the function. But given that this is the
end (and appears to be unlikely to have more logic inserted in the
future), this seems more reasonable to me as well. Do you want me to
respin or just let it up to you at this point?

Thanks,

--Ben

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] trusted-keys: match tpm_get_ops on all return paths
  2021-04-29 19:03     ` Ben Boeckel
@ 2021-04-29 19:08       ` James Bottomley
  0 siblings, 0 replies; 5+ messages in thread
From: James Bottomley @ 2021-04-29 19:08 UTC (permalink / raw)
  To: Ben Boeckel
  Cc: keyrings, Ben Boeckel, linux-integrity, linux-kernel,
	linux-security-module, Dan Carpenter

On Thu, 2021-04-29 at 15:03 -0400, Ben Boeckel wrote:
> On Thu, Apr 29, 2021 at 11:50:50 -0700, James Bottomley wrote:
> > Actually, I think this is a better fix to avoid multiple put and
> > returns.
> > 
> > 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;
> 
> Ah, that does look better. I had first added a new label, but that
> didn't seem like an improvement in readability. I grabbed this
> pattern from an early return earlier in the function. But given that
> this is the end (and appears to be unlikely to have more logic
> inserted in the future), this seems more reasonable to me as well. Do
> you want me to respin or just let it up to you at this point?

Can you respin? ... I'm a bit lossy at the moment due to pressure of
work.

Thanks,

James



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-29 19:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-29 18:37 [PATCH 0/1] trusted-keys: match tpm_get_ops on all return paths Ben Boeckel
2021-04-29 18:37 ` [PATCH 1/1] " Ben Boeckel
2021-04-29 18:50   ` James Bottomley
2021-04-29 19:03     ` Ben Boeckel
2021-04-29 19:08       ` James Bottomley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).