diff mbox series

[v2] tpm: Disable RNG for all AMD fTPMs

Message ID 20230802122533.19508-1-mario.limonciello@amd.com (mailing list archive)
State New, archived
Headers show
Series [v2] tpm: Disable RNG for all AMD fTPMs | expand

Commit Message

Mario Limonciello Aug. 2, 2023, 12:25 p.m. UTC
The TPM RNG functionality is not necessary for entropy when the CPU
already supports the RDRAND instruction. The TPM RNG functionality
was previously disabled on a subset of AMD fTPM series, but reports
continue to show problems on some systems causing stutter root caused
to TPM RNG functionality.

Expand disabling TPM RNG use for all AMD fTPMs whether they have versions
that claim to have fixed or not. To accomplish this, move the detection
into part of the TPM CRB registration and add a flag indicating that
the TPM should opt-out of registration to hwrng.

Cc: stable@vger.kernel.org # 6.1.y+
Fixes: b006c439d58d ("hwrng: core - start hwrng kthread also for untrusted sources")
Fixes: f1324bbc4011 ("tpm: disable hwrng for fTPM on some AMD designs")
Reported-by: daniil.stas@posteo.net
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217719
Reported-by: bitlord0xff@gmail.com
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217212
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * switch from callback to everything in tpm_crb
 * switch to open coded flags check instead of new inline
---
 drivers/char/tpm/tpm-chip.c | 68 ++-----------------------------------
 drivers/char/tpm/tpm_crb.c  | 30 ++++++++++++++++
 include/linux/tpm.h         |  1 +
 3 files changed, 33 insertions(+), 66 deletions(-)

Comments

