tpm: fix incorrect success returns from tpm_try_transmit()
diff mbox series

Message ID 1546280851.3079.2.camel@HansenPartnership.com
State New
Headers show
Series
  • tpm: fix incorrect success returns from tpm_try_transmit()
Related show

Commit Message

James Bottomley Dec. 31, 2018, 6:27 p.m. UTC
Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
runtime_pm" we have been returning success from tpm_try_transmit()
even if an error occurred.  The reason is that the introduction of rc
= tpm_go_idle() at the end of processing overwrites the value of rc if
it contains an error code (mostly with success).  Fix this by writing
the return to a new variable rc1 instead.

Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from runtime_pm"
Cc: stable@vger.kernel.org
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

Note: the goto out looks fishy as well.  The only go_idle implementor
is tpm_crb and that can return a timeout as -ETIME, so it looks like it
would then loop forever

Comments

Jarkko Sakkinen Jan. 3, 2019, 12:59 p.m. UTC | #1
On Mon, Dec 31, 2018 at 10:27:31AM -0800, James Bottomley wrote:
> Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
> runtime_pm" we have been returning success from tpm_try_transmit()
> even if an error occurred.  The reason is that the introduction of rc
> = tpm_go_idle() at the end of processing overwrites the value of rc if
> it contains an error code (mostly with success).  Fix this by writing
> the return to a new variable rc1 instead.
> 
> Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from runtime_pm"
> Cc: stable@vger.kernel.org
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> ---
> 
> Note: the goto out looks fishy as well.  The only go_idle implementor
> is tpm_crb and that can return a timeout as -ETIME, so it looks like it
> would then loop forever
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 129f640424b7..ac7ebab6140c 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -432,7 +432,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>  				unsigned int flags)
>  {
>  	struct tpm_output_header *header = (void *)buf;
> -	int rc;
> +	int rc, rc1;
>  	ssize_t len = 0;
>  	u32 count, ordinal;
>  	unsigned long stop;
> @@ -547,8 +547,8 @@ 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)
> +	rc1 = tpm_go_idle(chip, flags);
> +	if (rc1)
>  		goto out;
>  
>  	if (need_locality)

Thanks James and sorry for latency (holiday season). Just a small
suggestion. I would just:

if (tpm_go_idle(chip, flags))
	goto out;

What do you think?

/Jarkko
Winkler, Tomas Jan. 3, 2019, 1:37 p.m. UTC | #2
> 
> On Mon, Dec 31, 2018 at 10:27:31AM -0800, James Bottomley wrote:
> > Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > runtime_pm" we have been returning success from tpm_try_transmit()
> > even if an error occurred.  The reason is that the introduction of rc
> > = tpm_go_idle() at the end of processing overwrites the value of rc if
> > it contains an error code (mostly with success).  Fix this by writing
> > the return to a new variable rc1 instead.
> >
> > Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from runtime_pm"
> > Cc: stable@vger.kernel.org
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> >
> > ---
> >
> > Note: the goto out looks fishy as well.  The only go_idle implementor
> > is tpm_crb and that can return a timeout as -ETIME, so it looks like
> > it would then loop forever
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index 129f640424b7..ac7ebab6140c 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -432,7 +432,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> *chip,
> >  				unsigned int flags)
> >  {
> >  	struct tpm_output_header *header = (void *)buf;
> > -	int rc;
> > +	int rc, rc1;
> >  	ssize_t len = 0;
> >  	u32 count, ordinal;
> >  	unsigned long stop;
> > @@ -547,8 +547,8 @@ 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)
> > +	rc1 = tpm_go_idle(chip, flags);
> > +	if (rc1)
> >  		goto out;
> >
> >  	if (need_locality)
> 
> Thanks James and sorry for latency (holiday season). Just a small suggestion. I
> would just:
> 
> if (tpm_go_idle(chip, flags))
> 	goto out;
> 
> What do you think?


