On Tue Aug 1, 2023 at 9:52 PM EEST, Jarkko Sakkinen wrote: > On Tue Aug 1, 2023 at 6:04 AM EEST, Mario Limonciello wrote: > > On 7/31/23 18:40, Jason A. Donenfeld wrote: > > > Hi all, > > > > > > I've been tracking this issue with Mario on various threads and > > > bugzilla for a while now. My suggestion over at bugzilla was to just > > > disable all current AMD fTPMs by bumping the check for a major version > > > number, so that the hardware people can reenable it i it's ever fixed, > > > but only if this is something that the hardware people would actually > > > respect. As I understand it, Mario was going to check into it and see. > > > Failing that, yea, just disabling hwrng on fTPM seems like a fine > > > enough thing to do. > > > > > > The reason I'm not too concerned about that is twofold: > > > - Systems with fTPM all have RDRAND anyway, so there's no entropy problem. > > > - fTPM *probably* uses the same random source as RDRAND -- the > > > TRNG_OUT MMIO register -- so it's not really doing much more than what > > > we already have available. > > > > Yeah I have conversations ongoing about this topic, but also I concluded > > your suspicion is correct. They both get their values from the > > integrated CCP HW IP. > > > > > > > > So this all seems fine. And Jarkko's patch seems more or less the > > > straight forward way of disabling it. But with that said, in order of > > > priority, maybe we should first try these: > > > > > > 1) Adjust the version check to a major-place fTPM version that AMD's > > > hardware team pinky swears will have this bug fixed. (Though, I can > > > already imagine somebody on the list shouting, "we don't trust > > > hardware teams to do anything with unreleased stuff!", which could be > > > valid.) > > > > I find it very likely the actual root cause is similar to what Linus > > suggested. If that's the case I don't think the bug can be fixed > > by just an fTPM fix but would rather require a BIOS fix. > > > > This to me strengthens the argument to either not register fTPM as RNG > > in the first place or just use TPM for boot time entropy. > > > > > 2) Remove the version check, but add some other query to detect AMD > > > fTPM vs realTPM, and ban fTPM. > > > > AMD doesn't make dTPMs, only fTPMs. It's tempting to try to use > > TPM2_PT_VENDOR_TPM_TYPE, but this actually is a vendor specific value. > > > > I don't see a reliable way in the spec to do this. > > > > > - Remove the version check, and just check for AMD; this is Jarrko's patch. > > > > I have a counter-proposal to Jarkko's patch attached. This has two > > notable changes: > > > > 1) It only disables RNG generation in the case of having RDRAND or RDSEED. > > 2) It also matches Intel PTT. > > > > I still do also think Linus' idea of TPMs only providing boot time > > entropy is worth weighing out. > > You should add something like TPM_CHIP_HWRNG_DISABLED instead and > set this in tpm_crb before calling tpm_chip_register(). > > Nothing else concerning AMD hardware should be done in tpm-chip.c. > It should only check TPM_CHIP_HWRNG_DISABLED in the beginning of > tpm_add_hwrng(). In English: I think adding the function to tpm-chip.c was a really bad idea in the first place, so let's revert that decisions and do this correctly in tpm_crb.c. BR, Jarkko
