All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs
@ 2023-02-09 15:31 Jason A. Donenfeld
  2023-02-09 15:41 ` Limonciello, Mario
  2023-02-10  0:54 ` Jarkko Sakkinen
  0 siblings, 2 replies; 6+ messages in thread
From: Jason A. Donenfeld @ 2023-02-09 15:31 UTC (permalink / raw)
  To: regressions, linux-integrity, linux-kernel
  Cc: Jason A. Donenfeld, Mario Limonciello, Jarkko Sakkinen,
	Thorsten Leemhuis, James Bottomley

Do not register a hwrng for certain AMD TPMs that are running an old
known-buggy revision. Do this by probing the TPM2_PT_MANUFACTURER,
TPM2_PT_FIRMWARE_VERSION_1, and TPM2_PT_FIRMWARE_VERSION_2 properties,
and failing when an "AMD"-manufactured TPM2 chip is below a threshold.

BROKEN BROKEN BROKEN - I just made the version numbers up and haven't
tested this because I don't actually have hardware for it. I'm posting
this so that Mario can take over its development and submit a v2 himself
once he has confirmed the versioning info from inside AMD.

Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Thorsten Leemhuis <regressions@leemhuis.info>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/char/tpm/tpm-chip.c | 34 ++++++++++++++++-
 drivers/char/tpm/tpm.h      | 73 +++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 741d8f3e8fb3..e0f8134d31a3 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -512,6 +512,37 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
 	return 0;
 }
 
+static bool tpm_is_rng_defective(struct tpm_chip *chip)
+{
+	int ret;
+	u32 val1, val2;
+	u64 version;
+
+	/* No known-broken TPM1 chips. */
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+		return false;
+
+	/* Only known-broken are AMD. */
+	ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL);
+	if (ret < 0 || val1 != 0x414D4400U /* AMD */)
+		return false;
+
+	/* Grab and concat the version values. */
+	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL);
+	if (ret < 0)
+		return false;
+	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL);
+	if (ret < 0)
+		return false;
+	version = ((u64)val1 << 32) | val2;
+
+	/* Versions below 3.4e.2.9 are broken. */
+	if (version < 0x0003004E0002009ULL)
+		return true;
+
+	return false;
+}
+
 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);
@@ -521,7 +552,8 @@ 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))
+	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
+	    tpm_is_rng_defective(chip))
 		return 0;
 
 	snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 24ee4e1cc452..830014a26609 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -150,6 +150,79 @@ enum tpm_sub_capabilities {
 	TPM_CAP_PROP_TIS_DURATION = 0x120,
 };
 
