[PATCH 0/3] More changes related to TPM RNG handling

[PATCH 0/3] More changes related to TPM RNG handling

[Mario Limonciello]

Patch 1 fixes a mistake in error handling path introduced in
previous patch.  It should fold in previous patch or go to -fixes.

Patches 2 and 3 add command line option and drop Kconfig.
These should be targeted for -next.
Mario Limonciello (3):
  tpm: Add a missing check for TPM_CHIP_FLAG_HWRNG_DISABLED
  tpm: Add command line for not trusting tpm for RNG
  tpm: Drop CONFIG_HW_RANDOM_TPM

 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 drivers/char/tpm/Kconfig                        | 11 -----------
 drivers/char/tpm/tpm-chip.c                     | 17 ++++++++++++++---
 3 files changed, 19 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH 1/3] tpm: Add a missing check for TPM_CHIP_FLAG_HWRNG_DISABLED

[Mario Limonciello]

If the TPM is opted out of hwrng the error handling for
tpm_chip_register() needs to know this so it doesn't try to clean
up an uninitialized chip->hwrng.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/char/tpm/tpm-chip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index e904aae9771be..8f61b784810d6 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -629,7 +629,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 	return 0;
 
 out_hwrng:
-	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip))
+	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
+	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
 		hwrng_unregister(&chip->hwrng);
 out_ppi:
 	tpm_bios_log_teardown(chip);
-- 
2.34.1

[PATCH 2/3] tpm: Add command line for not trusting tpm for RNG

[Mario Limonciello]

The kernel supports random.cpu=off and random.bootloader=off.
As TPM RNG is also registered as a hwrng, add the ability to
prevent registering the TPM RNG.

Suggested-by: Mateusz Schyboll <dragonn@op.pl>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 drivers/char/tpm/tpm-chip.c                     | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a1457995fd41c..9ff602c09f55c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4672,6 +4672,11 @@
 			passed by the bootloader (if available) to
 			initialize the kernel's RNG.
 
+	random.trust_tpm=off
+			[KNL] Disable trusting the use of the TPM's
+			random number generator (if available) to
+			initialize the kernel's RNG.
+
 	randomize_kstack_offset=
 			[KNL] Enable or disable kernel stack offset
 			randomization, which provides roughly 5 bits of
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 8f61b784810d6..8fb42232bd7a5 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -32,6 +32,13 @@ struct class *tpm_class;
 struct class *tpmrm_class;
 dev_t tpm_devt;
 
