| Message ID | 20181015111434.7777-1-tomas.winkler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tpm: tpm_try_transmit() ignore value of go_to_idle() | expand |
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
> > 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
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
> -----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.
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);
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(-)