+enum tpm2_pt_props {
+	TPM2_PT_NONE = 0x00000000,
+	TPM2_PT_GROUP = 0x00000100,
+	TPM2_PT_FIXED = TPM2_PT_GROUP * 1,
+	TPM2_PT_FAMILY_INDICATOR = TPM2_PT_FIXED + 0,
+	TPM2_PT_LEVEL = TPM2_PT_FIXED + 1,
+	TPM2_PT_REVISION = TPM2_PT_FIXED + 2,
+	TPM2_PT_DAY_OF_YEAR = TPM2_PT_FIXED + 3,
+	TPM2_PT_YEAR = TPM2_PT_FIXED + 4,
+	TPM2_PT_MANUFACTURER = TPM2_PT_FIXED + 5,
+	TPM2_PT_VENDOR_STRING_1 = TPM2_PT_FIXED + 6,
+	TPM2_PT_VENDOR_STRING_2 = TPM2_PT_FIXED + 7,
+	TPM2_PT_VENDOR_STRING_3 = TPM2_PT_FIXED + 8,
+	TPM2_PT_VENDOR_STRING_4 = TPM2_PT_FIXED + 9,
+	TPM2_PT_VENDOR_TPM_TYPE = TPM2_PT_FIXED + 10,
+	TPM2_PT_FIRMWARE_VERSION_1 = TPM2_PT_FIXED + 11,
+	TPM2_PT_FIRMWARE_VERSION_2 = TPM2_PT_FIXED + 12,
+	TPM2_PT_INPUT_BUFFER = TPM2_PT_FIXED + 13,
+	TPM2_PT_HR_TRANSIENT_MIN = TPM2_PT_FIXED + 14,
+	TPM2_PT_HR_PERSISTENT_MIN = TPM2_PT_FIXED + 15,
+	TPM2_PT_HR_LOADED_MIN = TPM2_PT_FIXED + 16,
+	TPM2_PT_ACTIVE_SESSIONS_MAX = TPM2_PT_FIXED + 17,
+	TPM2_PT_PCR_COUNT = TPM2_PT_FIXED + 18,
+	TPM2_PT_PCR_SELECT_MIN = TPM2_PT_FIXED + 19,
+	TPM2_PT_CONTEXT_GAP_MAX = TPM2_PT_FIXED + 20,
+	TPM2_PT_NV_COUNTERS_MAX = TPM2_PT_FIXED + 22,
+	TPM2_PT_NV_INDEX_MAX = TPM2_PT_FIXED + 23,
+	TPM2_PT_MEMORY = TPM2_PT_FIXED + 24,
+	TPM2_PT_CLOCK_UPDATE = TPM2_PT_FIXED + 25,
+	TPM2_PT_CONTEXT_HASH = TPM2_PT_FIXED + 26,
+	TPM2_PT_CONTEXT_SYM = TPM2_PT_FIXED + 27,
+	TPM2_PT_CONTEXT_SYM_SIZE = TPM2_PT_FIXED + 28,
+	TPM2_PT_ORDERLY_COUNT = TPM2_PT_FIXED + 29,
+	TPM2_PT_MAX_COMMAND_SIZE = TPM2_PT_FIXED + 30,
+	TPM2_PT_MAX_RESPONSE_SIZE = TPM2_PT_FIXED + 31,
+	TPM2_PT_MAX_DIGEST = TPM2_PT_FIXED + 32,
+	TPM2_PT_MAX_OBJECT_CONTEXT = TPM2_PT_FIXED + 33,
+	TPM2_PT_MAX_SESSION_CONTEXT = TPM2_PT_FIXED + 34,
+	TPM2_PT_PS_FAMILY_INDICATOR = TPM2_PT_FIXED + 35,
+	TPM2_PT_PS_LEVEL = TPM2_PT_FIXED + 36,
+	TPM2_PT_PS_REVISION = TPM2_PT_FIXED + 37,
+	TPM2_PT_PS_DAY_OF_YEAR = TPM2_PT_FIXED + 38,
+	TPM2_PT_PS_YEAR = TPM2_PT_FIXED + 39,
+	TPM2_PT_SPLIT_MAX = TPM2_PT_FIXED + 40,
+	TPM2_PT_TOTAL_COMMANDS = TPM2_PT_FIXED + 41,
+	TPM2_PT_LIBRARY_COMMANDS = TPM2_PT_FIXED + 42,
+	TPM2_PT_VENDOR_COMMANDS = TPM2_PT_FIXED + 43,
+	TPM2_PT_NV_BUFFER_MAX = TPM2_PT_FIXED + 44,
+	TPM2_PT_MODES = TPM2_PT_FIXED + 45,
+	TPM2_PT_MAX_CAP_BUFFER = TPM2_PT_FIXED + 46,
+	TPM2_PT_VAR = TPM2_PT_GROUP * 2,
+	TPM2_PT_PERMANENT = TPM2_PT_VAR + 0,
+	TPM2_PT_STARTUP_CLEAR = TPM2_PT_VAR + 1,
+	TPM2_PT_HR_NV_INDEX = TPM2_PT_VAR + 2,
+	TPM2_PT_HR_LOADED = TPM2_PT_VAR + 3,
+	TPM2_PT_HR_LOADED_AVAIL = TPM2_PT_VAR + 4,
+	TPM2_PT_HR_ACTIVE = TPM2_PT_VAR + 5,
+	TPM2_PT_HR_ACTIVE_AVAIL = TPM2_PT_VAR + 6,
+	TPM2_PT_HR_TRANSIENT_AVAIL = TPM2_PT_VAR + 7,
+	TPM2_PT_HR_PERSISTENT = TPM2_PT_VAR + 8,
+	TPM2_PT_HR_PERSISTENT_AVAIL = TPM2_PT_VAR + 9,
+	TPM2_PT_NV_COUNTERS = TPM2_PT_VAR + 10,
+	TPM2_PT_NV_COUNTERS_AVAIL = TPM2_PT_VAR + 11,
+	TPM2_PT_ALGORITHM_SET = TPM2_PT_VAR + 12,
+	TPM2_PT_LOADED_CURVES = TPM2_PT_VAR + 13,
+	TPM2_PT_LOCKOUT_COUNTER = TPM2_PT_VAR + 14,
+	TPM2_PT_MAX_AUTH_FAIL = TPM2_PT_VAR + 15,
+	TPM2_PT_LOCKOUT_INTERVAL = TPM2_PT_VAR + 16,
+	TPM2_PT_LOCKOUT_RECOVERY = TPM2_PT_VAR + 17,
+	TPM2_PT_NV_WRITE_RECOVERY = TPM2_PT_VAR + 18,
+	TPM2_PT_AUDIT_COUNTER_0 = TPM2_PT_VAR + 19,
+	TPM2_PT_AUDIT_COUNTER_1 = TPM2_PT_VAR + 20,
+};
 
 /* 128 bytes is an arbitrary cap. This could be as large as TPM_BUFSIZE - 18
  * bytes, but 128 is still a relatively large number of random bytes and
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs
  2023-02-09 15:31 [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs Jason A. Donenfeld
@ 2023-02-09 15:41 ` Limonciello, Mario
  2023-02-09 17:26   ` Jason A. Donenfeld
  2023-02-11 15:15   ` Linux regression tracking (Thorsten Leemhuis)
  2023-02-10  0:54 ` Jarkko Sakkinen
  1 sibling, 2 replies; 6+ messages in thread
From: Limonciello, Mario @ 2023-02-09 15:41 UTC (permalink / raw)
  To: Jason A. Donenfeld, regressions, linux-integrity, linux-kernel
  Cc: Jarkko Sakkinen, Thorsten Leemhuis, James Bottomley

On 2/9/2023 09:31, Jason A. Donenfeld wrote:
> Do not register a hwrng for certain AMD TPMs that are running an old
> known-buggy revision. Do this by probing the TPM2_PT_MANUFACTURER,
> TPM2_PT_FIRMWARE_VERSION_1, and TPM2_PT_FIRMWARE_VERSION_2 properties,
> and failing when an "AMD"-manufactured TPM2 chip is below a threshold.
> 
> BROKEN BROKEN BROKEN - I just made the version numbers up and haven't
> tested this because I don't actually have hardware for it. I'm posting
> this so that Mario can take over its development and submit a v2 himself
> once he has confirmed the versioning info from inside AMD.

Thanks, happy to do that.  As I'll take over some comments are inline 
mostly for my future self when this is resubmitted.

Perhaps one of the people who can reproduce this right now can confirm 
this works?  The version number string should at least trigger one of 
the reported systems to avoid doing this.

> 
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Thorsten Leemhuis <regressions@leemhuis.info>
> Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

At least as a note for myself this needs:
1. 'Link' tag to the public advisory for this fTPM issue
2. A 'Fixes' tag to the commit that enabled fTPM as a hwrng source by 
default and exposed the issue.
3. A 'Link' tag to the bugzilla.

> ---
>   drivers/char/tpm/tpm-chip.c | 34 ++++++++++++++++-
>   drivers/char/tpm/tpm.h      | 73 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 741d8f3e8fb3..e0f8134d31a3 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -512,6 +512,37 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
>   	return 0;
>   }
>   
> +static bool tpm_is_rng_defective(struct tpm_chip *chip)
> +{
> +	int ret;
> +	u32 val1, val2;
> +	u64 version;
> +
> +	/* No known-broken TPM1 chips. */
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> +		return false;
> +
> +	/* Only known-broken are AMD. */
> +	ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL);
> +	if (ret < 0 || val1 != 0x414D4400U /* AMD */)
> +		return false;
> +
> +	/* Grab and concat the version values. */
> +	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL);
> +	if (ret < 0)
> +		return false;
> +	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL);
> +	if (ret < 0)
> +		return false;
> +	version = ((u64)val1 << 32) | val2;
> +
> +	/* Versions below 3.4e.2.9 are broken. */
> +	if (version < 0x0003004E0002009ULL)
> +		return true;

