diff mbox series

[1/1] tpm/tpm_crb: Fix error message in __crb_relinquish_locality()

Message ID 1668195533-16761-1-git-send-email-mikelley@microsoft.com
State New
Headers
Series [1/1] tpm/tpm_crb: Fix error message in __crb_relinquish_locality() |

Commit Message

Michael Kelley (LINUX) Nov. 11, 2022, 7:38 p.m. UTC 
  The error message in __crb_relinquish_locality() mentions requestAccess
instead of Relinquish. Fix it.

Fixes: 888d867df441 ("tpm: cmd_ready command can be issued only after granting locality")
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
 drivers/char/tpm/tpm_crb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tomas Winkler Nov. 12, 2022, 7:32 p.m. UTC | #1 
> -----Original Message-----
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Friday, November 11, 2022 21:39
> To: peterhuewe@gmx.de; jarkko@kernel.org; jgg@ziepe.ca; Winkler, Tomas
> <tomas.winkler@intel.com>; linux-integrity@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: mikelley@microsoft.com
> Subject: [PATCH 1/1] tpm/tpm_crb: Fix error message in
> __crb_relinquish_locality()
> 
> The error message in __crb_relinquish_locality() mentions requestAccess
> instead of Relinquish. Fix it.
> 
> Fixes: 888d867df441 ("tpm: cmd_ready command can be issued only after
> granting locality")
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Acked-by: Tomas Winkler <tomas.winkler@intel.com> 

> ---
>  drivers/char/tpm/tpm_crb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index
> 1860665..65f8f17 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -252,7 +252,7 @@ static int __crb_relinquish_locality(struct device *dev,
>  	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
>  	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
>  				 TPM2_TIMEOUT_C)) {
> -		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed
> out\n");
> +		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed
> out\n");
>  		return -ETIME;
>  	}
> 
> --
> 1.8.3.1
Michael Kelley (LINUX) Nov. 22, 2022, 4:23 p.m. UTC | #2 
From: Winkler, Tomas <tomas.winkler@intel.com> Sent: Saturday, November 12, 2022 11:33 AM
> > -----Original Message-----
> > From: Michael Kelley <mikelley@microsoft.com>
> > Sent: Friday, November 11, 2022 21:39
> > To: peterhuewe@gmx.de; jarkko@kernel.org; jgg@ziepe.ca; Winkler, Tomas
> > <tomas.winkler@intel.com>; linux-integrity@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Cc: mikelley@microsoft.com
> > Subject: [PATCH 1/1] tpm/tpm_crb: Fix error message in
> > __crb_relinquish_locality()
> >
> > The error message in __crb_relinquish_locality() mentions requestAccess
> > instead of Relinquish. Fix it.
> >
> > Fixes: 888d867df441 ("tpm: cmd_ready command can be issued only after
> > granting locality")
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> Acked-by: Tomas Winkler <tomas.winkler@intel.com>

Is there a maintainer who can pick up this fix?  It's not time critical; I'm
just trying to make sure it doesn't get lost.

Thanks,

Michael

> 
> > ---
> >  drivers/char/tpm/tpm_crb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index
> > 1860665..65f8f17 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -252,7 +252,7 @@ static int __crb_relinquish_locality(struct device *dev,
> >  	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
> >  	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
> >  				 TPM2_TIMEOUT_C)) {
> > -		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed
> > out\n");
> > +		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed
> > out\n");
> >  		return -ETIME;
> >  	}
> >
> > --
> > 1.8.3.1
Jarkko Sakkinen Nov. 27, 2022, 4:38 p.m. UTC | #3 
On Fri, Nov 11, 2022 at 11:38:53AM -0800, Michael Kelley wrote:
> The error message in __crb_relinquish_locality() mentions requestAccess
> instead of Relinquish. Fix it.
> 
> Fixes: 888d867df441 ("tpm: cmd_ready command can be issued only after granting locality")
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>  drivers/char/tpm/tpm_crb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 1860665..65f8f17 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -252,7 +252,7 @@ static int __crb_relinquish_locality(struct device *dev,
>  	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
>  	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
>  				 TPM2_TIMEOUT_C)) {
> -		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
> +		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed out\n");
>  		return -ETIME;
>  	}
>  
> -- 
> 1.8.3.1
> 

Please explain.

BR, Jarkko
Michael Kelley (LINUX) Nov. 29, 2022, 4:34 a.m. UTC | #4 
From: Jarkko Sakkinen <jarkko@kernel.org> Sent: Sunday, November 27, 2022 8:39 AM
> 
> On Fri, Nov 11, 2022 at 11:38:53AM -0800, Michael Kelley wrote:
> > The error message in __crb_relinquish_locality() mentions requestAccess
> > instead of Relinquish. Fix it.
> >
> > Fixes: 888d867df441 ("tpm: cmd_ready command can be issued only after granting locality")
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > ---
> >  drivers/char/tpm/tpm_crb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index 1860665..65f8f17 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -252,7 +252,7 @@ static int __crb_relinquish_locality(struct device *dev,
> >  	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
> >  	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
> >  				 TPM2_TIMEOUT_C)) {
> > -		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
> > +		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed out\n");
> >  		return -ETIME;
> >  	}
> >
> > --
> > 1.8.3.1
> >
> 
> Please explain.
> 

There are two parallel functions:  __crb_request_locality() and
__crb_relinquish_locality().  In the current code, both return the
same text in the error message if a timeout occurs.  That
error message seems appropriate for __crb_request_locality()
since it is setting the "requestAccess" bit.

