| Message ID | 20220425102552.663069-1-marten.lindahl@axis.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tpm: Add check for Failure mode for TPM2 modules | expand |
On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > TPM2 modules") it was said that: > > "If the TPM is in Failure mode, it will successfully respond to both > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > is in Failure mode." > > But a check was never added in the commit when calling > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > This commit corrects this by adding a check. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> I think this should actually have the fixes tag. It's a regression, right? I can add it. BR, Jarkko
On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote: > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > > TPM2 modules") it was said that: > > > > "If the TPM is in Failure mode, it will successfully respond to both > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > > is in Failure mode." > > > > But a check was never added in the commit when calling > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > > This commit corrects this by adding a check. > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > I think this should actually have the fixes tag. It's a regression, > right? I can add it. Hi Jarkko! Thank you. Yes it should have a fixes tag for the mentioned commit. Kind regards Mårten > > BR, Jarkko
> On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote: > > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > > > TPM2 modules") it was said that: > > > > > > "If the TPM is in Failure mode, it will successfully respond to both > > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > > > is in Failure mode." > > > > > > But a check was never added in the commit when calling > > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > > > This commit corrects this by adding a check. > > > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > I think this should actually have the fixes tag. It's a regression, > > right? I can add it. > > Hi Jarkko! > > Thank you. Yes it should have a fixes tag for the mentioned commit. > > Kind regards > Mårten > > > > > BR, Jarkko Hi Mårten, your patch introduces redundant code. The code block of the if below contains exactly the same code. Additionally, your patch masks a bug (derefencing uninitialized memory). I will send a patch set shortly. Regards Stefan
On Fri, May 06, 2022 at 02:00:41PM +0200, Stefan Mahnke-Hartmann wrote: > > On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote: > > > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > > > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > > > > TPM2 modules") it was said that: > > > > > > > > "If the TPM is in Failure mode, it will successfully respond to both > > > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > > > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > > > > is in Failure mode." > > > > > > > > But a check was never added in the commit when calling > > > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > > > > This commit corrects this by adding a check. > > > > > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > > > I think this should actually have the fixes tag. It's a regression, > > > right? I can add it. > > > > Hi Jarkko! > > > > Thank you. Yes it should have a fixes tag for the mentioned commit. > > > > Kind regards > > Mårten > > > > > > > > BR, Jarkko > > Hi Mårten, > > your patch introduces redundant code. The code block of the if below > contains exactly the same code. Hi! The checks are for different cases. The TPM may sucessfully pass startup and selftest but still be in failure mode. In this case only some capabilities are required to work (Trusted Platform Module Library Specification, Family "2.0", Level 00, Revision 01.59 – November 2019 Part 3, page 348 and 349), but other capabilities may fail. So in order for userspace to handle the TPM, this check is needed. The already existing check (below the out label) is specifically for upgrade mode. Yes the code blocks looks very similar, though the print is different. > > Additionally, your patch masks a bug (derefencing uninitialized > memory). I will send a patch set shortly. Masking bugs are never good. Please send a patch for this. Kind regards Mårten > > Regards > Stefan
On Mon, Apr 25, 2022 at 05:52:04PM +0200, Mårten Lindahl wrote: > On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote: > > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > > > TPM2 modules") it was said that: > > > > > > "If the TPM is in Failure mode, it will successfully respond to both > > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > > > is in Failure mode." > > > > > > But a check was never added in the commit when calling > > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > > > This commit corrects this by adding a check. > > > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > I think this should actually have the fixes tag. It's a regression, > > right? I can add it. Hi Jarkko! Just curious. I can't see this patch was added to linux-next. Was I expected to do something more to it? Kind regards Mårten > > Hi Jarkko! > > Thank you. Yes it should have a fixes tag for the mentioned commit. > > Kind regards > Mårten > > > > > BR, Jarkko
On Mon, 2022-05-30 at 17:26 +0200, Marten Lindahl wrote: > On Mon, Apr 25, 2022 at 05:52:04PM +0200, Mårten Lindahl wrote: > > On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote: > > > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > > > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > > > > TPM2 modules") it was said that: > > > > > > > > "If the TPM is in Failure mode, it will successfully respond to both > > > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > > > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > > > > is in Failure mode." > > > > > > > > But a check was never added in the commit when calling > > > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > > > > This commit corrects this by adding a check. > > > > > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > > > I think this should actually have the fixes tag. It's a regression, > > > right? I can add it. > > Hi Jarkko! > > Just curious. I can't see this patch was added to linux-next. Was I > expected to do something more to it? No it's just me failing to do my job properly. I'll pick this and make a 2nd pull request v5.19. Apologies. BR, Jarkko
On Wed, Jun 01, 2022 at 05:05:08AM +0300, Jarkko Sakkinen wrote: > On Mon, 2022-05-30 at 17:26 +0200, Marten Lindahl wrote: > > On Mon, Apr 25, 2022 at 05:52:04PM +0200, Mårten Lindahl wrote: > > > On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote: > > > > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > > > > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > > > > > TPM2 modules") it was said that: > > > > > > > > > > "If the TPM is in Failure mode, it will successfully respond to both > > > > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > > > > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > > > > > is in Failure mode." > > > > > > > > > > But a check was never added in the commit when calling > > > > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > > > > > This commit corrects this by adding a check. > > > > > > > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > > > > > I think this should actually have the fixes tag. It's a regression, > > > > right? I can add it. > > > > Hi Jarkko! > > > > Just curious. I can't see this patch was added to linux-next. Was I > > expected to do something more to it? > > No it's just me failing to do my job properly. I'll pick this and > make a 2nd pull request v5.19. Apologies. I revisited the patch, and it does not look right to me. If tpm2_get_cc_attrs_tbl() fails, there's no way to deduce from that "fact" that the TPM response was TPM_RC_FAILURE. BR, Jarkko
On Thu, Jun 02, 2022 at 01:15:25PM +0200, Jarkko Sakkinen wrote: > On Wed, Jun 01, 2022 at 05:05:08AM +0300, Jarkko Sakkinen wrote: > > On Mon, 2022-05-30 at 17:26 +0200, Marten Lindahl wrote: > > > On Mon, Apr 25, 2022 at 05:52:04PM +0200, Mårten Lindahl wrote: > > > > On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote: > > > > > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote: > > > > > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for > > > > > > TPM2 modules") it was said that: > > > > > > > > > > > > "If the TPM is in Failure mode, it will successfully respond to both > > > > > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to > > > > > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM > > > > > > is in Failure mode." > > > > > > > > > > > > But a check was never added in the commit when calling > > > > > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. > > > > > > This commit corrects this by adding a check. > > > > > > > > > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > > > > > > > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > > > > > > > I think this should actually have the fixes tag. It's a regression, > > > > > right? I can add it. > > > > > > Hi Jarkko! > > > > > > Just curious. I can't see this patch was added to linux-next. Was I > > > expected to do something more to it? > > > > No it's just me failing to do my job properly. I'll pick this and > > make a 2nd pull request v5.19. Apologies. > > I revisited the patch, and it does not look right to me. > Hi Jarkko! I'm sorry for the delay. > If tpm2_get_cc_attrs_tbl() fails, there's no way to deduce > from that "fact" that the TPM response was TPM_RC_FAILURE. Yes, that is true. If the TPM responds with TPM2_RC_FAILURE it should be caught here. I will add it. Kind regards Mårten > > BR, Jarkko
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index 4704fa553098..9da433521a0b 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -743,6 +743,11 @@ int tpm2_auto_startup(struct tpm_chip *chip) } rc = tpm2_get_cc_attrs_tbl(chip); + if (rc < 0 && rc != -ENOMEM) { + dev_info(&chip->dev, "TPM in field failure mode, requires firmware upgrade\n"); + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_UPGRADE; + rc = 0; + } out: if (rc == TPM2_RC_UPGRADE) {
In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for TPM2 modules") it was said that: "If the TPM is in Failure mode, it will successfully respond to both tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM is in Failure mode." But a check was never added in the commit when calling tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode. This commit corrects this by adding a check. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- drivers/char/tpm/tpm2-cmd.c | 5 +++++ 1 file changed, 5 insertions(+)