We probably want to also do a 'dev_info_once' or 'dev_warn_once' here to 
let people know that they're not getting a fTPM RNG so it's not a surprise.

> +
> +	return false;
> +}
> +
>   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);
> @@ -521,7 +552,8 @@ 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))
> +	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> +	    tpm_is_rng_defective(chip))
>   		return 0;
>   
>   	snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 24ee4e1cc452..830014a26609 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -150,6 +150,79 @@ enum tpm_sub_capabilities {
>   	TPM_CAP_PROP_TIS_DURATION = 0x120,
>   };
>   
> +enum tpm2_pt_props {
> +	TPM2_PT_NONE = 0x00000000,
> +	TPM2_PT_GROUP = 0x00000100,
> +	TPM2_PT_FIXED = TPM2_PT_GROUP * 1,
> +	TPM2_PT_FAMILY_INDICATOR = TPM2_PT_FIXED + 0,
> +	TPM2_PT_LEVEL = TPM2_PT_FIXED + 1,
> +	TPM2_PT_REVISION = TPM2_PT_FIXED + 2,
> +	TPM2_PT_DAY_OF_YEAR = TPM2_PT_FIXED + 3,
> +	TPM2_PT_YEAR = TPM2_PT_FIXED + 4,
> +	TPM2_PT_MANUFACTURER = TPM2_PT_FIXED + 5,
> +	TPM2_PT_VENDOR_STRING_1 = TPM2_PT_FIXED + 6,
> +	TPM2_PT_VENDOR_STRING_2 = TPM2_PT_FIXED + 7,
> +	TPM2_PT_VENDOR_STRING_3 = TPM2_PT_FIXED + 8,
> +	TPM2_PT_VENDOR_STRING_4 = TPM2_PT_FIXED + 9,
> +	TPM2_PT_VENDOR_TPM_TYPE = TPM2_PT_FIXED + 10,
> +	TPM2_PT_FIRMWARE_VERSION_1 = TPM2_PT_FIXED + 11,
> +	TPM2_PT_FIRMWARE_VERSION_2 = TPM2_PT_FIXED + 12,
> +	TPM2_PT_INPUT_BUFFER = TPM2_PT_FIXED + 13,
> +	TPM2_PT_HR_TRANSIENT_MIN = TPM2_PT_FIXED + 14,
> +	TPM2_PT_HR_PERSISTENT_MIN = TPM2_PT_FIXED + 15,
> +	TPM2_PT_HR_LOADED_MIN = TPM2_PT_FIXED + 16,
> +	TPM2_PT_ACTIVE_SESSIONS_MAX = TPM2_PT_FIXED + 17,
> +	TPM2_PT_PCR_COUNT = TPM2_PT_FIXED + 18,
> +	TPM2_PT_PCR_SELECT_MIN = TPM2_PT_FIXED + 19,
> +	TPM2_PT_CONTEXT_GAP_MAX = TPM2_PT_FIXED + 20,
> +	TPM2_PT_NV_COUNTERS_MAX = TPM2_PT_FIXED + 22,
> +	TPM2_PT_NV_INDEX_MAX = TPM2_PT_FIXED + 23,
> +	TPM2_PT_MEMORY = TPM2_PT_FIXED + 24,
> +	TPM2_PT_CLOCK_UPDATE = TPM2_PT_FIXED + 25,
> +	TPM2_PT_CONTEXT_HASH = TPM2_PT_FIXED + 26,
> +	TPM2_PT_CONTEXT_SYM = TPM2_PT_FIXED + 27,
> +	TPM2_PT_CONTEXT_SYM_SIZE = TPM2_PT_FIXED + 28,
> +	TPM2_PT_ORDERLY_COUNT = TPM2_PT_FIXED + 29,
> +	TPM2_PT_MAX_COMMAND_SIZE = TPM2_PT_FIXED + 30,
> +	TPM2_PT_MAX_RESPONSE_SIZE = TPM2_PT_FIXED + 31,
> +	TPM2_PT_MAX_DIGEST = TPM2_PT_FIXED + 32,
> +	TPM2_PT_MAX_OBJECT_CONTEXT = TPM2_PT_FIXED + 33,
> +	TPM2_PT_MAX_SESSION_CONTEXT = TPM2_PT_FIXED + 34,
> +	TPM2_PT_PS_FAMILY_INDICATOR = TPM2_PT_FIXED + 35,
> +	TPM2_PT_PS_LEVEL = TPM2_PT_FIXED + 36,
> +	TPM2_PT_PS_REVISION = TPM2_PT_FIXED + 37,
> +	TPM2_PT_PS_DAY_OF_YEAR = TPM2_PT_FIXED + 38,
> +	TPM2_PT_PS_YEAR = TPM2_PT_FIXED + 39,
> +	TPM2_PT_SPLIT_MAX = TPM2_PT_FIXED + 40,
> +	TPM2_PT_TOTAL_COMMANDS = TPM2_PT_FIXED + 41,
> +	TPM2_PT_LIBRARY_COMMANDS = TPM2_PT_FIXED + 42,
> +	TPM2_PT_VENDOR_COMMANDS = TPM2_PT_FIXED + 43,
> +	TPM2_PT_NV_BUFFER_MAX = TPM2_PT_FIXED + 44,
> +	TPM2_PT_MODES = TPM2_PT_FIXED + 45,
> +	TPM2_PT_MAX_CAP_BUFFER = TPM2_PT_FIXED + 46,
> +	TPM2_PT_VAR = TPM2_PT_GROUP * 2,
> +	TPM2_PT_PERMANENT = TPM2_PT_VAR + 0,
> +	TPM2_PT_STARTUP_CLEAR = TPM2_PT_VAR + 1,
> +	TPM2_PT_HR_NV_INDEX = TPM2_PT_VAR + 2,
> +	TPM2_PT_HR_LOADED = TPM2_PT_VAR + 3,
> +	TPM2_PT_HR_LOADED_AVAIL = TPM2_PT_VAR + 4,
> +	TPM2_PT_HR_ACTIVE = TPM2_PT_VAR + 5,
> +	TPM2_PT_HR_ACTIVE_AVAIL = TPM2_PT_VAR + 6,
> +	TPM2_PT_HR_TRANSIENT_AVAIL = TPM2_PT_VAR + 7,
> +	TPM2_PT_HR_PERSISTENT = TPM2_PT_VAR + 8,
> +	TPM2_PT_HR_PERSISTENT_AVAIL = TPM2_PT_VAR + 9,
> +	TPM2_PT_NV_COUNTERS = TPM2_PT_VAR + 10,
> +	TPM2_PT_NV_COUNTERS_AVAIL = TPM2_PT_VAR + 11,
> +	TPM2_PT_ALGORITHM_SET = TPM2_PT_VAR + 12,
> +	TPM2_PT_LOADED_CURVES = TPM2_PT_VAR + 13,
> +	TPM2_PT_LOCKOUT_COUNTER = TPM2_PT_VAR + 14,
> +	TPM2_PT_MAX_AUTH_FAIL = TPM2_PT_VAR + 15,
> +	TPM2_PT_LOCKOUT_INTERVAL = TPM2_PT_VAR + 16,
> +	TPM2_PT_LOCKOUT_RECOVERY = TPM2_PT_VAR + 17,
> +	TPM2_PT_NV_WRITE_RECOVERY = TPM2_PT_VAR + 18,
> +	TPM2_PT_AUDIT_COUNTER_0 = TPM2_PT_VAR + 19,
> +	TPM2_PT_AUDIT_COUNTER_1 = TPM2_PT_VAR + 20,
> +};
>   

