Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18

[Jarkko Sakkinen <jarkko@kernel.org>]

On Sun, Oct 05, 2025 at 11:24:09AM -0700, Linus Torvalds wrote:
> On Sun, 5 Oct 2025 at 08:47, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > and apologies for this late pull request. This pull request disables
> > TCG_TPM2_HMAC from the default configuration as it does not perform well
> > enough
> 
> So having looked more at this, not only does it disable that
> TCG_TPM2_HMAC, it does a lot of other things too.
> 
> I really am going to require a better pull request, and I have thrown
> this one away.

What I think I think it is good call.

> 
> The exclusive access looks debatable to me too. I think you should
> also require that the open was done not only with O_EXCL, but as a
> write too.
> 
> Exclusive reads do not make sense.

True, I agree with this.

After reading this email I realized also another issue with these patch
when I tested them sequentially building a VM for each commit ID.

Without "tpm: Require O_EXCL for exclusive /dev/tpm access" applied,
there's a regression: usually a daemon of some sort opens /dev/tpm0:

COMMAND      PID USER   FD   TYPE DEVICE SIZE/OFF NODE NAME
tpm2-abrm 771444  tss    5u   CHR 10,224      0t0   94 /dev/tpm0

Without top patch this leaves /dev/tpmrm0 unusuable, which is a huge
developer experience downgrade as it is nice and covenient device
to try and do things. I.e. tail patch needs to be squashed and
the whole patch set needs to be re-reviewed.

I don't know systemd source code too well, but after eading
documentation of systemd-cryptenroll, it can also use /dev/tpmrm0:

https://www.freedesktop.org/software/systemd/man/latest/systemd-cryptenroll.html

Based on this it's now like there's a breaking patch and the top
most patch fixes the bug.

And based on this I'm happy to postpone O_EXCL changes to 6.19.
Patch set just needs to be restructured better so that in-the
middle of the series patches don't break things. And also it'd
be better if this patch would be relocated as the first in the
series: "tpm: Remove tpm_find_get_ops".


> 
> Now, I certainly *hope* that nobody has /dev/tmp being world-readable,
> so it probably doesn't matter, but that new exclusive access thing is
> very different than what the code used to do, and if I read it
> correctly it will also disable the kernel doing certain operations. So
> it needs to be as limited as possible.

Not disagreeing with this.

> 
> And damn it, it needs to be *explained*. Not have a pull request where
> one single line is explained badly.

'll send tomorrow a new PR without O_EXCL patches.

As per hmac encryption, I think my decision was the right call
but cover letter did suck agreed (sorry about that).

> 
>              Linus

BR, Jarkko