Re: [PATCH v2] tpm: prevents local DOS via tpm/tpm0/ppi/*operations

[Jarkko Sakkinen <jarkko@kernel.org>]

On Thu, Aug 28, 2025 at 11:35:31AM -0400, Denis Aleksandrov wrote:
> On Wed, Aug 27, 2025 at 8:48 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Wed, Aug 27, 2025 at 07:55:23AM +0200, Paul Menzel wrote:
> > > Dear Denis,
> > >
> > >
> > > Thank you for your patch. In the summary, I’d use imperative mood:
> >
> > +1
> >
> 
> I can add this in a v3.
> 
> > >
> > > tpm: Prevent local DOS …
> > >
> > > Am 27.08.25 um 04:21 schrieb Denis Aleksandrov:
> > > > Reads on tpm/tpm0/ppi/*operations can become very long on
> > > > misconfigured systems. Reading the TPM is a blocking operation,
> > > > thus a user could effectively trigger a DOS.
> > > >
> > > > Resolve this by caching the results and avoiding the blocking
> > > > operations after the first read.
> > >
> > > If you could elaborate, how to test this, and in possible error cases, how
> > > to debug this – for example, how to disable the cache–, that’d be great.
> >
> > +1
> >
> 
> The issue is that this bug is not replicable on most systems, but the way that
> I've been able to test it is by running the following:
> $ time cat /sys/devices/pnp0/00:0a/tpm/tpm0/ppi/tcg_operations
> and
> $ time cat /sys/devices/pnp0/00:0a/tpm/tpm0/ppi/vs_operations
> On a system that I know is experiencing the DOS symptom.
> 
> For debugging, I've been using an unpatched kernel and running the same
> commands.
> 
> > >
> > > >
> > > > Reported-by: Jan Stancek <jstancek@redhat.com>
> > > > Signed-off-by: Denis Aleksandrov <daleksan@redhat.com>
> 
> I'll make sure to add the Suggested-by tag in the future, and the v3.
> Sorry about that.
> 
> > > > ---
> > > >
> > > > Changes in v2:
> > > >   - Replaced file permission change with a caching mechanism as
> > > >     suggested by Jarkko.
> > > >
> > > >   drivers/char/tpm/tpm_ppi.c | 88 ++++++++++++++++++++++++++++----------
> > > >   1 file changed, 65 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
> > > > index d53fce1c9d6f..e0212893748e 100644
> > > > --- a/drivers/char/tpm/tpm_ppi.c
> > > > +++ b/drivers/char/tpm/tpm_ppi.c
> > > > @@ -33,6 +33,21 @@ static const guid_t tpm_ppi_guid =
> > > >     GUID_INIT(0x3DDDFAA6, 0x361B, 0x4EB4,
> > > >               0xA4, 0x24, 0x8D, 0x10, 0x08, 0x9D, 0x16, 0x53);
> > > > +static const char * const tpm_ppi_info[] = {
> > > > +   "Not implemented",
> > > > +   "BIOS only",
> > > > +   "Blocked for OS by BIOS",
> > >
> > > Is this x86 specific? If not maybe use *system firmware*?
> > >
> 
> This was the original implementation, but I can change the info message to
> be more general. I can add it to the v3.

Sure, no need for apologies it was just a remark :-)

Go ahead and send v3. I'll test that version.

BR, Jarkko