I guess these properties should be split out to a separate patch in a 
series.

>   /* 128 bytes is an arbitrary cap. This could be as large as TPM_BUFSIZE - 18
>    * bytes, but 128 is still a relatively large number of random bytes and


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs
  2023-02-09 15:41 ` Limonciello, Mario
@ 2023-02-09 17:26   ` Jason A. Donenfeld
  2023-02-11 15:15   ` Linux regression tracking (Thorsten Leemhuis)
  1 sibling, 0 replies; 6+ messages in thread
From: Jason A. Donenfeld @ 2023-02-09 17:26 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: regressions, linux-integrity, linux-kernel, Jarkko Sakkinen,
	Thorsten Leemhuis, James Bottomley

On Thu, Feb 9, 2023 at 12:41 PM Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
> On 2/9/2023 09:31, Jason A. Donenfeld wrote:
> > Do not register a hwrng for certain AMD TPMs that are running an old
> > known-buggy revision. Do this by probing the TPM2_PT_MANUFACTURER,
> > TPM2_PT_FIRMWARE_VERSION_1, and TPM2_PT_FIRMWARE_VERSION_2 properties,
> > and failing when an "AMD"-manufactured TPM2 chip is below a threshold.
> >
> > BROKEN BROKEN BROKEN - I just made the version numbers up and haven't
> > tested this because I don't actually have hardware for it. I'm posting
> > this so that Mario can take over its development and submit a v2 himself
> > once he has confirmed the versioning info from inside AMD.
>
> Thanks, happy to do that.  As I'll take over some comments are inline
> mostly for my future self when this is resubmitted.
>
> Perhaps one of the people who can reproduce this right now can confirm
> this works?  The version number string should at least trigger one of
> the reported systems to avoid doing this.
>
> >
> > Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> > Cc: Jarkko Sakkinen <jarkko@kernel.org>
> > Cc: Thorsten Leemhuis <regressions@leemhuis.info>
> > Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
> At least as a note for myself this needs:
> 1. 'Link' tag to the public advisory for this fTPM issue
> 2. A 'Fixes' tag to the commit that enabled fTPM as a hwrng source by
> default and exposed the issue.
> 3. A 'Link' tag to the bugzilla.
>
> > ---
> >   drivers/char/tpm/tpm-chip.c | 34 ++++++++++++++++-
> >   drivers/char/tpm/tpm.h      | 73 +++++++++++++++++++++++++++++++++++++
> >   2 files changed, 106 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 741d8f3e8fb3..e0f8134d31a3 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -512,6 +512,37 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> >       return 0;
> >   }
> >
> > +static bool tpm_is_rng_defective(struct tpm_chip *chip)
> > +{
> > +     int ret;
> > +     u32 val1, val2;
> > +     u64 version;
> > +
> > +     /* No known-broken TPM1 chips. */
> > +     if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > +             return false;
> > +
> > +     /* Only known-broken are AMD. */
> > +     ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL);
> > +     if (ret < 0 || val1 != 0x414D4400U /* AMD */)
> > +             return false;
> > +
> > +     /* Grab and concat the version values. */
> > +     ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL);
> > +     if (ret < 0)
> > +             return false;
> > +     ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL);
> > +     if (ret < 0)
> > +             return false;
> > +     version = ((u64)val1 << 32) | val2;
> > +
> > +     /* Versions below 3.4e.2.9 are broken. */
> > +     if (version < 0x0003004E0002009ULL)
> > +             return true;
>
> We probably want to also do a 'dev_info_once' or 'dev_warn_once' here to
> let people know that they're not getting a fTPM RNG so it's not a surprise.