This is wrong as well because of the jump to 'out'.
I've introduced this bug in version 4 of the patch, when the wrappers were added, before the jump to 'out' was correct.
If it's okay with you I will post my version of the fix.
Thanks
Tomas
Jarkko Sakkinen Jan. 3, 2019, 3:03 p.m. UTC | #3
On Thu, Jan 03, 2019 at 01:37:09PM +0000, Winkler, Tomas wrote:
> > 
> > On Mon, Dec 31, 2018 at 10:27:31AM -0800, James Bottomley wrote:
> > > Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > > runtime_pm" we have been returning success from tpm_try_transmit()
> > > even if an error occurred.  The reason is that the introduction of rc
> > > = tpm_go_idle() at the end of processing overwrites the value of rc if
> > > it contains an error code (mostly with success).  Fix this by writing
> > > the return to a new variable rc1 instead.
> > >
> > > Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from runtime_pm"
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > >
> > > ---
> > >
> > > Note: the goto out looks fishy as well.  The only go_idle implementor
> > > is tpm_crb and that can return a timeout as -ETIME, so it looks like
> > > it would then loop forever
> > >
> > > diff --git a/drivers/char/tpm/tpm-interface.c
> > > b/drivers/char/tpm/tpm-interface.c
> > > index 129f640424b7..ac7ebab6140c 100644
> > > --- a/drivers/char/tpm/tpm-interface.c
> > > +++ b/drivers/char/tpm/tpm-interface.c
> > > @@ -432,7 +432,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> > *chip,
> > >  				unsigned int flags)
> > >  {
> > >  	struct tpm_output_header *header = (void *)buf;
> > > -	int rc;
> > > +	int rc, rc1;
> > >  	ssize_t len = 0;
> > >  	u32 count, ordinal;
> > >  	unsigned long stop;
> > > @@ -547,8 +547,8 @@ 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)
> > > +	rc1 = tpm_go_idle(chip, flags);
> > > +	if (rc1)
> > >  		goto out;
> > >
> > >  	if (need_locality)
> > 
> > Thanks James and sorry for latency (holiday season). Just a small suggestion. I
> > would just:
> > 
> > if (tpm_go_idle(chip, flags))
> > 	goto out;
> > 
> > What do you think?
> 
> 
> This is wrong as well because of the jump to 'out'.
> I've introduced this bug in version 4 of the patch, when the wrappers were added, before the jump to 'out' was correct.
> If it's okay with you I will post my version of the fix.
> Thanks
> Tomas

Works for me.

/Jarkko
James Bottomley Jan. 3, 2019, 3:23 p.m. UTC | #4
On Thu, 2019-01-03 at 14:59 +0200, Jarkko Sakkinen wrote:
> On Mon, Dec 31, 2018 at 10:27:31AM -0800, James Bottomley wrote:
> > Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > runtime_pm" we have been returning success from tpm_try_transmit()
> > even if an error occurred.  The reason is that the introduction of
> > rc
> > = tpm_go_idle() at the end of processing overwrites the value of rc
> > if
> > it contains an error code (mostly with success).  Fix this by
> > writing
> > the return to a new variable rc1 instead.
> > 
> > Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > runtime_pm"
> > Cc: stable@vger.kernel.org
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > om>
> > 
> > ---
> > 
> > Note: the goto out looks fishy as well.  The only go_idle
> > implementor
> > is tpm_crb and that can return a timeout as -ETIME, so it looks
> > like it
> > would then loop forever
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index 129f640424b7..ac7ebab6140c 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -432,7 +432,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> > *chip,
> >  				unsigned int flags)
> >  {
> >  	struct tpm_output_header *header = (void *)buf;
> > -	int rc;
> > +	int rc, rc1;
> >  	ssize_t len = 0;
> >  	u32 count, ordinal;
> >  	unsigned long stop;
> > @@ -547,8 +547,8 @@ 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)
> > +	rc1 = tpm_go_idle(chip, flags);
> > +	if (rc1)
> >  		goto out;
> >  
> >  	if (need_locality)
> 
> Thanks James and sorry for latency (holiday season). Just a small
> suggestion. I would just:
> 
> if (tpm_go_idle(chip, flags))
> 	goto out;
> 
> What do you think?

That it doesn't solve the loop forever with no warning problem.  If
anything, I think the correct thing is probably

	rc1 = tpm_go_idle(chip, flags);
	if (rc1)
  		dev_err(&chip->dev, "go idle failed with %d\n", rc1);

