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

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

[James Bottomley]

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;
 

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

[James Bottomley]

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

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

[Jarkko Sakkinen]

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

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

[Jarkko Sakkinen]

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

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

[James Bottomley]

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

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

[Jarkko Sakkinen]

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