AFAICT, this runs once per TPM device. So maybe a simple dev_warn()
without the _once part will be good, so you actually see in dmesg each
buggy device.

>
> > +
> > +     return false;
> > +}
> > +
> >   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);
> > @@ -521,7 +552,8 @@ 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))
> > +     if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> > +         tpm_is_rng_defective(chip))
> >               return 0;
> >
> >       snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 24ee4e1cc452..830014a26609 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -150,6 +150,79 @@ enum tpm_sub_capabilities {
> >       TPM_CAP_PROP_TIS_DURATION = 0x120,
> >   };
> >
> > +enum tpm2_pt_props {
> > +     TPM2_PT_NONE = 0x00000000,
> > +     TPM2_PT_GROUP = 0x00000100,
> > +     TPM2_PT_FIXED = TPM2_PT_GROUP * 1,
> > +     TPM2_PT_FAMILY_INDICATOR = TPM2_PT_FIXED + 0,
> > +     TPM2_PT_LEVEL = TPM2_PT_FIXED + 1,
> > +     TPM2_PT_REVISION = TPM2_PT_FIXED + 2,
> > +     TPM2_PT_DAY_OF_YEAR = TPM2_PT_FIXED + 3,
> > +     TPM2_PT_YEAR = TPM2_PT_FIXED + 4,
> > +     TPM2_PT_MANUFACTURER = TPM2_PT_FIXED + 5,
> > +     TPM2_PT_VENDOR_STRING_1 = TPM2_PT_FIXED + 6,
> > +     TPM2_PT_VENDOR_STRING_2 = TPM2_PT_FIXED + 7,
> > +     TPM2_PT_VENDOR_STRING_3 = TPM2_PT_FIXED + 8,
> > +     TPM2_PT_VENDOR_STRING_4 = TPM2_PT_FIXED + 9,
> > +     TPM2_PT_VENDOR_TPM_TYPE = TPM2_PT_FIXED + 10,
> > +     TPM2_PT_FIRMWARE_VERSION_1 = TPM2_PT_FIXED + 11,
> > +     TPM2_PT_FIRMWARE_VERSION_2 = TPM2_PT_FIXED + 12,
> > +     TPM2_PT_INPUT_BUFFER = TPM2_PT_FIXED + 13,
> > +     TPM2_PT_HR_TRANSIENT_MIN = TPM2_PT_FIXED + 14,
> > +     TPM2_PT_HR_PERSISTENT_MIN = TPM2_PT_FIXED + 15,
> > +     TPM2_PT_HR_LOADED_MIN = TPM2_PT_FIXED + 16,
> > +     TPM2_PT_ACTIVE_SESSIONS_MAX = TPM2_PT_FIXED + 17,
> > +     TPM2_PT_PCR_COUNT = TPM2_PT_FIXED + 18,
> > +     TPM2_PT_PCR_SELECT_MIN = TPM2_PT_FIXED + 19,
> > +     TPM2_PT_CONTEXT_GAP_MAX = TPM2_PT_FIXED + 20,
> > +     TPM2_PT_NV_COUNTERS_MAX = TPM2_PT_FIXED + 22,
> > +     TPM2_PT_NV_INDEX_MAX = TPM2_PT_FIXED + 23,
> > +     TPM2_PT_MEMORY = TPM2_PT_FIXED + 24,
> > +     TPM2_PT_CLOCK_UPDATE = TPM2_PT_FIXED + 25,
> > +     TPM2_PT_CONTEXT_HASH = TPM2_PT_FIXED + 26,
> > +     TPM2_PT_CONTEXT_SYM = TPM2_PT_FIXED + 27,
> > +     TPM2_PT_CONTEXT_SYM_SIZE = TPM2_PT_FIXED + 28,
> > +     TPM2_PT_ORDERLY_COUNT = TPM2_PT_FIXED + 29,
> > +     TPM2_PT_MAX_COMMAND_SIZE = TPM2_PT_FIXED + 30,
> > +     TPM2_PT_MAX_RESPONSE_SIZE = TPM2_PT_FIXED + 31,
> > +     TPM2_PT_MAX_DIGEST = TPM2_PT_FIXED + 32,
> > +     TPM2_PT_MAX_OBJECT_CONTEXT = TPM2_PT_FIXED + 33,
> > +     TPM2_PT_MAX_SESSION_CONTEXT = TPM2_PT_FIXED + 34,
> > +     TPM2_PT_PS_FAMILY_INDICATOR = TPM2_PT_FIXED + 35,
> > +     TPM2_PT_PS_LEVEL = TPM2_PT_FIXED + 36,
> > +     TPM2_PT_PS_REVISION = TPM2_PT_FIXED + 37,
> > +     TPM2_PT_PS_DAY_OF_YEAR = TPM2_PT_FIXED + 38,
> > +     TPM2_PT_PS_YEAR = TPM2_PT_FIXED + 39,
> > +     TPM2_PT_SPLIT_MAX = TPM2_PT_FIXED + 40,
> > +     TPM2_PT_TOTAL_COMMANDS = TPM2_PT_FIXED + 41,
> > +     TPM2_PT_LIBRARY_COMMANDS = TPM2_PT_FIXED + 42,
> > +     TPM2_PT_VENDOR_COMMANDS = TPM2_PT_FIXED + 43,
> > +     TPM2_PT_NV_BUFFER_MAX = TPM2_PT_FIXED + 44,
> > +     TPM2_PT_MODES = TPM2_PT_FIXED + 45,
> > +     TPM2_PT_MAX_CAP_BUFFER = TPM2_PT_FIXED + 46,
> > +     TPM2_PT_VAR = TPM2_PT_GROUP * 2,
> > +     TPM2_PT_PERMANENT = TPM2_PT_VAR + 0,
> > +     TPM2_PT_STARTUP_CLEAR = TPM2_PT_VAR + 1,
> > +     TPM2_PT_HR_NV_INDEX = TPM2_PT_VAR + 2,
> > +     TPM2_PT_HR_LOADED = TPM2_PT_VAR + 3,
> > +     TPM2_PT_HR_LOADED_AVAIL = TPM2_PT_VAR + 4,
> > +     TPM2_PT_HR_ACTIVE = TPM2_PT_VAR + 5,
> > +     TPM2_PT_HR_ACTIVE_AVAIL = TPM2_PT_VAR + 6,
> > +     TPM2_PT_HR_TRANSIENT_AVAIL = TPM2_PT_VAR + 7,
> > +     TPM2_PT_HR_PERSISTENT = TPM2_PT_VAR + 8,
> > +     TPM2_PT_HR_PERSISTENT_AVAIL = TPM2_PT_VAR + 9,
> > +     TPM2_PT_NV_COUNTERS = TPM2_PT_VAR + 10,
> > +     TPM2_PT_NV_COUNTERS_AVAIL = TPM2_PT_VAR + 11,
> > +     TPM2_PT_ALGORITHM_SET = TPM2_PT_VAR + 12,
> > +     TPM2_PT_LOADED_CURVES = TPM2_PT_VAR + 13,
> > +     TPM2_PT_LOCKOUT_COUNTER = TPM2_PT_VAR + 14,
> > +     TPM2_PT_MAX_AUTH_FAIL = TPM2_PT_VAR + 15,
> > +     TPM2_PT_LOCKOUT_INTERVAL = TPM2_PT_VAR + 16,
> > +     TPM2_PT_LOCKOUT_RECOVERY = TPM2_PT_VAR + 17,
> > +     TPM2_PT_NV_WRITE_RECOVERY = TPM2_PT_VAR + 18,
> > +     TPM2_PT_AUDIT_COUNTER_0 = TPM2_PT_VAR + 19,
> > +     TPM2_PT_AUDIT_COUNTER_1 = TPM2_PT_VAR + 20,
> > +};
> >
>
> I guess these properties should be split out to a separate patch in a
> series.

