Ping.
Just checking in on this patch.
It has received a "Reviewed-by" tag, and I was wondering if there is
anything else needed from my side for it to be picked up.
Thanks,
Denis
On Thu, Jul 3, 2025 at 8:00 AM Denis Aleksandrov <dalek...@redhat.com> wrote:
>
> Hi Jarkko,
>
> Thank you for the review. I'll add your Reviewed-by tag to my local commit.
> Please let me know if you would like me to send a v2 version of the
> patch with your tag included.
>
> Best,
> Denis
>
>
> On Thu, Jul 3, 2025 at 7:56 AM Denis Aleksandrov <dalek...@redhat.com> wrote:
> >
> > Hi Jarkko,
> >
> > Thank you for the review. I'll add your Reviewed-by tag to my local commit.
> > Please let me know if you would like me to send a v2 version of the patch
> > with your tag included.
> >
> > Best,
> > Denis
> >
> > On Wed, Jul 2, 2025 at 6:46 PM Jarkko Sakkinen <jar...@kernel.org> wrote:
> >>
> >> 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 <jar...@kernel.org>
> >>
> >> >
> >> > Reported-by: Jan Stancek <jstan...@redhat.com>
> >> > Signed-off-by: Denis Aleksandrov <dalek...@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
> >>