so we log the problem and move on.  If it is a timeout, it will likely
show up on the next TPM operation.  Since this is the only caller of
tpm_go_idle(), I think all looping should be done inside that function,
but we should probably wait for Tomas to comment since he wrote it.

James
Winkler, Tomas Jan. 3, 2019, 3:34 p.m. UTC | #5
> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Thursday, January 03, 2019 17:24
> To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Cc: Winkler, Tomas <tomas.winkler@intel.com>; linux-
> integrity@vger.kernel.org
> Subject: Re: [PATCH] tpm: fix incorrect success returns from tpm_try_transmit()
> 
> On Thu, 2019-01-03 at 14:59 +0200, Jarkko Sakkinen wrote:
> > On Mon, Dec 31, 2018 at 10:27:31AM -0800, James Bottomley wrote:
> > > Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > > runtime_pm" we have been returning success from tpm_try_transmit()
> > > even if an error occurred.  The reason is that the introduction of
> > > rc = tpm_go_idle() at the end of processing overwrites the value of
> > > rc if it contains an error code (mostly with success).  Fix this by
> > > writing the return to a new variable rc1 instead.
> > >
> > > Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > > runtime_pm"
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > > om>
> > >
> > > ---
> > >
> > > Note: the goto out looks fishy as well.  The only go_idle
> > > implementor is tpm_crb and that can return a timeout as -ETIME, so
> > > it looks like it would then loop forever
> > >
> > > diff --git a/drivers/char/tpm/tpm-interface.c
> > > b/drivers/char/tpm/tpm-interface.c
> > > index 129f640424b7..ac7ebab6140c 100644
> > > --- a/drivers/char/tpm/tpm-interface.c
> > > +++ b/drivers/char/tpm/tpm-interface.c
> > > @@ -432,7 +432,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip
> > > *chip,
> > >  				unsigned int flags)
> > >  {
> > >  	struct tpm_output_header *header = (void *)buf;
> > > -	int rc;
> > > +	int rc, rc1;
> > >  	ssize_t len = 0;
> > >  	u32 count, ordinal;
> > >  	unsigned long stop;
> > > @@ -547,8 +547,8 @@ 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)
> > > +	rc1 = tpm_go_idle(chip, flags);
> > > +	if (rc1)
> > >  		goto out;
> > >
> > >  	if (need_locality)
> >
> > Thanks James and sorry for latency (holiday season). Just a small
> > suggestion. I would just:
> >
> > if (tpm_go_idle(chip, flags))
> > 	goto out;
> >
> > What do you think?
> 
> That it doesn't solve the loop forever with no warning problem.  If anything, I
> think the correct thing is probably
> 
> 	rc1 = tpm_go_idle(chip, flags);
> 	if (rc1)
>   		dev_err(&chip->dev, "go idle failed with %d\n", rc1);
> 
> so we log the problem and move on.  If it is a timeout, it will likely show up on
> the next TPM operation.  Since this is the only caller of tpm_go_idle(), I think all
> looping should be done inside that function, but we should probably wait for
> Tomas to comment since he wrote it.
> 

We've already fixed it, I forgot myself , we were drinking too much :)
https://patchwork.kernel.org/patch/10643565/
Not sure why it was dropped.