That will make backporting this more annoying. Up to Jarkko,
obviously, but personally I wouldn't bother.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs
  2023-02-09 15:31 [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs Jason A. Donenfeld
  2023-02-09 15:41 ` Limonciello, Mario
@ 2023-02-10  0:54 ` Jarkko Sakkinen
  2023-02-10  1:08   ` Jason A. Donenfeld
  1 sibling, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2023-02-10  0:54 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: regressions, linux-integrity, linux-kernel, Mario Limonciello,
	Thorsten Leemhuis, James Bottomley

On Thu, Feb 09, 2023 at 12:31:20PM -0300, Jason A. Donenfeld wrote:
> Do not register a hwrng for certain AMD TPMs that are running an old

What do you mean by "certain AMD TPMs"?

> known-buggy revision. Do this by probing the TPM2_PT_MANUFACTURER,

Which revision?

> TPM2_PT_FIRMWARE_VERSION_1, and TPM2_PT_FIRMWARE_VERSION_2 properties,
> and failing when an "AMD"-manufactured TPM2 chip is below a threshold.

What do you mean by "threshold"?

This also lacks desription of:

1. What kind of changes are done.
2. Why do they they are reasonable.

E.g. "failing" does not have any useful and measurable meaning...

> BROKEN BROKEN BROKEN - I just made the version numbers up and haven't
> tested this because I don't actually have hardware for it. I'm posting
> this so that Mario can take over its development and submit a v2 himself
> once he has confirmed the versioning info from inside AMD.

Is this paragraph meant for commit log?

> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Thorsten Leemhuis <regressions@leemhuis.info>
> Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  drivers/char/tpm/tpm-chip.c | 34 ++++++++++++++++-
>  drivers/char/tpm/tpm.h      | 73 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 741d8f3e8fb3..e0f8134d31a3 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -512,6 +512,37 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
>  	return 0;
>  }
>  
> +static bool tpm_is_rng_defective(struct tpm_chip *chip)
> +{
> +	int ret;
> +	u32 val1, val2;
> +	u64 version;
> +
> +	/* No known-broken TPM1 chips. */
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> +		return false;
> +
> +	/* Only known-broken are AMD. */
> +	ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL);
> +	if (ret < 0 || val1 != 0x414D4400U /* AMD */)
> +		return false;
> +
> +	/* Grab and concat the version values. */
> +	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL);
> +	if (ret < 0)
> +		return false;
> +	ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL);
> +	if (ret < 0)
> +		return false;
> +	version = ((u64)val1 << 32) | val2;
> +
> +	/* Versions below 3.4e.2.9 are broken. */
> +	if (version < 0x0003004E0002009ULL)
> +		return true;
> +
> +	return false;
> +}
> +
>  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);
> @@ -521,7 +552,8 @@ 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))
> +	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> +	    tpm_is_rng_defective(chip))
>  		return 0;
>  
>  	snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 24ee4e1cc452..830014a26609 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -150,6 +150,79 @@ enum tpm_sub_capabilities {
>  	TPM_CAP_PROP_TIS_DURATION = 0x120,
>  };
>  
> +enum tpm2_pt_props {
> +	TPM2_PT_NONE = 0x00000000,
> +	TPM2_PT_GROUP = 0x00000100,
> +	TPM2_PT_FIXED = TPM2_PT_GROUP * 1,
> +	TPM2_PT_FAMILY_INDICATOR = TPM2_PT_FIXED + 0,
> +	TPM2_PT_LEVEL = TPM2_PT_FIXED + 1,
> +	TPM2_PT_REVISION = TPM2_PT_FIXED + 2,
> +	TPM2_PT_DAY_OF_YEAR = TPM2_PT_FIXED + 3,
> +	TPM2_PT_YEAR = TPM2_PT_FIXED + 4,
> +	TPM2_PT_MANUFACTURER = TPM2_PT_FIXED + 5,
> +	TPM2_PT_VENDOR_STRING_1 = TPM2_PT_FIXED + 6,
> +	TPM2_PT_VENDOR_STRING_2 = TPM2_PT_FIXED + 7,
> +	TPM2_PT_VENDOR_STRING_3 = TPM2_PT_FIXED + 8,
> +	TPM2_PT_VENDOR_STRING_4 = TPM2_PT_FIXED + 9,
> +	TPM2_PT_VENDOR_TPM_TYPE = TPM2_PT_FIXED + 10,
> +	TPM2_PT_FIRMWARE_VERSION_1 = TPM2_PT_FIXED + 11,
> +	TPM2_PT_FIRMWARE_VERSION_2 = TPM2_PT_FIXED + 12,
> +	TPM2_PT_INPUT_BUFFER = TPM2_PT_FIXED + 13,
> +	TPM2_PT_HR_TRANSIENT_MIN = TPM2_PT_FIXED + 14,
> +	TPM2_PT_HR_PERSISTENT_MIN = TPM2_PT_FIXED + 15,
> +	TPM2_PT_HR_LOADED_MIN = TPM2_PT_FIXED + 16,
> +	TPM2_PT_ACTIVE_SESSIONS_MAX = TPM2_PT_FIXED + 17,
> +	TPM2_PT_PCR_COUNT = TPM2_PT_FIXED + 18,
> +	TPM2_PT_PCR_SELECT_MIN = TPM2_PT_FIXED + 19,
> +	TPM2_PT_CONTEXT_GAP_MAX = TPM2_PT_FIXED + 20,
> +	TPM2_PT_NV_COUNTERS_MAX = TPM2_PT_FIXED + 22,
> +	TPM2_PT_NV_INDEX_MAX = TPM2_PT_FIXED + 23,
> +	TPM2_PT_MEMORY = TPM2_PT_FIXED + 24,
> +	TPM2_PT_CLOCK_UPDATE = TPM2_PT_FIXED + 25,
> +	TPM2_PT_CONTEXT_HASH = TPM2_PT_FIXED + 26,
> +	TPM2_PT_CONTEXT_SYM = TPM2_PT_FIXED + 27,
> +	TPM2_PT_CONTEXT_SYM_SIZE = TPM2_PT_FIXED + 28,
> +	TPM2_PT_ORDERLY_COUNT = TPM2_PT_FIXED + 29,
> +	TPM2_PT_MAX_COMMAND_SIZE = TPM2_PT_FIXED + 30,
> +	TPM2_PT_MAX_RESPONSE_SIZE = TPM2_PT_FIXED + 31,
> +	TPM2_PT_MAX_DIGEST = TPM2_PT_FIXED + 32,
> +	TPM2_PT_MAX_OBJECT_CONTEXT = TPM2_PT_FIXED + 33,
> +	TPM2_PT_MAX_SESSION_CONTEXT = TPM2_PT_FIXED + 34,
> +	TPM2_PT_PS_FAMILY_INDICATOR = TPM2_PT_FIXED + 35,
> +	TPM2_PT_PS_LEVEL = TPM2_PT_FIXED + 36,
> +	TPM2_PT_PS_REVISION = TPM2_PT_FIXED + 37,
> +	TPM2_PT_PS_DAY_OF_YEAR = TPM2_PT_FIXED + 38,
> +	TPM2_PT_PS_YEAR = TPM2_PT_FIXED + 39,
> +	TPM2_PT_SPLIT_MAX = TPM2_PT_FIXED + 40,
> +	TPM2_PT_TOTAL_COMMANDS = TPM2_PT_FIXED + 41,
> +	TPM2_PT_LIBRARY_COMMANDS = TPM2_PT_FIXED + 42,
> +	TPM2_PT_VENDOR_COMMANDS = TPM2_PT_FIXED + 43,
> +	TPM2_PT_NV_BUFFER_MAX = TPM2_PT_FIXED + 44,
> +	TPM2_PT_MODES = TPM2_PT_FIXED + 45,
> +	TPM2_PT_MAX_CAP_BUFFER = TPM2_PT_FIXED + 46,
> +	TPM2_PT_VAR = TPM2_PT_GROUP * 2,
> +	TPM2_PT_PERMANENT = TPM2_PT_VAR + 0,
> +	TPM2_PT_STARTUP_CLEAR = TPM2_PT_VAR + 1,
> +	TPM2_PT_HR_NV_INDEX = TPM2_PT_VAR + 2,
> +	TPM2_PT_HR_LOADED = TPM2_PT_VAR + 3,
> +	TPM2_PT_HR_LOADED_AVAIL = TPM2_PT_VAR + 4,
> +	TPM2_PT_HR_ACTIVE = TPM2_PT_VAR + 5,
> +	TPM2_PT_HR_ACTIVE_AVAIL = TPM2_PT_VAR + 6,
> +	TPM2_PT_HR_TRANSIENT_AVAIL = TPM2_PT_VAR + 7,
> +	TPM2_PT_HR_PERSISTENT = TPM2_PT_VAR + 8,
> +	TPM2_PT_HR_PERSISTENT_AVAIL = TPM2_PT_VAR + 9,
> +	TPM2_PT_NV_COUNTERS = TPM2_PT_VAR + 10,
> +	TPM2_PT_NV_COUNTERS_AVAIL = TPM2_PT_VAR + 11,
> +	TPM2_PT_ALGORITHM_SET = TPM2_PT_VAR + 12,
> +	TPM2_PT_LOADED_CURVES = TPM2_PT_VAR + 13,
> +	TPM2_PT_LOCKOUT_COUNTER = TPM2_PT_VAR + 14,
> +	TPM2_PT_MAX_AUTH_FAIL = TPM2_PT_VAR + 15,
> +	TPM2_PT_LOCKOUT_INTERVAL = TPM2_PT_VAR + 16,
> +	TPM2_PT_LOCKOUT_RECOVERY = TPM2_PT_VAR + 17,
> +	TPM2_PT_NV_WRITE_RECOVERY = TPM2_PT_VAR + 18,
> +	TPM2_PT_AUDIT_COUNTER_0 = TPM2_PT_VAR + 19,
> +	TPM2_PT_AUDIT_COUNTER_1 = TPM2_PT_VAR + 20,
> +};
>  
>  /* 128 bytes is an arbitrary cap. This could be as large as TPM_BUFSIZE - 18
>   * bytes, but 128 is still a relatively large number of random bytes and
> -- 
> 2.39.1
> 

BR, Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs
  2023-02-10  0:54 ` Jarkko Sakkinen
@ 2023-02-10  1:08   ` Jason A. Donenfeld
  0 siblings, 0 replies; 6+ messages in thread
From: Jason A. Donenfeld @ 2023-02-10  1:08 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: regressions, linux-integrity, linux-kernel, Mario Limonciello,
	Thorsten Leemhuis, James Bottomley

On Fri, Feb 10, 2023 at 02:54:36AM +0200, Jarkko Sakkinen wrote:
> On Thu, Feb 09, 2023 at 12:31:20PM -0300, Jason A. Donenfeld wrote:
> > Do not register a hwrng for certain AMD TPMs that are running an old
> 
> What do you mean by "certain AMD TPMs"?
> 
> > known-buggy revision. Do this by probing the TPM2_PT_MANUFACTURER,
> 
> Which revision?
> 
> > TPM2_PT_FIRMWARE_VERSION_1, and TPM2_PT_FIRMWARE_VERSION_2 properties,
> > and failing when an "AMD"-manufactured TPM2 chip is below a threshold.
> 
> What do you mean by "threshold"?
> 
> This also lacks desription of:
> 
> 1. What kind of changes are done.
> 2. Why do they they are reasonable.

YEP! And guess what? It doesn't matter at all one bit. Why? Because
Mario, the AMD engineer working on this, is tuned into the goal here and
he's the one who will be reworking this PoC draft into a real non-RFC
commit. We've been discussing this over on the bugzilla Thorsten sent to
you on Feb 2: https://bugzilla.kernel.org/show_bug.cgi?id=216989

> > BROKEN BROKEN BROKEN - I just made the version numbers up and haven't
> > tested this because I don't actually have hardware for it. I'm posting
> > this so that Mario can take over its development and submit a v2 himself
> > once he has confirmed the versioning info from inside AMD.
> 
> Is this paragraph meant for commit log?

Obviously not; did you read it?

Jason

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs
  2023-02-09 15:41 ` Limonciello, Mario
  2023-02-09 17:26   ` Jason A. Donenfeld
@ 2023-02-11 15:15   ` Linux regression tracking (Thorsten Leemhuis)
  1 sibling, 0 replies; 6+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-02-11 15:15 UTC (permalink / raw)
  To: Limonciello, Mario, Jason A. Donenfeld, regressions,
	linux-integrity, linux-kernel
  Cc: Jarkko Sakkinen, James Bottomley

On 09.02.23 16:41, Limonciello, Mario wrote:
> On 2/9/2023 09:31, Jason A. Donenfeld wrote:
>> Do not register a hwrng for certain AMD TPMs that are running an old
>> known-buggy revision. Do this by probing the TPM2_PT_MANUFACTURER,
>> TPM2_PT_FIRMWARE_VERSION_1, and TPM2_PT_FIRMWARE_VERSION_2 properties,
>> and failing when an "AMD"-manufactured TPM2 chip is below a threshold.
>>
>> BROKEN BROKEN BROKEN - I just made the version numbers up and haven't
>> tested this because I don't actually have hardware for it. I'm posting
>> this so that Mario can take over its development and submit a v2 himself
>> once he has confirmed the versioning info from inside AMD.
> 
> Thanks, happy to do that. 

Just a quick note from my side: many many thx to both of you for taking
care of this!

Ciao, Thorsten

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-02-11 15:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-09 15:31 [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs Jason A. Donenfeld
2023-02-09 15:41 ` Limonciello, Mario
2023-02-09 17:26   ` Jason A. Donenfeld
2023-02-11 15:15   ` Linux regression tracking (Thorsten Leemhuis)
2023-02-10  0:54 ` Jarkko Sakkinen
2023-02-10  1:08   ` Jason A. Donenfeld

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.