+static bool trust_tpm __initdata = true;
+static int __init parse_trust_tpm(char *arg)
+{
+	return kstrtobool(arg, &trust_tpm);
+}
+early_param("random.trust_tpm", parse_trust_tpm);
+
 static int tpm_request_locality(struct tpm_chip *chip)
 {
 	int rc;
@@ -523,6 +530,9 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 
 static int tpm_add_hwrng(struct tpm_chip *chip)
 {
+	if (!trust_tpm)
+		chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
+
 	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
 	    chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
 		return 0;
-- 
2.34.1

[PATCH 3/3] tpm: Drop CONFIG_HW_RANDOM_TPM

[Mario Limonciello]

As the behavior of whether a TPM is registered for hwrng can be controlled
by command line, drop the kernel configuration option.

Cc: Mateusz Schyboll <dragonn@op.pl>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/char/tpm/Kconfig    | 11 -----------
 drivers/char/tpm/tpm-chip.c |  6 +++---
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 927088b2c3d3f..69aaa730dc208 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -27,17 +27,6 @@ menuconfig TCG_TPM
 
 if TCG_TPM
 
-config HW_RANDOM_TPM
-	bool "TPM HW Random Number Generator support"
-	depends on TCG_TPM && HW_RANDOM && !(TCG_TPM=y && HW_RANDOM=m)
-	default y
-	help
-	  This setting exposes the TPM's Random Number Generator as a hwrng
-	  device. This allows the kernel to collect randomness from the TPM at
-	  boot, and provides the TPM randomines in /dev/hwrng.
-
-	  If unsure, say Y.
-
 config TCG_TIS_CORE
 	tristate
 	help
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 8fb42232bd7a5..0d69335743469 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -533,7 +533,7 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
 	if (!trust_tpm)
 		chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
 
-	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
+	if (tpm_is_firmware_upgrade(chip) ||
 	    chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
 		return 0;
 
@@ -639,7 +639,7 @@ int tpm_chip_register(struct tpm_chip *chip)
 	return 0;
 
 out_hwrng:
-	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
+	if (!tpm_is_firmware_upgrade(chip) &&
 	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
 		hwrng_unregister(&chip->hwrng);
 out_ppi:
@@ -665,7 +665,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
 void tpm_chip_unregister(struct tpm_chip *chip)
 {
 	tpm_del_legacy_sysfs(chip);
-	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
+	if (!tpm_is_firmware_upgrade(chip) &&
 	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
 		hwrng_unregister(&chip->hwrng);
 	tpm_bios_log_teardown(chip);
-- 
2.34.1

Re: [PATCH 3/3] tpm: Drop CONFIG_HW_RANDOM_TPM

[Paul Menzel]

Dear Mario,


Thank you for the patch.

Am 03.08.23 um 03:50 schrieb Mario Limonciello:
> As the behavior of whether a TPM is registered for hwrng can be controlled
> by command line, drop the kernel configuration option.

Shouldn’t this be left in to be able to set the default without having 
to change the Linux kernel command line?


Kind regards

Paul


> Cc: Mateusz Schyboll <dragonn@op.pl>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/char/tpm/Kconfig    | 11 -----------
>   drivers/char/tpm/tpm-chip.c |  6 +++---
>   2 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 927088b2c3d3f..69aaa730dc208 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -27,17 +27,6 @@ menuconfig TCG_TPM
>   
>   if TCG_TPM
>   
> -config HW_RANDOM_TPM
> -	bool "TPM HW Random Number Generator support"
> -	depends on TCG_TPM && HW_RANDOM && !(TCG_TPM=y && HW_RANDOM=m)
> -	default y
> -	help
> -	  This setting exposes the TPM's Random Number Generator as a hwrng
> -	  device. This allows the kernel to collect randomness from the TPM at
> -	  boot, and provides the TPM randomines in /dev/hwrng.
> -
> -	  If unsure, say Y.
> -
>   config TCG_TIS_CORE
>   	tristate
>   	help
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 8fb42232bd7a5..0d69335743469 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -533,7 +533,7 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
>   	if (!trust_tpm)
>   		chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
>   
> -	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> +	if (tpm_is_firmware_upgrade(chip) ||
>   	    chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
>   		return 0;
>   
> @@ -639,7 +639,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>   	return 0;
>   
>   out_hwrng:
> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
> +	if (!tpm_is_firmware_upgrade(chip) &&
>   	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>   		hwrng_unregister(&chip->hwrng);
>   out_ppi:
> @@ -665,7 +665,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
>   void tpm_chip_unregister(struct tpm_chip *chip)
>   {
>   	tpm_del_legacy_sysfs(chip);
> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
> +	if (!tpm_is_firmware_upgrade(chip) &&
>   	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>   		hwrng_unregister(&chip->hwrng);
>   	tpm_bios_log_teardown(chip);

Re: [PATCH 1/3] tpm: Add a missing check for TPM_CHIP_FLAG_HWRNG_DISABLED

[Jarkko Sakkinen]

On Thu Aug 3, 2023 at 4:50 AM EEST, Mario Limonciello wrote:
> If the TPM is opted out of hwrng the error handling for
> tpm_chip_register() needs to know this so it doesn't try to clean
> up an uninitialized chip->hwrng.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/char/tpm/tpm-chip.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index e904aae9771be..8f61b784810d6 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -629,7 +629,8 @@ int tpm_chip_register(struct tpm_chip *chip)
>  	return 0;
>  
>  out_hwrng:
> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip))
> +	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
> +	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>  		hwrng_unregister(&chip->hwrng);
>  out_ppi:
>  	tpm_bios_log_teardown(chip);
> -- 
> 2.34.1

Please add a fixes tag.

BR, Jarkko

Re: [PATCH 3/3] tpm: Drop CONFIG_HW_RANDOM_TPM

[Jarkko Sakkinen]

On Thu Aug 3, 2023 at 4:50 AM EEST, Mario Limonciello wrote:
> As the behavior of whether a TPM is registered for hwrng can be controlled
> by command line, drop the kernel configuration option.
>
> Cc: Mateusz Schyboll <dragonn@op.pl>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/char/tpm/Kconfig    | 11 -----------
>  drivers/char/tpm/tpm-chip.c |  6 +++---
>  2 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 927088b2c3d3f..69aaa730dc208 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -27,17 +27,6 @@ menuconfig TCG_TPM
>  
>  if TCG_TPM
>  
> -config HW_RANDOM_TPM
> -	bool "TPM HW Random Number Generator support"
> -	depends on TCG_TPM && HW_RANDOM && !(TCG_TPM=y && HW_RANDOM=m)
> -	default y
> -	help
> -	  This setting exposes the TPM's Random Number Generator as a hwrng
> -	  device. This allows the kernel to collect randomness from the TPM at
> -	  boot, and provides the TPM randomines in /dev/hwrng.
> -
> -	  If unsure, say Y.
> -
>  config TCG_TIS_CORE
>  	tristate
>  	help
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 8fb42232bd7a5..0d69335743469 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -533,7 +533,7 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
>  	if (!trust_tpm)
>  		chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
>  
> -	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> +	if (tpm_is_firmware_upgrade(chip) ||
>  	    chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
>  		return 0;
>  
> @@ -639,7 +639,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>  	return 0;
>  
>  out_hwrng:
> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
> +	if (!tpm_is_firmware_upgrade(chip) &&
>  	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>  		hwrng_unregister(&chip->hwrng);
>  out_ppi:
> @@ -665,7 +665,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
>  void tpm_chip_unregister(struct tpm_chip *chip)
>  {
>  	tpm_del_legacy_sysfs(chip);
> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
> +	if (!tpm_is_firmware_upgrade(chip) &&
>  	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>  		hwrng_unregister(&chip->hwrng);
>  	tpm_bios_log_teardown(chip);
> -- 
> 2.34.1

I don't understand this but please take it a way from patch set, which
should only contain critical fixes, which this definitely is not.

BR, Jarkko

Re: [PATCH 3/3] tpm: Drop CONFIG_HW_RANDOM_TPM

[Jarkko Sakkinen]

On Thu Aug 3, 2023 at 10:22 AM EEST, Paul Menzel wrote:
> Dear Mario,
>
>
> Thank you for the patch.
>
> Am 03.08.23 um 03:50 schrieb Mario Limonciello:
> > As the behavior of whether a TPM is registered for hwrng can be controlled
> > by command line, drop the kernel configuration option.
>
> Shouldn’t this be left in to be able to set the default without having 
> to change the Linux kernel command line?

Even if it made sense it is completely urelated to the real-world
issues at hand.

BR, Jarkko

Re: [PATCH 1/3] tpm: Add a missing check for TPM_CHIP_FLAG_HWRNG_DISABLED

[Mario Limonciello]

On 8/3/23 03:59, Jarkko Sakkinen wrote:
> On Thu Aug 3, 2023 at 4:50 AM EEST, Mario Limonciello wrote:
>> If the TPM is opted out of hwrng the error handling for
>> tpm_chip_register() needs to know this so it doesn't try to clean
>> up an uninitialized chip->hwrng.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/char/tpm/tpm-chip.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index e904aae9771be..8f61b784810d6 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -629,7 +629,8 @@ int tpm_chip_register(struct tpm_chip *chip)
>>   	return 0;
>>   
>>   out_hwrng:
>> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip))
>> +	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
>> +	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>>   		hwrng_unregister(&chip->hwrng);
>>   out_ppi:
>>   	tpm_bios_log_teardown(chip);
>> -- 
>> 2.34.1
> 
> Please add a fixes tag.
> 
> BR, Jarkko

I didn't add a fixes tag because you hadn't sent a PR for the other one 
yet so I wasn't sure the hash would be stable.  Also I thought it might 
just make sense to squash into it.

If the hash is now stable, could you just just commit and add that tag 
with it yourself?  Or do you want me to re-send as a v2 with that?

Re: [PATCH 3/3] tpm: Drop CONFIG_HW_RANDOM_TPM

[Mario Limonciello]

On 8/3/23 02:22, Paul Menzel wrote:
> Dear Mario,
> 
> 
> Thank you for the patch.
> 
> Am 03.08.23 um 03:50 schrieb Mario Limonciello:
>> As the behavior of whether a TPM is registered for hwrng can be 
>> controlled
>> by command line, drop the kernel configuration option.
> 
> Shouldn’t this be left in to be able to set the default without having 
> to change the Linux kernel command line?
> 
It's the same thing as these commits:

b9b01a5625b5a ("random: use random.trust_{bootloader,cpu} command line 
option only")

d97c68d178fbf ("random: treat bootloader trust toggle the same way as 
cpu trust toggle")

So it matches the behavior of those.

> 
> Kind regards
> 
> Paul
> 
> 
>> Cc: Mateusz Schyboll <dragonn@op.pl>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/char/tpm/Kconfig    | 11 -----------
>>   drivers/char/tpm/tpm-chip.c |  6 +++---
>>   2 files changed, 3 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>> index 927088b2c3d3f..69aaa730dc208 100644
>> --- a/drivers/char/tpm/Kconfig
>> +++ b/drivers/char/tpm/Kconfig
>> @@ -27,17 +27,6 @@ menuconfig TCG_TPM
>>   if TCG_TPM
>> -config HW_RANDOM_TPM
>> -    bool "TPM HW Random Number Generator support"
>> -    depends on TCG_TPM && HW_RANDOM && !(TCG_TPM=y && HW_RANDOM=m)
>> -    default y
>> -    help
>> -      This setting exposes the TPM's Random Number Generator as a hwrng
>> -      device. This allows the kernel to collect randomness from the 
>> TPM at
>> -      boot, and provides the TPM randomines in /dev/hwrng.
>> -
>> -      If unsure, say Y.
>> -
>>   config TCG_TIS_CORE
>>       tristate
>>       help
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 8fb42232bd7a5..0d69335743469 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -533,7 +533,7 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
>>       if (!trust_tpm)
>>           chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
>> -    if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || 
>> tpm_is_firmware_upgrade(chip) ||
>> +    if (tpm_is_firmware_upgrade(chip) ||
>>           chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
>>           return 0;
>> @@ -639,7 +639,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>>       return 0;
>>   out_hwrng:
>> -    if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && 
>> !tpm_is_firmware_upgrade(chip) &&
>> +    if (!tpm_is_firmware_upgrade(chip) &&
>>           !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>>           hwrng_unregister(&chip->hwrng);
>>   out_ppi:
>> @@ -665,7 +665,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
>>   void tpm_chip_unregister(struct tpm_chip *chip)
>>   {
>>       tpm_del_legacy_sysfs(chip);
>> -    if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && 
>> !tpm_is_firmware_upgrade(chip) &&
>> +    if (!tpm_is_firmware_upgrade(chip) &&
>>           !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>>           hwrng_unregister(&chip->hwrng);
>>       tpm_bios_log_teardown(chip);

Re: [PATCH 2/3] tpm: Add command line for not trusting tpm for RNG

[Jason A. Donenfeld]

On Wed, Aug 02, 2023 at 08:50:14PM -0500, Mario Limonciello wrote:
> The kernel supports random.cpu=off and random.bootloader=off.
> As TPM RNG is also registered as a hwrng, add the ability to
> prevent registering the TPM RNG.

Please do *not* do this. I agree with Jarkko that this doesn't belong.

Firstly, you're proposing a flag for the tpm driver, so the `random.`
namespace is inappropriate. Do not use the `random.` namespace if you're
not dealing with random.c specifically. Rather, this is very much a
`tpm.register_hwrng=1/0` flag, which describes better what this is about.

Secondly, I think you're making a mountain out of a molehill. You first
wanted to also disable Intel devices too, even though they aren't
affected by this bug. Now you're proposing a way for users to disable
everything. But so far there's no evidence that this matter goes any
further than AMD's fTPM. So let's calm a bit and not make too big deal
of this. If we suddenly get lots of reports that there's broken behavior
across the board, then maybe we should consider something like this. But
insofar as this is just an AMD derp, let's keep it simple and not over
complicate everything with more knobs. Fewer knobs, please!

Finally, with regards to AMD, my hope is that eventually the fTPM
becomes useful as a hwrng, and then we can relax the disabling to
re-enable it for whatever new revision might come to exist in the
future.

Thanks,
Jason

Re: [PATCH 1/3] tpm: Add a missing check for TPM_CHIP_FLAG_HWRNG_DISABLED

[Jason A. Donenfeld]

On Thu, Aug 03, 2023 at 06:35:36AM -0500, Mario Limonciello wrote:
> On 8/3/23 03:59, Jarkko Sakkinen wrote:
> > On Thu Aug 3, 2023 at 4:50 AM EEST, Mario Limonciello wrote:
> >> If the TPM is opted out of hwrng the error handling for
> >> tpm_chip_register() needs to know this so it doesn't try to clean
> >> up an uninitialized chip->hwrng.
> >>
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >> ---
> >>   drivers/char/tpm/tpm-chip.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >> index e904aae9771be..8f61b784810d6 100644
> >> --- a/drivers/char/tpm/tpm-chip.c
> >> +++ b/drivers/char/tpm/tpm-chip.c
> >> @@ -629,7 +629,8 @@ int tpm_chip_register(struct tpm_chip *chip)
> >>   	return 0;
> >>   
> >>   out_hwrng:
> >> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip))
> >> +	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
> >> +	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
> >>   		hwrng_unregister(&chip->hwrng);
> >>   out_ppi:
> >>   	tpm_bios_log_teardown(chip);
> >> -- 
> >> 2.34.1
> > 
> > Please add a fixes tag.
> > 
> > BR, Jarkko
> 
> I didn't add a fixes tag because you hadn't sent a PR for the other one 
> yet so I wasn't sure the hash would be stable.  Also I thought it might 
> just make sense to squash into it.
> 
> If the hash is now stable, could you just just commit and add that tag 
> with it yourself?  Or do you want me to re-send as a v2 with that?

What about just sending a v3 of the patch that this patch fixes? The
stable@/fixes tags in that are wrong/incomplete so Jarkko's tree will
need to be fixed before pushing to Linus anyway.

Jason

Re: [PATCH 1/3] tpm: Add a missing check for TPM_CHIP_FLAG_HWRNG_DISABLED

[Jarkko Sakkinen]

On Thu Aug 3, 2023 at 4:50 PM EEST, Jason A. Donenfeld wrote:
> On Thu, Aug 03, 2023 at 06:35:36AM -0500, Mario Limonciello wrote:
> > On 8/3/23 03:59, Jarkko Sakkinen wrote:
> > > On Thu Aug 3, 2023 at 4:50 AM EEST, Mario Limonciello wrote:
> > >> If the TPM is opted out of hwrng the error handling for
> > >> tpm_chip_register() needs to know this so it doesn't try to clean
> > >> up an uninitialized chip->hwrng.
> > >>
> > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > >> ---
> > >>   drivers/char/tpm/tpm-chip.c | 3 ++-
> > >>   1 file changed, 2 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > >> index e904aae9771be..8f61b784810d6 100644
> > >> --- a/drivers/char/tpm/tpm-chip.c
> > >> +++ b/drivers/char/tpm/tpm-chip.c
> > >> @@ -629,7 +629,8 @@ int tpm_chip_register(struct tpm_chip *chip)
> > >>   	return 0;
> > >>   
> > >>   out_hwrng:
> > >> -	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip))
> > >> +	if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) &&
> > >> +	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
> > >>   		hwrng_unregister(&chip->hwrng);
> > >>   out_ppi:
> > >>   	tpm_bios_log_teardown(chip);
> > >> -- 
> > >> 2.34.1
> > > 
> > > Please add a fixes tag.
> > > 
> > > BR, Jarkko
> > 
> > I didn't add a fixes tag because you hadn't sent a PR for the other one 
> > yet so I wasn't sure the hash would be stable.  Also I thought it might 
> > just make sense to squash into it.
> > 
> > If the hash is now stable, could you just just commit and add that tag 
> > with it yourself?  Or do you want me to re-send as a v2 with that?
>
> What about just sending a v3 of the patch that this patch fixes? The
> stable@/fixes tags in that are wrong/incomplete so Jarkko's tree will
> need to be fixed before pushing to Linus anyway.

Sounds reasonable. I can hold the PR to rc6 and send it on Monday.

BR, Jarkko