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

[Jarkko Sakkinen <jarkko@kernel.org>]

On Wed, Jul 02, 2025 at 04:28:51PM -0400, Denis Aleksandrov wrote:
> This bug is not seen on most machines. 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 restricting unprivileged user from reading the
> above-mentioned device files.

I suppose we can do this. I'm going to holiday for one week next
week so I'll hold for additional feedback for that period and
apply this if nothing comes up.

There's no use case for unprivileged user, or app that stops
working because of this. If you cut hairs, with patch shifting
uapi you have to we always prepared that tree falls down
somewhere but I'm willing to take risk with this :-)


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

> 
> Reported-by: Jan Stancek <jstancek@redhat.com>
> Signed-off-by: Denis Aleksandrov <daleksan@redhat.com>
> ---
> 
> Running scripts/checkpatch.pl suggested that the permissions be
> changed to octal format. What do the maintainers think of this?
> The rest of the permissions in the file are macros.
> 
> Lastly, this bug was reproduced and the fix was tested accordingly.
> 
>  drivers/char/tpm/tpm_ppi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
> index bc7b1b4501b3..ac6e0aee566e 100644
> --- a/drivers/char/tpm/tpm_ppi.c
> +++ b/drivers/char/tpm/tpm_ppi.c
> @@ -347,8 +347,8 @@ static DEVICE_ATTR(request, S_IRUGO | S_IWUSR | S_IWGRP,
>  static DEVICE_ATTR(transition_action, S_IRUGO,
>  		   tpm_show_ppi_transition_action, NULL);
>  static DEVICE_ATTR(response, S_IRUGO, tpm_show_ppi_response, NULL);
> -static DEVICE_ATTR(tcg_operations, S_IRUGO, tpm_show_ppi_tcg_operations, NULL);
> -static DEVICE_ATTR(vs_operations, S_IRUGO, tpm_show_ppi_vs_operations, NULL);
> +static DEVICE_ATTR(tcg_operations, S_IRUSR | S_IRGRP, tpm_show_ppi_tcg_operations, NULL);
> +static DEVICE_ATTR(vs_operations, S_IRUSR | S_IRGRP, tpm_show_ppi_vs_operations, NULL);
>  
>  static struct attribute *ppi_attrs[] = {
>  	&dev_attr_version.attr,
> -- 
> 2.48.1
> 

BR, Jarkko