On Mon, Oct 27, 2025 at 07:50:46AM -0400, Mimi Zohar wrote: > On Fri, 2025-10-24 at 21:55 +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 20, 2025 at 12:30:32PM +0100, Jonathan McDowell wrote: > > > I hit a problem where ~ 1% of TPM firmware upgrades were failing. > > > Investigating revealed the issue was that although the upgrade tool uses > > > /dev/tpm0 this does not actually prevent access via /dev/tpmrm0, nor > > > internal kernel users. It *does* prevent access to others via /dev/tpm0 > > > > > > So the upgrade process started, the HW RNG came in to get some > > > randomness in the middle, did the HMAC context dance, and confused > > > everything to the point the TPM was no longer visible to the OS even > > > after a reboot. > > > > > > Thankfully I've been able to recover those devices, but really what I'd > > > like is the ability for a userspace tool to exclusively access the TPM > > > without something coming in behind it. Given the lightweight attempt at > > > locking that already exists I think this was the original intention. > > > > > > I've reworked this series based on feedback received. > > > > > > Firstly, it's been reordered TPM sharing functionality doesn't break > > > during bisection. > > > > > > Secondly, the O_EXCL check has been tightend up to ensure the caller is > > > also opening the device O_RDWR. Callers shouldn't really be opening the > > > TPM except for reading + writing, but this should help guard against > > > unexpected flags usage a bit more. > > > > > > Finally, this revision keeps the prohibition on more than one user of > > > /dev/tpm#, to avoid unexpected breakages for clients that expect this to > > > guard against multiple invocations. A client only then needs to use > > > O_EXCL if it wants to prevent *all* other access, even with > > > ContextSaves, such as the firmware upgrade case. > > > > > > (I've sent a separate standalone patch that allows the TPM HW RNG to be > > > disabled at run time, and it's now in -next, but even with that I think > > > something like this is a good idea as well.) > > > > > > Jonathan McDowell (4): > > > tpm: Remove tpm_find_get_ops > > > tpm: Add O_EXCL for exclusive /dev/tpm access > > > tpm: Include /dev/tpmrm<n> when checking exclusive userspace TPM > > > access > > > tpm: Allow for exclusive TPM access when using /dev/tpm<n> > > > > > > drivers/char/tpm/tpm-chip.c | 90 +++++++++++++++---------------- > > > drivers/char/tpm/tpm-dev-common.c | 8 +-- > > > drivers/char/tpm/tpm-dev.c | 35 ++++++++++-- > > > drivers/char/tpm/tpm-dev.h | 1 + > > > drivers/char/tpm/tpm-interface.c | 20 +++++-- > > > drivers/char/tpm/tpm.h | 3 +- > > > drivers/char/tpm/tpm2-space.c | 5 +- > > > drivers/char/tpm/tpm_tis_core.c | 3 +- > > > drivers/char/tpm/tpmrm-dev.c | 20 ++++++- > > > include/linux/tpm.h | 4 +- > > > 10 files changed, 124 insertions(+), 65 deletions(-) > > > > > > -- > > > 2.51.0 > > > > > > > I will put to queue with my tags but I just want to make first sure that we > > do not > > break anything. > > > > I'll upgrade my test suite first to have TPM 1.2 tests (which is also > > needed for my own series) and run it in bunch of configurations. And on > > TPM2 I check the behavior with TSS2 daemon on / off. > > > > I have no doubts on the code changes, and it is most importantly a > > security improvement, given that "who has the access and how long" > > can be deduced for a system configuration. I just feel that with > > this code change it is better to check and verify everything :-) > > Roberto has already commented on this patch set, saying it would affect > IMA[1]. > I still need to look at the patch set, but please don't break IMA.
See my response in that thread. > > [1]https://lore.kernel.org/linux-integrity/[email protected]/ > > -- > thanks, > > Mimi BR, Jarkko
