diff mbox series

tpm: fix build break in tpm-chip.c caused by AMD fTPM quirk

Message ID de3ee520780be213c421685805c751dcda0754df.camel@HansenPartnership.com (mailing list archive)
State New, archived
Headers show
Series tpm: fix build break in tpm-chip.c caused by AMD fTPM quirk | expand

Commit Message

James Bottomley March 20, 2023, 11:15 a.m. UTC 
The test for the AMD fTPM problem, which just went in, actually uses
the wrong function template for request_locality().  It's missing an
argument so the build breaks:

drivers/char/tpm/tpm-chip.c:568:8: error: too few arguments to function ‘tpm_request_locality’
  ret = tpm_request_locality(chip);
        ^~~~~~~~~~~~~~~~~~~~
drivers/char/tpm/tpm-chip.c:43:12: note: declared here
 static int tpm_request_locality(struct tpm_chip *chip, int locality)
            ^~~~~~~~~~~~~~~~~~~~

Fix this by requesting zero locality.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Fixes: f1324bbc4011 ("tpm: disable hwrng for fTPM on some AMD designs")

---

Comments

James Bottomley March 20, 2023, 11:22 a.m. UTC | #1 
On Mon, 2023-03-20 at 07:15 -0400, James Bottomley wrote:
> The test for the AMD fTPM problem, which just went in, actually uses
> the wrong function template for request_locality().  It's missing an
> argument so the build breaks:
> 
> drivers/char/tpm/tpm-chip.c:568:8: error: too few arguments to
> function ‘tpm_request_locality’
>   ret = tpm_request_locality(chip);
>         ^~~~~~~~~~~~~~~~~~~~
> drivers/char/tpm/tpm-chip.c:43:12: note: declared here
>  static int tpm_request_locality(struct tpm_chip *chip, int locality)
>             ^~~~~~~~~~~~~~~~~~~~
> 
> Fix this by requesting zero locality.

Actually, this is a bad interaction with the non-upstream patch to run
the kernel in locality two to allow key policy to distinguish kernel
release from user space release, which goes back to the debate over
hibernation keys.  I'll carry it separately until (or if ever) we get a
resolution on how to do this.

James
Jarkko Sakkinen March 20, 2023, 1:37 p.m. UTC | #2 
On Mon, Mar 20, 2023 at 07:15:52AM -0400, James Bottomley wrote:
> The test for the AMD fTPM problem, which just went in, actually uses
> the wrong function template for request_locality().  It's missing an
> argument so the build breaks:
> 
> drivers/char/tpm/tpm-chip.c:568:8: error: too few arguments to function ‘tpm_request_locality’
>   ret = tpm_request_locality(chip);
>         ^~~~~~~~~~~~~~~~~~~~
> drivers/char/tpm/tpm-chip.c:43:12: note: declared here
>  static int tpm_request_locality(struct tpm_chip *chip, int locality)
>             ^~~~~~~~~~~~~~~~~~~~
> 
> Fix this by requesting zero locality.
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Fixes: f1324bbc4011 ("tpm: disable hwrng for fTPM on some AMD designs")
> 
> ---
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index c04d101c7779..fee061780468 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -565,7 +565,7 @@ static bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
>  	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
>  		return false;
>  
> -	ret = tpm_request_locality(chip);
> +	ret = tpm_request_locality(chip, 0);
>  	if (ret)
>  		return false;
>  
> 

Thank you, I sent a PR for rc4.

BR, Jarkko
Jarkko Sakkinen March 20, 2023, 1:47 p.m. UTC | #3 
On Mon, Mar 20, 2023 at 07:22:52AM -0400, James Bottomley wrote:
> On Mon, 2023-03-20 at 07:15 -0400, James Bottomley wrote:
> > The test for the AMD fTPM problem, which just went in, actually uses
> > the wrong function template for request_locality().  It's missing an
> > argument so the build breaks:
> > 
> > drivers/char/tpm/tpm-chip.c:568:8: error: too few arguments to
> > function ‘tpm_request_locality’
> >   ret = tpm_request_locality(chip);
> >         ^~~~~~~~~~~~~~~~~~~~
> > drivers/char/tpm/tpm-chip.c:43:12: note: declared here
> >  static int tpm_request_locality(struct tpm_chip *chip, int locality)
> >             ^~~~~~~~~~~~~~~~~~~~
> > 
> > Fix this by requesting zero locality.
> 
> Actually, this is a bad interaction with the non-upstream patch to run
> the kernel in locality two to allow key policy to distinguish kernel
> release from user space release, which goes back to the debate over
> hibernation keys.  I'll carry it separately until (or if ever) we get a
> resolution on how to do this.

