tpm: tpm_try_transmit() ignore value of go_to_idle()
diff mbox series

Message ID 20181015111434.7777-1-tomas.winkler@intel.com
State New
Headers show
Series
  • tpm: tpm_try_transmit() ignore value of go_to_idle()
Related show

Commit Message

Winkler, Tomas Oct. 15, 2018, 11:14 a.m. UTC
Ignore the return value of go_to_idle() in tpm_try_transmit().
Once it may shadow the return value of actual tpm operation,
second the consequent command will fail as well and the error
will be caought anyway.
Last fix wrong goto, that jumped back instead of forward.

Fixes: 627448e85c76 ("tpm: separate cmd_ready/go_idle from runtime_pm")
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/char/tpm/tpm-interface.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Jason Gunthorpe Oct. 16, 2018, 5:41 a.m. UTC | #1
On Mon, Oct 15, 2018 at 02:14:34PM +0300, Tomas Winkler wrote:
> Ignore the return value of go_to_idle() in tpm_try_transmit().
> Once it may shadow the return value of actual tpm operation,
> second the consequent command will fail as well and the error
> will be caought anyway.
> Last fix wrong goto, that jumped back instead of forward.
> 
> Fixes: 627448e85c76 ("tpm: separate cmd_ready/go_idle from runtime_pm")
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>  drivers/char/tpm/tpm-interface.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 129f640424b7..f69c711bf74a 100644
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -547,9 +547,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>  		dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
>  
>  out:
> -	rc = tpm_go_idle(chip, flags);
> -	if (rc)
> -		goto out;
> +	(void)tpm_go_idle(chip, flags);

We don't really use this style in the kernel, AFAIK.

Jason
Winkler, Tomas Oct. 16, 2018, 6:20 a.m. UTC | #2
> 
> On Mon, Oct 15, 2018 at 02:14:34PM +0300, Tomas Winkler wrote:
> > Ignore the return value of go_to_idle() in tpm_try_transmit().
> > Once it may shadow the return value of actual tpm operation, second
> > the consequent command will fail as well and the error will be caought
> > anyway.
> > Last fix wrong goto, that jumped back instead of forward.
> >
> > Fixes: 627448e85c76 ("tpm: separate cmd_ready/go_idle from
> > runtime_pm")
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > drivers/char/tpm/tpm-interface.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index 129f640424b7..f69c711bf74a 100644
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -547,9 +547,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> *chip,
> >  		dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
> >
> >  out:
> > -	rc = tpm_go_idle(chip, flags);
> > -	if (rc)
> > -		goto out;
> > +	(void)tpm_go_idle(chip, flags);
> 
> We don't really use this style in the kernel, AFAIK.


Right, there are no many of those; just wanted to emphasize that return value is ignored. I can drop (void).

Thanks
Tomas
Jarkko Sakkinen Oct. 17, 2018, 11:01 p.m. UTC | #3
On Mon, 15 Oct 2018, Tomas Winkler wrote:
> Ignore the return value of go_to_idle() in tpm_try_transmit().
> Once it may shadow the return value of actual tpm operation,
> second the consequent command will fail as well and the error
> will be caought anyway.
> Last fix wrong goto, that jumped back instead of forward.
>
> Fixes: 627448e85c76 ("tpm: separate cmd_ready/go_idle from runtime_pm")
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
> drivers/char/tpm/tpm-interface.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 129f640424b7..f69c711bf74a 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -547,9 +547,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> 		dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
>
> out:
> -	rc = tpm_go_idle(chip, flags);
> -	if (rc)
> -		goto out;
> +	(void)tpm_go_idle(chip, flags);
>
> 	if (need_locality)
> 		tpm_relinquish_locality(chip, flags);
> -- 
> 2.14.4
>
>

LGTM. Should be probably Cc'd to stable (can add).

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko
Winkler, Tomas Oct. 17, 2018, 11:08 p.m. UTC | #4
> -----Original Message-----
> From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> Sent: Thursday, October 18, 2018 02:01
> To: Winkler, Tomas <tomas.winkler@intel.com>
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>; Jason Gunthorpe
> <jgg@ziepe.ca>; Nayna Jain <nayna@linux.vnet.ibm.com>; Usyskin,
> Alexander <alexander.usyskin@intel.com>; Struk, Tadeusz
> <tadeusz.struk@intel.com>; linux-integrity@vger.kernel.org; linux-security-
> module@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle()
> 
> On Mon, 15 Oct 2018, Tomas Winkler wrote:
> > Ignore the return value of go_to_idle() in tpm_try_transmit().
> > Once it may shadow the return value of actual tpm operation, second
> > the consequent command will fail as well and the error will be caought
> > anyway.
> > Last fix wrong goto, that jumped back instead of forward.
> >
> > Fixes: 627448e85c76 ("tpm: separate cmd_ready/go_idle from
> > runtime_pm")
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > ---
> > drivers/char/tpm/tpm-interface.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index 129f640424b7..f69c711bf74a 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -547,9 +547,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> *chip,
> > 		dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
> >
> > out:
> > -	rc = tpm_go_idle(chip, flags);
> > -	if (rc)
> > -		goto out;
> > +	(void)tpm_go_idle(chip, flags);
> >
> > 	if (need_locality)
> > 		tpm_relinquish_locality(chip, flags);
> > --
> > 2.14.4
> >
> >
> 
> LGTM. Should be probably Cc'd to stable (can add).
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
I've posted another one, there are more issues in the flow.

Patch
diff mbox series

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 129f640424b7..f69c711bf74a 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -547,9 +547,7 @@  static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 		dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
 
 out:
-	rc = tpm_go_idle(chip, flags);
-	if (rc)
-		goto out;
+	(void)tpm_go_idle(chip, flags);
 
 	if (need_locality)
 		tpm_relinquish_locality(chip, flags);