But the error message seems inappropriate for
__crb_relinquish_locality(), which is setting the "Relinquish" bit.
So the patch changes the error message to indicate that the
timeout occurred in setting the Relinquish bit.

I'm looking at Section 6.5.3.2.2.1 in the TCG PC Client Platform
TPM Profile Specification for TPM 2.0, Version 1.05 Revision 14.
This is where the "requestAccess" and "Relinquish" bits are defined.

Or maybe I am not understanding what you are getting at with
"Please explain."

Michael
Jarkko Sakkinen Dec. 4, 2022, 7:51 p.m. UTC | #5 
On Tue, Nov 29, 2022 at 04:34:09AM +0000, Michael Kelley (LINUX) wrote:
> From: Jarkko Sakkinen <jarkko@kernel.org> Sent: Sunday, November 27, 2022 8:39 AM
> > 
> > On Fri, Nov 11, 2022 at 11:38:53AM -0800, Michael Kelley wrote:
> > > The error message in __crb_relinquish_locality() mentions requestAccess
> > > instead of Relinquish. Fix it.
> > >
> > > Fixes: 888d867df441 ("tpm: cmd_ready command can be issued only after granting locality")
> > > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > > ---
> > >  drivers/char/tpm/tpm_crb.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > > index 1860665..65f8f17 100644
> > > --- a/drivers/char/tpm/tpm_crb.c
> > > +++ b/drivers/char/tpm/tpm_crb.c
> > > @@ -252,7 +252,7 @@ static int __crb_relinquish_locality(struct device *dev,
> > >  	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
> > >  	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
> > >  				 TPM2_TIMEOUT_C)) {
> > > -		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
> > > +		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed out\n");
> > >  		return -ETIME;
> > >  	}
> > >
> > > --
> > > 1.8.3.1
> > >
> > 
> > Please explain.
> > 
> 
> There are two parallel functions:  __crb_request_locality() and
> __crb_relinquish_locality().  In the current code, both return the
> same text in the error message if a timeout occurs.  That
> error message seems appropriate for __crb_request_locality()
> since it is setting the "requestAccess" bit.
> 
> But the error message seems inappropriate for
> __crb_relinquish_locality(), which is setting the "Relinquish" bit.
> So the patch changes the error message to indicate that the
> timeout occurred in setting the Relinquish bit.
> 
> I'm looking at Section 6.5.3.2.2.1 in the TCG PC Client Platform
> TPM Profile Specification for TPM 2.0, Version 1.05 Revision 14.
> This is where the "requestAccess" and "Relinquish" bits are defined.
> 
> Or maybe I am not understanding what you are getting at with
> "Please explain."

I misread the callback name, when I first looked into this (in
too much rush). You're absolutely correct.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko
Jarkko Sakkinen Dec. 4, 2022, 7:53 p.m. UTC | #6 
On Sun, Dec 04, 2022 at 07:51:20PM +0000, Jarkko Sakkinen wrote:
> On Tue, Nov 29, 2022 at 04:34:09AM +0000, Michael Kelley (LINUX) wrote:
> > From: Jarkko Sakkinen <jarkko@kernel.org> Sent: Sunday, November 27, 2022 8:39 AM
> > > 
> > > On Fri, Nov 11, 2022 at 11:38:53AM -0800, Michael Kelley wrote:
> > > > The error message in __crb_relinquish_locality() mentions requestAccess
> > > > instead of Relinquish. Fix it.
> > > >
> > > > Fixes: 888d867df441 ("tpm: cmd_ready command can be issued only after granting locality")
> > > > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > > > ---
> > > >  drivers/char/tpm/tpm_crb.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > > > index 1860665..65f8f17 100644
> > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > @@ -252,7 +252,7 @@ static int __crb_relinquish_locality(struct device *dev,
> > > >  	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
> > > >  	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
> > > >  				 TPM2_TIMEOUT_C)) {
> > > > -		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
> > > > +		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed out\n");
> > > >  		return -ETIME;
> > > >  	}
> > > >
> > > > --
> > > > 1.8.3.1
> > > >
> > > 
> > > Please explain.
> > > 
> > 
> > There are two parallel functions:  __crb_request_locality() and
> > __crb_relinquish_locality().  In the current code, both return the
> > same text in the error message if a timeout occurs.  That
> > error message seems appropriate for __crb_request_locality()
> > since it is setting the "requestAccess" bit.
> > 
> > But the error message seems inappropriate for
> > __crb_relinquish_locality(), which is setting the "Relinquish" bit.
> > So the patch changes the error message to indicate that the
> > timeout occurred in setting the Relinquish bit.
> > 
> > I'm looking at Section 6.5.3.2.2.1 in the TCG PC Client Platform
> > TPM Profile Specification for TPM 2.0, Version 1.05 Revision 14.
> > This is where the "requestAccess" and "Relinquish" bits are defined.
> > 
> > Or maybe I am not understanding what you are getting at with
> > "Please explain."
> 
> I misread the callback name, when I first looked into this (in
> too much rush). You're absolutely correct.
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git

BR, Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 1860665..65f8f17 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -252,7 +252,7 @@  static int __crb_relinquish_locality(struct device *dev,
 	iowrite32(CRB_LOC_CTRL_RELINQUISH, &priv->regs_h->loc_ctrl);
 	if (!crb_wait_for_reg_32(&priv->regs_h->loc_state, mask, value,
 				 TPM2_TIMEOUT_C)) {
-		dev_warn(dev, "TPM_LOC_STATE_x.requestAccess timed out\n");
+		dev_warn(dev, "TPM_LOC_STATE_x.Relinquish timed out\n");
 		return -ETIME;
 	}