BTW, do you have a newer version of

https://lore.kernel.org/linux-integrity/20230216201410.15010-1-James.Bottomley@HansenPartnership.com/

I'm planning to flush testing queue as I have now more bandwidth
for TPM and keyring (actually I'm looking RISC-V fTPM's at work).

BR, Jarkko

BR, Jarkko
James Bottomley March 23, 2023, 12:20 p.m. UTC | #4 
On Mon, 2023-03-20 at 15:47 +0200, Jarkko Sakkinen wrote:
> On Mon, Mar 20, 2023 at 07:22:52AM -0400, James Bottomley wrote:
> > On Mon, 2023-03-20 at 07:15 -0400, James Bottomley wrote:
> > > The test for the AMD fTPM problem, which just went in, actually
> > > uses the wrong function template for request_locality().  It's
> > > missing an argument so the build breaks:
> > > 
> > > drivers/char/tpm/tpm-chip.c:568:8: error: too few arguments to
> > > function ‘tpm_request_locality’
> > >   ret = tpm_request_locality(chip);
> > >         ^~~~~~~~~~~~~~~~~~~~
> > > drivers/char/tpm/tpm-chip.c:43:12: note: declared here
> > >  static int tpm_request_locality(struct tpm_chip *chip, int
> > > locality)
> > >             ^~~~~~~~~~~~~~~~~~~~
> > > 
> > > Fix this by requesting zero locality.
> > 
> > Actually, this is a bad interaction with the non-upstream patch to
> > run the kernel in locality two to allow key policy to distinguish
> > kernel release from user space release, which goes back to the
> > debate over hibernation keys.  I'll carry it separately until (or
> > if ever) we get a resolution on how to do this.
> 
> BTW, do you have a newer version of
> 
> https://lore.kernel.org/linux-integrity/20230216201410.15010-1-James.Bottomley@HansenPartnership.com/
> 
> I'm planning to flush testing queue as I have now more bandwidth
> for TPM and keyring (actually I'm looking RISC-V fTPM's at work).

Hopefully next week.  I'm on a business trip and conference this week,
so most of my cycles have been going into that and converting the TPM2
engine to a provider, but I'm back home next week and the provider
conversion is pretty much done.

Regards,

James
Jarkko Sakkinen March 29, 2023, 11:52 p.m. UTC | #5 
On Thu, Mar 23, 2023 at 08:20:29AM -0400, James Bottomley wrote:
> On Mon, 2023-03-20 at 15:47 +0200, Jarkko Sakkinen wrote:
> > On Mon, Mar 20, 2023 at 07:22:52AM -0400, James Bottomley wrote:
> > > On Mon, 2023-03-20 at 07:15 -0400, James Bottomley wrote:
> > > > The test for the AMD fTPM problem, which just went in, actually
> > > > uses the wrong function template for request_locality().  It's
> > > > missing an argument so the build breaks:
> > > > 
> > > > drivers/char/tpm/tpm-chip.c:568:8: error: too few arguments to
> > > > function ‘tpm_request_locality’
> > > >   ret = tpm_request_locality(chip);
> > > >         ^~~~~~~~~~~~~~~~~~~~
> > > > drivers/char/tpm/tpm-chip.c:43:12: note: declared here
> > > >  static int tpm_request_locality(struct tpm_chip *chip, int
> > > > locality)
> > > >             ^~~~~~~~~~~~~~~~~~~~
> > > > 
> > > > Fix this by requesting zero locality.
> > > 
> > > Actually, this is a bad interaction with the non-upstream patch to
> > > run the kernel in locality two to allow key policy to distinguish
> > > kernel release from user space release, which goes back to the
> > > debate over hibernation keys.  I'll carry it separately until (or
> > > if ever) we get a resolution on how to do this.
> > 
> > BTW, do you have a newer version of
> > 
> > https://lore.kernel.org/linux-integrity/20230216201410.15010-1-James.Bottomley@HansenPartnership.com/
> > 
> > I'm planning to flush testing queue as I have now more bandwidth
> > for TPM and keyring (actually I'm looking RISC-V fTPM's at work).
> 
> Hopefully next week.  I'm on a business trip and conference this week,
> so most of my cycles have been going into that and converting the TPM2
> engine to a provider, but I'm back home next week and the provider
> conversion is pretty much done.

Yeah, as said in other reponse things have not moved because I got sick
on Friday... Anyway, I'm planning to test the current version, unless
new comes available before I get on it.

BR, Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index c04d101c7779..fee061780468 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -565,7 +565,7 @@  static bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
 	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
 		return false;
 
-	ret = tpm_request_locality(chip);
+	ret = tpm_request_locality(chip, 0);
 	if (ret)
 		return false;