Thanks
Tomas
James Bottomley Jan. 3, 2019, 4:17 p.m. UTC | #6
On Thu, 2019-01-03 at 15:34 +0000, Winkler, Tomas wrote:
> > -----Original Message-----
> > From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com
> > ]
> > Sent: Thursday, January 03, 2019 17:24
> > To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Cc: Winkler, Tomas <tomas.winkler@intel.com>; linux-
> > integrity@vger.kernel.org
> > Subject: Re: [PATCH] tpm: fix incorrect success returns from
> > tpm_try_transmit()
> > 
> > On Thu, 2019-01-03 at 14:59 +0200, Jarkko Sakkinen wrote:
> > > On Mon, Dec 31, 2018 at 10:27:31AM -0800, James Bottomley wrote:
> > > > Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > > > runtime_pm" we have been returning success from
> > > > tpm_try_transmit() even if an error occurred.  The reason is
> > > > that the introduction of rc = tpm_go_idle() at the end of
> > > > processing overwrites the value of rc if it contains an error
> > > > code (mostly with success).  Fix this by writing the return to
> > > > a new variable rc1 instead.
> > > > 
> > > > Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > > > runtime_pm"
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnersh
> > > > ip.c
> > > > om>
> > > > 
> > > > ---
> > > > 
> > > > Note: the goto out looks fishy as well.  The only go_idle
> > > > implementor is tpm_crb and that can return a timeout as -ETIME,
> > > > so it looks like it would then loop forever
> > > > 
> > > > diff --git a/drivers/char/tpm/tpm-interface.c
> > > > b/drivers/char/tpm/tpm-interface.c
> > > > index 129f640424b7..ac7ebab6140c 100644
> > > > --- a/drivers/char/tpm/tpm-interface.c
> > > > +++ b/drivers/char/tpm/tpm-interface.c
> > > > @@ -432,7 +432,7 @@ static ssize_t tpm_try_transmit(struct
> > > > tpm_chip
> > > > *chip,
> > > >  				unsigned int flags)
> > > >  {
> > > >  	struct tpm_output_header *header = (void *)buf;
> > > > -	int rc;
> > > > +	int rc, rc1;
> > > >  	ssize_t len = 0;
> > > >  	u32 count, ordinal;
> > > >  	unsigned long stop;
> > > > @@ -547,8 +547,8 @@ 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)
> > > > +	rc1 = tpm_go_idle(chip, flags);
> > > > +	if (rc1)
> > > >  		goto out;
> > > > 
> > > >  	if (need_locality)
> > > 
> > > Thanks James and sorry for latency (holiday season). Just a small
> > > suggestion. I would just:
> > > 
> > > if (tpm_go_idle(chip, flags))
> > > 	goto out;
> > > 
> > > What do you think?
> > 
> > That it doesn't solve the loop forever with no warning problem.  If
> > anything, I think the correct thing is probably
> > 
> > 	rc1 = tpm_go_idle(chip, flags);
> > 	if (rc1)
> >   		dev_err(&chip->dev, "go idle failed with %d\n",
> > rc1);
> > 
> > so we log the problem and move on.  If it is a timeout, it will
> > likely show up on the next TPM operation.  Since this is the only
> > caller of tpm_go_idle(), I think all looping should be done inside
> > that function, but we should probably wait for Tomas to comment
> > since he wrote it.
> > 
> 
> We've already fixed it, I forgot myself , we were drinking too much
> :)
> https://patchwork.kernel.org/patch/10643565/
> Not sure why it was dropped.

Taking the trouble to gather error returns and then ignoring them is
not a good practice (it's actually been the bane of filesystems for a
while).  If you want to do it this way, tpm_go_idle() needs to be a
void function that emits an error message for every problem condition.

James
Jarkko Sakkinen Jan. 10, 2019, 5:16 p.m. UTC | #7
On Thu, Jan 03, 2019 at 03:34:24PM +0000, Winkler, Tomas wrote:
> We've already fixed it, I forgot myself , we were drinking too much :)
> https://patchwork.kernel.org/patch/10643565/
> Not sure why it was dropped.
> 
> Thanks
> Tomas

LOL what. Clearly I'm coming from holidays. This fix is in my tree
and it was part of my last PR to James (for 5.0).

And James' patch applies neither my master and next braches so I
suppose he is using some old tree.

