Re: [PATCH] tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations

From: Doug Anderson
Date: Thu Jun 13 2019 - 11:25:43 EST


Hi,

On Thu, Jun 13, 2019 at 6:59 AM Jarkko Sakkinen
<jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote:
>
> On Wed, Jun 12, 2019 at 10:16:18PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Jun 10, 2019 at 03:01:18PM -0700, Douglas Anderson wrote:
> > > From: Vadim Sukhomlinov <sukhomlinov@xxxxxxxxxx>
> > >
> > > TPM 2.0 Shutdown involve sending TPM2_Shutdown to TPM chip and disabling
> > > future TPM operations. TPM 1.2 behavior was different, future TPM
> > > operations weren't disabled, causing rare issues. This patch ensures
> > > that future TPM operations are disabled.
> > >
> > > Signed-off-by: Vadim Sukhomlinov <sukhomlinov@xxxxxxxxxx>
> > > [dianders: resolved merge conflicts with mainline]
> > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> >
> > Nice catch. Thank you.
> >
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
>
> Applied to my master branch. I also added a fixes tag.
>
> Can you check that it looks legit to you?

Found the patch in your tree at
<http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/41f15a4f02092d531fb34b42a06e9a1603a7df27>.
I'm decidedly a non-expert here, mostly just wrangling a patch that
someone else came up with. :-) ...but let's see...

I think you're asking if the "Fixes" looks sane. I guess it depends
on what you're trying to accomplish. Certainly what you've tagged in
"Fixes" marks the point where it would be easiest to backport this fix
to. ...but I think the problem is much older than that patch.

As I understand it, this problem has existed for much longer. I
believe that ${SUBJECT} patch evolved from an investigation that Luigi
Semenzato did back in 2013 when we got back some Chromebooks whose
TPMs claimed that they had been "attacked". Said another way, I
believe it is an evolution of the patch <https://crrev.com/c/57988>
("CHROMIUM: workaround for Infineon TPM broken defensive timeout").

...so technically someone ought to want this on all old kernels.
Maybe keep the "Cc: stable" but remove the "Fixes"?


-Doug