Jarkko Sakkinen Aug. 2, 2023, 2:40 p.m. UTC | #1
On Wed Aug 2, 2023 at 3:25 PM EEST, Mario Limonciello wrote:
> The TPM RNG functionality is not necessary for entropy when the CPU
> already supports the RDRAND instruction. The TPM RNG functionality
> was previously disabled on a subset of AMD fTPM series, but reports
> continue to show problems on some systems causing stutter root caused
> to TPM RNG functionality.
>
> Expand disabling TPM RNG use for all AMD fTPMs whether they have versions
> that claim to have fixed or not. To accomplish this, move the detection
> into part of the TPM CRB registration and add a flag indicating that
> the TPM should opt-out of registration to hwrng.
>
> Cc: stable@vger.kernel.org # 6.1.y+
> Fixes: b006c439d58d ("hwrng: core - start hwrng kthread also for untrusted sources")
> Fixes: f1324bbc4011 ("tpm: disable hwrng for fTPM on some AMD designs")
> Reported-by: daniil.stas@posteo.net
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217719
> Reported-by: bitlord0xff@gmail.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217212
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>  * switch from callback to everything in tpm_crb
>  * switch to open coded flags check instead of new inline
> ---
>  drivers/char/tpm/tpm-chip.c | 68 ++-----------------------------------
>  drivers/char/tpm/tpm_crb.c  | 30 ++++++++++++++++
>  include/linux/tpm.h         |  1 +
>  3 files changed, 33 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index cf5499e51999b..e904aae9771be 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -510,70 +510,6 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
>  	return 0;
>  }
>  
> -/*
> - * Some AMD fTPM versions may cause stutter
> - * https://www.amd.com/en/support/kb/faq/pa-410
> - *
> - * Fixes are available in two series of fTPM firmware:
> - * 6.x.y.z series: 6.0.18.6 +
> - * 3.x.y.z series: 3.57.y.5 +
> - */
> -#ifdef CONFIG_X86
> -static bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
> -{
> -	u32 val1, val2;
> -	u64 version;
> -	int ret;
> -
> -	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> -		return false;
> -
> -	ret = tpm_request_locality(chip);
> -	if (ret)
> -		return false;
> -
> -	ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL);
> -	if (ret)
> -		goto release;
> -	if (val1 != 0x414D4400U /* AMD */) {
> -		ret = -ENODEV;
> -		goto release;
> -	}
> -	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL);
> -	if (ret)
> -		goto release;
> -	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL);
> -
> -release:
> -	tpm_relinquish_locality(chip);
> -
> -	if (ret)
> -		return false;
> -
> -	version = ((u64)val1 << 32) | val2;
> -	if ((version >> 48) == 6) {
> -		if (version >= 0x0006000000180006ULL)
> -			return false;
> -	} else if ((version >> 48) == 3) {
> -		if (version >= 0x0003005700000005ULL)
> -			return false;
> -	} else {
> -		return false;
> -	}
> -
> -	dev_warn(&chip->dev,
> -		 "AMD fTPM version 0x%llx causes system stutter; hwrng disabled\n",
> -		 version);
> -
> -	return true;
> -}
> -#else
> -static inline bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
> -{
> -	return false;
> -}
> -#endif /* CONFIG_X86 */
> -
>  static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>  {
>  	struct tpm_chip *chip = container_of(rng, struct tpm_chip, hwrng);
> @@ -588,7 +524,7 @@ 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 (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> -	    tpm_amd_is_rng_defective(chip))
> +	    chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
>  		return 0;
>  
>  	snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
> @@ -719,7 +655,7 @@ 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) &&
> -	    !tpm_amd_is_rng_defective(chip))
> +	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
>  		hwrng_unregister(&chip->hwrng);
>  	tpm_bios_log_teardown(chip);
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip))
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 1a5d09b185134..9eb1a18590123 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -463,6 +463,28 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
>  	return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
>  }
>  
> +static int crb_check_flags(struct tpm_chip *chip)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = crb_request_locality(chip, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val, NULL);
> +	if (ret)
> +		goto release;
> +
> +	if (val == 0x414D4400U /* AMD */)
> +		chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
> +
> +release:
> +	crb_relinquish_locality(chip, 0);
> +
> +	return ret;
> +}
> +
>  static const struct tpm_class_ops tpm_crb = {
>  	.flags = TPM_OPS_AUTO_STARTUP,
>  	.status = crb_status,
> @@ -800,6 +822,14 @@ static int crb_acpi_add(struct acpi_device *device)
>  	chip->acpi_dev_handle = device->handle;
>  	chip->flags = TPM_CHIP_FLAG_TPM2;
>  
> +	rc = tpm_chip_bootstrap(chip);
> +	if (rc)
> +		goto out;
> +
> +	rc = crb_check_flags(chip);
> +	if (rc)
> +		goto out;
> +
>  	rc = tpm_chip_register(chip);
>  
>  out:
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 6a1e8f1572551..4ee9d13749adc 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -283,6 +283,7 @@ enum tpm_chip_flags {
>  	TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED	= BIT(6),
>  	TPM_CHIP_FLAG_FIRMWARE_UPGRADE		= BIT(7),
>  	TPM_CHIP_FLAG_SUSPENDED			= BIT(8),
> +	TPM_CHIP_FLAG_HWRNG_DISABLED		= BIT(9),
>  };
>  
>  #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> -- 
> 2.34.1

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

Thank you, great work.

I pushed the patch to my next branch:

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/?h=next

I'll hold on for tested-by's from AMD users, and send a pull
request tomorrow afternoon (GMT+3).

BR, Jarkko
Jason A. Donenfeld Aug. 3, 2023, 1:45 p.m. UTC | #2
On Wed, Aug 02, 2023 at 07:25:33AM -0500, Mario Limonciello wrote:
> The TPM RNG functionality is not necessary for entropy when the CPU
> already supports the RDRAND instruction. The TPM RNG functionality
> was previously disabled on a subset of AMD fTPM series, but reports
> continue to show problems on some systems causing stutter root caused
> to TPM RNG functionality.
> 
> Expand disabling TPM RNG use for all AMD fTPMs whether they have versions
> that claim to have fixed or not. To accomplish this, move the detection
> into part of the TPM CRB registration and add a flag indicating that
> the TPM should opt-out of registration to hwrng.
> 
> Cc: stable@vger.kernel.org # 6.1.y+
> Fixes: b006c439d58d ("hwrng: core - start hwrng kthread also for untrusted sources")
> Fixes: f1324bbc4011 ("tpm: disable hwrng for fTPM on some AMD designs")

Users can trigger this by reading from /dev/hwrng, which some userspace
daemons are known to do. So I think the proper fixes tag and stable@
version range actually begins with whenever fTPM support was introduced.

> Reported-by: daniil.stas@posteo.net
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217719
> Reported-by: bitlord0xff@gmail.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217212
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Hopefully future AMD hardware won't be broken and we can revisit this.
Until then, LGTM:

Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Jarkko Sakkinen Aug. 4, 2023, 8:35 p.m. UTC | #3
On Thu Aug 3, 2023 at 4:45 PM EEST, Jason A. Donenfeld wrote:
> On Wed, Aug 02, 2023 at 07:25:33AM -0500, Mario Limonciello wrote:
> > The TPM RNG functionality is not necessary for entropy when the CPU
> > already supports the RDRAND instruction. The TPM RNG functionality
> > was previously disabled on a subset of AMD fTPM series, but reports
> > continue to show problems on some systems causing stutter root caused
> > to TPM RNG functionality.
> > 
> > Expand disabling TPM RNG use for all AMD fTPMs whether they have versions
> > that claim to have fixed or not. To accomplish this, move the detection
> > into part of the TPM CRB registration and add a flag indicating that
> > the TPM should opt-out of registration to hwrng.
> > 
> > Cc: stable@vger.kernel.org # 6.1.y+
> > Fixes: b006c439d58d ("hwrng: core - start hwrng kthread also for untrusted sources")
> > Fixes: f1324bbc4011 ("tpm: disable hwrng for fTPM on some AMD designs")
>
> Users can trigger this by reading from /dev/hwrng, which some userspace
> daemons are known to do. So I think the proper fixes tag and stable@
> version range actually begins with whenever fTPM support was introduced.
>
> > Reported-by: daniil.stas@posteo.net
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217719
> > Reported-by: bitlord0xff@gmail.com
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217212
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>
> Hopefully future AMD hardware won't be broken and we can revisit this.
> Until then, LGTM:
>
> Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>

Thanks. I'll add your tag.

BR, Jarkko
Jarkko Sakkinen Aug. 4, 2023, 11:08 p.m. UTC | #4
On Fri Aug 4, 2023 at 11:41 PM EEST, Jason A. Donenfeld wrote:
> Please don't. Instead take Mario's v3:
> https://lore.kernel.org/all/20230803182428.25753-1-mario.limonciello@amd.com/

Sure! I noticed it.

BR, Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index cf5499e51999b..e904aae9771be 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -510,70 +510,6 @@  static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
 	return 0;
 }
 
