* [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle()
@ 2018-10-15 11:14 Tomas Winkler
2018-10-16 5:41 ` Jason Gunthorpe
2018-10-17 23:01 ` Jarkko Sakkinen
0 siblings, 2 replies; 5+ messages in thread
From: Tomas Winkler @ 2018-10-15 11:14 UTC (permalink / raw)
To: Jarkko Sakkinen, Jason Gunthorpe
Cc: Nayna Jain, Alexander Usyskin, Tadeusz Struk, linux-integrity,
linux-security-module, linux-kernel, Tomas Winkler
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle()
2018-10-15 11:14 [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle() Tomas Winkler
@ 2018-10-16 5:41 ` Jason Gunthorpe
2018-10-16 6:20 ` Winkler, Tomas
2018-10-17 23:01 ` Jarkko Sakkinen
1 sibling, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2018-10-16 5:41 UTC (permalink / raw)
To: Tomas Winkler
Cc: Jarkko Sakkinen, Nayna Jain, Alexander Usyskin, Tadeusz Struk,
linux-integrity, linux-security-module, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle()
2018-10-16 5:41 ` Jason Gunthorpe
@ 2018-10-16 6:20 ` Winkler, Tomas
0 siblings, 0 replies; 5+ messages in thread
From: Winkler, Tomas @ 2018-10-16 6:20 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Jarkko Sakkinen, Nayna Jain, Usyskin, Alexander, Struk, Tadeusz,
linux-integrity, linux-security-module, linux-kernel
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle()
2018-10-15 11:14 [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle() Tomas Winkler
2018-10-16 5:41 ` Jason Gunthorpe
@ 2018-10-17 23:01 ` Jarkko Sakkinen
2018-10-17 23:08 ` Winkler, Tomas
1 sibling, 1 reply; 5+ messages in thread
From: Jarkko Sakkinen @ 2018-10-17 23:01 UTC (permalink / raw)
To: Tomas Winkler
Cc: Jarkko Sakkinen, Jason Gunthorpe, Nayna Jain, Alexander Usyskin,
Tadeusz Struk, linux-integrity, linux-security-module,
linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle()
2018-10-17 23:01 ` Jarkko Sakkinen
@ 2018-10-17 23:08 ` Winkler, Tomas
0 siblings, 0 replies; 5+ messages in thread
From: Winkler, Tomas @ 2018-10-17 23:08 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Jason Gunthorpe, Nayna Jain, Usyskin, Alexander, Struk, Tadeusz,
linux-integrity, linux-security-module, linux-kernel
> -----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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-17 23:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-15 11:14 [PATCH] tpm: tpm_try_transmit() ignore value of go_to_idle() Tomas Winkler
2018-10-16 5:41 ` Jason Gunthorpe
2018-10-16 6:20 ` Winkler, Tomas
2018-10-17 23:01 ` Jarkko Sakkinen
2018-10-17 23:08 ` Winkler, Tomas
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).