/Jarkko
Jarkko Sakkinen Jan. 10, 2019, 5:16 p.m. UTC | #8
On Thu, Jan 03, 2019 at 08:17:18AM -0800, James Bottomley wrote:
> On Thu, 2019-01-03 at 15:34 +0000, Winkler, Tomas wrote:
> > > -----Original Message-----
> > > From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com
> > > ]
> > > Sent: Thursday, January 03, 2019 17:24
> > > To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > Cc: Winkler, Tomas <tomas.winkler@intel.com>; linux-
> > > integrity@vger.kernel.org
> > > Subject: Re: [PATCH] tpm: fix incorrect success returns from
> > > tpm_try_transmit()
> > > 
> > > On Thu, 2019-01-03 at 14:59 +0200, Jarkko Sakkinen wrote:
> > > > On Mon, Dec 31, 2018 at 10:27:31AM -0800, James Bottomley wrote:
> > > > > Ever since 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > > > > runtime_pm" we have been returning success from
> > > > > tpm_try_transmit() even if an error occurred.  The reason is
> > > > > that the introduction of rc = tpm_go_idle() at the end of
> > > > > processing overwrites the value of rc if it contains an error
> > > > > code (mostly with success).  Fix this by writing the return to
> > > > > a new variable rc1 instead.
> > > > > 
> > > > > Fixes: 627448e85c766 "tpm: separate cmd_ready/go_idle from
> > > > > runtime_pm"
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnersh
> > > > > ip.c
> > > > > om>
> > > > > 
> > > > > ---
> > > > > 
> > > > > Note: the goto out looks fishy as well.  The only go_idle
> > > > > implementor is tpm_crb and that can return a timeout as -ETIME,
> > > > > so it looks like it would then loop forever
> > > > > 
> > > > > diff --git a/drivers/char/tpm/tpm-interface.c
> > > > > b/drivers/char/tpm/tpm-interface.c
> > > > > index 129f640424b7..ac7ebab6140c 100644
> > > > > --- a/drivers/char/tpm/tpm-interface.c
> > > > > +++ b/drivers/char/tpm/tpm-interface.c
> > > > > @@ -432,7 +432,7 @@ static ssize_t tpm_try_transmit(struct
> > > > > tpm_chip
> > > > > *chip,
> > > > >  				unsigned int flags)
> > > > >  {
> > > > >  	struct tpm_output_header *header = (void *)buf;
> > > > > -	int rc;
> > > > > +	int rc, rc1;
> > > > >  	ssize_t len = 0;
> > > > >  	u32 count, ordinal;
> > > > >  	unsigned long stop;
> > > > > @@ -547,8 +547,8 @@ 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)
> > > > > +	rc1 = tpm_go_idle(chip, flags);
> > > > > +	if (rc1)
> > > > >  		goto out;
> > > > > 
> > > > >  	if (need_locality)
> > > > 
> > > > Thanks James and sorry for latency (holiday season). Just a small
> > > > suggestion. I would just:
> > > > 
> > > > if (tpm_go_idle(chip, flags))
> > > > 	goto out;
> > > > 
> > > > What do you think?
> > > 
> > > That it doesn't solve the loop forever with no warning problem.  If
> > > anything, I think the correct thing is probably
> > > 
> > > 	rc1 = tpm_go_idle(chip, flags);
> > > 	if (rc1)
> > >   		dev_err(&chip->dev, "go idle failed with %d\n",
> > > rc1);
> > > 
> > > so we log the problem and move on.  If it is a timeout, it will
> > > likely show up on the next TPM operation.  Since this is the only
> > > caller of tpm_go_idle(), I think all looping should be done inside
> > > that function, but we should probably wait for Tomas to comment
> > > since he wrote it.
> > > 
> > 
> > We've already fixed it, I forgot myself , we were drinking too much
> > :)
> > https://patchwork.kernel.org/patch/10643565/
> > Not sure why it was dropped.
> 
> Taking the trouble to gather error returns and then ignoring them is
> not a good practice (it's actually been the bane of filesystems for a
> while).  If you want to do it this way, tpm_go_idle() needs to be a
> void function that emits an error message for every problem condition.

I'm happy to take a patch that adds logging in.

/Jarkko

Patch
diff mbox series

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 129f640424b7..ac7ebab6140c 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -432,7 +432,7 @@  static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 				unsigned int flags)
 {
 	struct tpm_output_header *header = (void *)buf;
-	int rc;
+	int rc, rc1;
 	ssize_t len = 0;
 	u32 count, ordinal;
 	unsigned long stop;
@@ -547,8 +547,8 @@  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)
+	rc1 = tpm_go_idle(chip, flags);
+	if (rc1)
 		goto out;
 
 	if (need_locality)