-/*
- * Some AMD fTPM versions may cause stutter
- * https://www.amd.com/en/support/kb/faq/pa-410
- *
- * Fixes are available in two series of fTPM firmware:
- * 6.x.y.z series: 6.0.18.6 +
- * 3.x.y.z series: 3.57.y.5 +
- */
-#ifdef CONFIG_X86
-static bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
-{
-	u32 val1, val2;
-	u64 version;
-	int ret;
-
-	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
-		return false;
-
-	ret = tpm_request_locality(chip);
-	if (ret)
-		return false;
-
-	ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL);
-	if (ret)
-		goto release;
-	if (val1 != 0x414D4400U /* AMD */) {
-		ret = -ENODEV;
-		goto release;
-	}
-	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL);
-	if (ret)
-		goto release;
-	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL);
-
-release:
-	tpm_relinquish_locality(chip);
-
-	if (ret)
-		return false;
-
-	version = ((u64)val1 << 32) | val2;
-	if ((version >> 48) == 6) {
-		if (version >= 0x0006000000180006ULL)
-			return false;
-	} else if ((version >> 48) == 3) {
-		if (version >= 0x0003005700000005ULL)
-			return false;
-	} else {
-		return false;
-	}
-
-	dev_warn(&chip->dev,
-		 "AMD fTPM version 0x%llx causes system stutter; hwrng disabled\n",
-		 version);
-
-	return true;
-}
-#else
-static inline bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
-{
-	return false;
-}
-#endif /* CONFIG_X86 */
-
 static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 {
 	struct tpm_chip *chip = container_of(rng, struct tpm_chip, hwrng);
@@ -588,7 +524,7 @@  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 (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
-	    tpm_amd_is_rng_defective(chip))
+	    chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
 		return 0;
 
 	snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
@@ -719,7 +655,7 @@  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) &&
-	    !tpm_amd_is_rng_defective(chip))
+	    !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED))
 		hwrng_unregister(&chip->hwrng);
 	tpm_bios_log_teardown(chip);
 	if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip))
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 1a5d09b185134..9eb1a18590123 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -463,6 +463,28 @@  static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
 	return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
 }
 
+static int crb_check_flags(struct tpm_chip *chip)
+{
+	u32 val;
+	int ret;
+
+	ret = crb_request_locality(chip, 0);
+	if (ret)
+		return ret;
+
+	ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val, NULL);
+	if (ret)
+		goto release;
+
+	if (val == 0x414D4400U /* AMD */)
+		chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
+
+release:
+	crb_relinquish_locality(chip, 0);
+
+	return ret;
+}
+
 static const struct tpm_class_ops tpm_crb = {
 	.flags = TPM_OPS_AUTO_STARTUP,
 	.status = crb_status,
@@ -800,6 +822,14 @@  static int crb_acpi_add(struct acpi_device *device)
 	chip->acpi_dev_handle = device->handle;
 	chip->flags = TPM_CHIP_FLAG_TPM2;
 
+	rc = tpm_chip_bootstrap(chip);
+	if (rc)
+		goto out;
+
+	rc = crb_check_flags(chip);
+	if (rc)
+		goto out;
+
 	rc = tpm_chip_register(chip);
 
 out:
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 6a1e8f1572551..4ee9d13749adc 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -283,6 +283,7 @@  enum tpm_chip_flags {
 	TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED	= BIT(6),
 	TPM_CHIP_FLAG_FIRMWARE_UPGRADE		= BIT(7),
 	TPM_CHIP_FLAG_SUSPENDED			= BIT(8),
+	TPM_CHIP_FLAG_HWRNG_DISABLED		= BIT(9),
 };
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)