| Message ID | 20230808041229.22514-1-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tpm: Add a helper for checking hwrng enabled | expand |
On Tue Aug 8, 2023 at 7:12 AM EEST, Mario Limonciello wrote: > The same checks are repeated in 3 places to decide whether to use > hwrng. Consolidate these into a helper. > > Also this fixes a case that one of them was missing a check in the > cleanup path. > > Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/char/tpm/tpm-chip.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index e904aae9771be..ea6b4013bc38f 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -521,10 +521,20 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) > return tpm_get_random(chip, data, max); > } > > +static bool tpm_is_hwrng_enabled(struct tpm_chip *chip) > +{ > + if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM)) > + return false; > + if (tpm_is_firmware_upgrade(chip)) > + return false; > + if (chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) > + return false; > + return true; > +} > + > static int tpm_add_hwrng(struct tpm_chip *chip) > { > - if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) || > - chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) > + if (!tpm_is_hwrng_enabled(chip)) > return 0; > > snprintf(chip->hwrng_name, sizeof(chip->hwrng_name), > @@ -629,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_hwrng_enabled(chip)) > hwrng_unregister(&chip->hwrng); > out_ppi: > tpm_bios_log_teardown(chip); > @@ -654,8 +664,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) && > - !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)) > + if (tpm_is_hwrng_enabled(chip)) > hwrng_unregister(&chip->hwrng); > tpm_bios_log_teardown(chip); > if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip)) Given my previous queries: should I apply this or revert and apply v3? BR, Jarkko
On 8/10/2023 10:07 AM, Jarkko Sakkinen wrote: > On Tue Aug 8, 2023 at 7:12 AM EEST, Mario Limonciello wrote: >> The same checks are repeated in 3 places to decide whether to use >> hwrng. Consolidate these into a helper. >> >> Also this fixes a case that one of them was missing a check in the >> cleanup path. >> >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs") >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> drivers/char/tpm/tpm-chip.c | 19 ++++++++++++++----- >> 1 file changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c >> index e904aae9771be..ea6b4013bc38f 100644 >> --- a/drivers/char/tpm/tpm-chip.c >> +++ b/drivers/char/tpm/tpm-chip.c >> @@ -521,10 +521,20 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) >> return tpm_get_random(chip, data, max); >> } >> >> +static bool tpm_is_hwrng_enabled(struct tpm_chip *chip) >> +{ >> + if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM)) >> + return false; >> + if (tpm_is_firmware_upgrade(chip)) >> + return false; >> + if (chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) >> + return false; >> + return true; >> +} >> + >> static int tpm_add_hwrng(struct tpm_chip *chip) >> { >> - if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) || >> - chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) >> + if (!tpm_is_hwrng_enabled(chip)) >> return 0; >> >> snprintf(chip->hwrng_name, sizeof(chip->hwrng_name), >> @@ -629,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_hwrng_enabled(chip)) >> hwrng_unregister(&chip->hwrng); >> out_ppi: >> tpm_bios_log_teardown(chip); >> @@ -654,8 +664,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) && >> - !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)) >> + if (tpm_is_hwrng_enabled(chip)) >> hwrng_unregister(&chip->hwrng); >> tpm_bios_log_teardown(chip); >> if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip)) > > Given my previous queries: should I apply this or revert and apply v3? > > BR, Jarkko Linus picked it up already directly.
On Thu Aug 10, 2023 at 6:08 PM EEST, Limonciello, Mario wrote: > > > On 8/10/2023 10:07 AM, Jarkko Sakkinen wrote: > > On Tue Aug 8, 2023 at 7:12 AM EEST, Mario Limonciello wrote: > >> The same checks are repeated in 3 places to decide whether to use > >> hwrng. Consolidate these into a helper. > >> > >> Also this fixes a case that one of them was missing a check in the > >> cleanup path. > >> > >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs") > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > >> --- > >> drivers/char/tpm/tpm-chip.c | 19 ++++++++++++++----- > >> 1 file changed, 14 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > >> index e904aae9771be..ea6b4013bc38f 100644 > >> --- a/drivers/char/tpm/tpm-chip.c > >> +++ b/drivers/char/tpm/tpm-chip.c > >> @@ -521,10 +521,20 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) > >> return tpm_get_random(chip, data, max); > >> } > >> > >> +static bool tpm_is_hwrng_enabled(struct tpm_chip *chip) > >> +{ > >> + if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM)) > >> + return false; > >> + if (tpm_is_firmware_upgrade(chip)) > >> + return false; > >> + if (chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) > >> + return false; > >> + return true; > >> +} > >> + > >> static int tpm_add_hwrng(struct tpm_chip *chip) > >> { > >> - if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) || > >> - chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) > >> + if (!tpm_is_hwrng_enabled(chip)) > >> return 0; > >> > >> snprintf(chip->hwrng_name, sizeof(chip->hwrng_name), > >> @@ -629,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_hwrng_enabled(chip)) > >> hwrng_unregister(&chip->hwrng); > >> out_ppi: > >> tpm_bios_log_teardown(chip); > >> @@ -654,8 +664,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) && > >> - !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)) > >> + if (tpm_is_hwrng_enabled(chip)) > >> hwrng_unregister(&chip->hwrng); > >> tpm_bios_log_teardown(chip); > >> if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip)) > > > > Given my previous queries: should I apply this or revert and apply v3? > > > > BR, Jarkko > > Linus picked it up already directly. I came two days behind, sorry. BR, Jarkko
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index e904aae9771be..ea6b4013bc38f 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -521,10 +521,20 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) return tpm_get_random(chip, data, max); } +static bool tpm_is_hwrng_enabled(struct tpm_chip *chip) +{ + if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM)) + return false; + if (tpm_is_firmware_upgrade(chip)) + return false; + if (chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) + return false; + return true; +} + static int tpm_add_hwrng(struct tpm_chip *chip) { - if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) || - chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) + if (!tpm_is_hwrng_enabled(chip)) return 0; snprintf(chip->hwrng_name, sizeof(chip->hwrng_name), @@ -629,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_hwrng_enabled(chip)) hwrng_unregister(&chip->hwrng); out_ppi: tpm_bios_log_teardown(chip); @@ -654,8 +664,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) && - !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)) + if (tpm_is_hwrng_enabled(chip)) hwrng_unregister(&chip->hwrng); tpm_bios_log_teardown(chip); if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip))
The same checks are repeated in 3 places to decide whether to use hwrng. Consolidate these into a helper. Also this fixes a case that one of them was missing a check in the cleanup path. Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs") Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/char/tpm/tpm-chip.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)