From: Vadim Sukhomlinov <[email protected]>
[ Upstream commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 ]
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.
Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
Cc: [email protected]
Signed-off-by: Vadim Sukhomlinov <[email protected]>
[dianders: resolved merge conflicts with mainline]
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/char/tpm/tpm-chip.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 46caadca916a0..0b01eb7b14e53 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -187,12 +187,13 @@ static int tpm_class_shutdown(struct device *dev)
{
struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
+ down_write(&chip->ops_sem);
if (chip->flags & TPM_CHIP_FLAG_TPM2) {
- down_write(&chip->ops_sem);
tpm2_shutdown(chip, TPM2_SU_CLEAR);
chip->ops = NULL;
- up_write(&chip->ops_sem);
}
+ chip->ops = NULL;
+ up_write(&chip->ops_sem);
return 0;
}
--
2.20.1
Hi,
On Tue, Sep 3, 2019 at 9:28 AM Sasha Levin <[email protected]> wrote:
>
> From: Vadim Sukhomlinov <[email protected]>
>
> [ Upstream commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 ]
>
> 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.
>
> Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
> Cc: [email protected]
> Signed-off-by: Vadim Sukhomlinov <[email protected]>
> [dianders: resolved merge conflicts with mainline]
> Signed-off-by: Douglas Anderson <[email protected]>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> drivers/char/tpm/tpm-chip.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Jarkko: did you deal with the issues that came up in response to my
post? Are you happy with this going into 4.19 stable at this point?
I notice this has your Signed-off-by so maybe?
-Doug
On Tue Sep 03 19, Doug Anderson wrote:
>Hi,
>
>On Tue, Sep 3, 2019 at 9:28 AM Sasha Levin <[email protected]> wrote:
>>
>> From: Vadim Sukhomlinov <[email protected]>
>>
>> [ Upstream commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 ]
>>
>> 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.
>>
>> Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
>> Cc: [email protected]
>> Signed-off-by: Vadim Sukhomlinov <[email protected]>
>> [dianders: resolved merge conflicts with mainline]
>> Signed-off-by: Douglas Anderson <[email protected]>
>> Reviewed-by: Jarkko Sakkinen <[email protected]>
>> Signed-off-by: Jarkko Sakkinen <[email protected]>
>> Signed-off-by: Sasha Levin <[email protected]>
>> ---
>> drivers/char/tpm/tpm-chip.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>Jarkko: did you deal with the issues that came up in response to my
>post? Are you happy with this going into 4.19 stable at this point?
>I notice this has your Signed-off-by so maybe?
>
I think that is just the signed-off-by chain coming from the upstream patch.
Jarkko mentioned getting to the backports after Linux Plumbers, which is next week.
>-Doug
On Tue, Sep 03, 2019 at 09:53:46AM -0700, Jerry Snitselaar wrote:
>On Tue Sep 03 19, Doug Anderson wrote:
>>Hi,
>>
>>On Tue, Sep 3, 2019 at 9:28 AM Sasha Levin <[email protected]> wrote:
>>>
>>>From: Vadim Sukhomlinov <[email protected]>
>>>
>>>[ Upstream commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 ]
>>>
>>>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.
>>>
>>>Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
>>>Cc: [email protected]
>>>Signed-off-by: Vadim Sukhomlinov <[email protected]>
>>>[dianders: resolved merge conflicts with mainline]
>>>Signed-off-by: Douglas Anderson <[email protected]>
>>>Reviewed-by: Jarkko Sakkinen <[email protected]>
>>>Signed-off-by: Jarkko Sakkinen <[email protected]>
>>>Signed-off-by: Sasha Levin <[email protected]>
>>>---
>>> drivers/char/tpm/tpm-chip.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>>Jarkko: did you deal with the issues that came up in response to my
>>post? Are you happy with this going into 4.19 stable at this point?
>>I notice this has your Signed-off-by so maybe?
>>
>
>I think that is just the signed-off-by chain coming from the upstream patch.
>Jarkko mentioned getting to the backports after Linux Plumbers, which is next week.
Right. I gave a go at backporting a few patches and this happens to be
one of them. It will be a while before it goes in a stable tree
(probably way after after LPC).
--
Thanks,
Sasha
On Tue, 2019-09-03 at 09:39 -0700, Doug Anderson wrote:
> Hi,
>
> On Tue, Sep 3, 2019 at 9:28 AM Sasha Levin <[email protected]> wrote:
> > From: Vadim Sukhomlinov <[email protected]>
> >
> > [ Upstream commit db4d8cb9c9f2af71c4d087817160d866ed572cc9 ]
> >
> > 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.
> >
> > Fixes: d1bd4a792d39 ("tpm: Issue a TPM2_Shutdown for TPM2 devices.")
> > Cc: [email protected]
> > Signed-off-by: Vadim Sukhomlinov <[email protected]>
> > [dianders: resolved merge conflicts with mainline]
> > Signed-off-by: Douglas Anderson <[email protected]>
> > Reviewed-by: Jarkko Sakkinen <[email protected]>
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > Signed-off-by: Sasha Levin <[email protected]>
> > ---
> > drivers/char/tpm/tpm-chip.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
>
> Jarkko: did you deal with the issues that came up in response to my
> post? Are you happy with this going into 4.19 stable at this point?
> I notice this has your Signed-off-by so maybe?
No I have not dealt with the issues yet. The last thing I've said about
this is:
https://lore.kernel.org/stable/[email protected]/
I was actually going to look into this during the plane trip to Lissabon
tomorrow morning. I have in mind that this needs to be backported first:
db4d8cb9c9f2 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
/Jarkko
On Tue, 2019-09-03 at 15:43 -0400, Sasha Levin wrote:
> Right. I gave a go at backporting a few patches and this happens to be
> one of them. It will be a while before it goes in a stable tree
> (probably way after after LPC).
It *semantically* depends on
db4d8cb9c9f2 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
I.e. can cause crashes without the above patch. As a code change your
patch is fine but it needs the above patch backported to work in stable
manner.
So... either I can backport that one (because ultimately I have
responsibility to do that as the maintainer) but if you want to finish
this one that is what you need to backport in addition and then it
should be fine.
/Jarkko
On Sat, Sep 07, 2019 at 09:55:18PM +0300, Jarkko Sakkinen wrote:
>On Tue, 2019-09-03 at 15:43 -0400, Sasha Levin wrote:
>> Right. I gave a go at backporting a few patches and this happens to be
>> one of them. It will be a while before it goes in a stable tree
>> (probably way after after LPC).
>
>It *semantically* depends on
>
>db4d8cb9c9f2 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
>
>I.e. can cause crashes without the above patch. As a code change your
>patch is fine but it needs the above patch backported to work in stable
>manner.
>
>So... either I can backport that one (because ultimately I have
>responsibility to do that as the maintainer) but if you want to finish
>this one that is what you need to backport in addition and then it
>should be fine.
If you're ok with the backport of this commit, I can just add
db4d8cb9c9f2 on top.
--
Thanks,
Sasha
On Sat, Sep 07, 2019 at 06:04:48PM -0400, Sasha Levin wrote:
> On Sat, Sep 07, 2019 at 09:55:18PM +0300, Jarkko Sakkinen wrote:
> > On Tue, 2019-09-03 at 15:43 -0400, Sasha Levin wrote:
> > > Right. I gave a go at backporting a few patches and this happens to be
> > > one of them. It will be a while before it goes in a stable tree
> > > (probably way after after LPC).
> >
> > It *semantically* depends on
> >
> > db4d8cb9c9f2 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
> >
> > I.e. can cause crashes without the above patch. As a code change your
> > patch is fine but it needs the above patch backported to work in stable
> > manner.
> >
> > So... either I can backport that one (because ultimately I have
> > responsibility to do that as the maintainer) but if you want to finish
> > this one that is what you need to backport in addition and then it
> > should be fine.
>
> If you're ok with the backport of this commit, I can just add
> db4d8cb9c9f2 on top.
Sure, I've already gave my promise to do that :-)
/Jarkko
On Mon, Sep 09, 2019 at 05:28:08PM +0100, Jarkko Sakkinen wrote:
>On Sat, Sep 07, 2019 at 06:04:48PM -0400, Sasha Levin wrote:
>> On Sat, Sep 07, 2019 at 09:55:18PM +0300, Jarkko Sakkinen wrote:
>> > On Tue, 2019-09-03 at 15:43 -0400, Sasha Levin wrote:
>> > > Right. I gave a go at backporting a few patches and this happens to be
>> > > one of them. It will be a while before it goes in a stable tree
>> > > (probably way after after LPC).
>> >
>> > It *semantically* depends on
>> >
>> > db4d8cb9c9f2 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
>> >
>> > I.e. can cause crashes without the above patch. As a code change your
>> > patch is fine but it needs the above patch backported to work in stable
>> > manner.
>> >
>> > So... either I can backport that one (because ultimately I have
>> > responsibility to do that as the maintainer) but if you want to finish
>> > this one that is what you need to backport in addition and then it
>> > should be fine.
>>
>> If you're ok with the backport of this commit, I can just add
>> db4d8cb9c9f2 on top.
>
>Sure, I've already gave my promise to do that :-)
I think that the dependency in question is actually:
2677ca98ae377 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
Which is tricky to backport. I think I'll drop this patch for now and
wait for your backport instead.
--
Thanks,
Sasha
On Mon, Sep 09, 2019 at 05:28:08PM +0100, Jarkko Sakkinen wrote:
> On Sat, Sep 07, 2019 at 06:04:48PM -0400, Sasha Levin wrote:
> > On Sat, Sep 07, 2019 at 09:55:18PM +0300, Jarkko Sakkinen wrote:
> > > On Tue, 2019-09-03 at 15:43 -0400, Sasha Levin wrote:
> > > > Right. I gave a go at backporting a few patches and this happens to be
> > > > one of them. It will be a while before it goes in a stable tree
> > > > (probably way after after LPC).
> > >
> > > It *semantically* depends on
> > >
> > > db4d8cb9c9f2 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
> > >
> > > I.e. can cause crashes without the above patch. As a code change your
> > > patch is fine but it needs the above patch backported to work in stable
> > > manner.
> > >
> > > So... either I can backport that one (because ultimately I have
> > > responsibility to do that as the maintainer) but if you want to finish
> > > this one that is what you need to backport in addition and then it
> > > should be fine.
> >
> > If you're ok with the backport of this commit, I can just add
> > db4d8cb9c9f2 on top.
>
> Sure, I've already gave my promise to do that :-)
I ended up with:
db4d8cb9c9f2 tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations
2677ca98ae37 tpm: use tpm_try_get_ops() in tpm-sysfs.c.
da379f3c1db0 tpm: migrate pubek_show to struct tpm_buf
Since some time has passed I'l just restate that the reason for
backporting 2677ca98ae37 was that tpm_class_shutdown() could pull carpet
under the TPM 1.2 code. tpm_try_get_ops() makes sure that read lock is
taken and chip->ops is not NULL if it successfully returns.
Still need to test the patches with TPM 1.2 hardware before I can send
them.
/Jarkko