[PATCH v3 0/2] tpm: Some fixes

[PATCH v3 0/2] tpm: Some fixes

[Stefan Berger]

This series of patches fixes the TPM SPAPR device model so that it reacts
in the same way as the other device models do when the backend device did
not start up properly. It now calls exit(1).

Due to a change in the TPM 2 code, the pcrUpdateCounter (14th byte) in the
TPM2_Pcrread response now returns a different value than before. So it's
better to skip the 14th byte when comparing expected against actual responses.

   Stefan

v2->v3:
  - more elaborate commit messages

v1->v2:
  - simplified skipping of 14th byte in response


Stefan Berger (2):
  tpm: tpm_spapr: Exit on TPM backend failures
  tests: tpm: Skip over pcrUpdateCounter byte in result comparison

 hw/tpm/tpm_spapr.c     | 5 ++++-
 tests/qtest/tpm-util.c | 6 +++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.24.1

[PATCH v3 1/2] tpm: tpm_spapr: Exit on TPM backend failures

[Stefan Berger]

Exit on TPM backend failures in the same way as the TPM CRB and TIS device
models do. With this change we now get an error report when the backend
did not start up properly:

error: internal error: qemu unexpectedly closed the monitor:
2020-07-07T12:49:28.333928Z qemu-system-ppc64: tpm-emulator: \
  TPM result for CMD_INIT: 0x101 operation failed

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_spapr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
index cb4dfd1e6a..8288ab0a15 100644
--- a/hw/tpm/tpm_spapr.c
+++ b/hw/tpm/tpm_spapr.c
@@ -306,7 +306,10 @@ static void tpm_spapr_reset(SpaprVioDevice *dev)
                             TPM_SPAPR_BUFFER_MAX);
 
     tpm_backend_reset(s->be_driver);
-    tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
+
+    if (tpm_spapr_do_startup_tpm(s, s->be_buffer_size) < 0) {
+        exit(1);
+    }
 }
 
 static enum TPMVersion tpm_spapr_get_version(TPMIf *ti)
-- 
2.24.1

[PATCH v3 2/2] tests: tpm: Skip over pcrUpdateCounter byte in result comparison

[Stefan Berger]

The TPM 2 code in libtpms was fixed to handle the PCR 'TCB group' according
to the PCClient profile. The change of the PCRs belonging to the 'TCB group'
now affects the pcrUpdateCounter in the TPM2_PCRRead() responses where its
value is now different (typically lower by '1') than what it was before. To
not fail the tests, we skip the comparison of the 14th byte, which
represents the pcrUpdateCounter.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 tests/qtest/tpm-util.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/tpm-util.c b/tests/qtest/tpm-util.c
index 34efae8f18..58a9593745 100644
--- a/tests/qtest/tpm-util.c
+++ b/tests/qtest/tpm-util.c
@@ -139,7 +139,11 @@ void tpm_util_pcrread(QTestState *s, tx_func *tx,
 
     tx(s, tpm_pcrread, sizeof(tpm_pcrread), buffer, sizeof(buffer));
 
-    g_assert_cmpmem(buffer, exp_resp_size, exp_resp, exp_resp_size);
+    /* skip pcrUpdateCounter (14th byte) in comparison */
+    g_assert(exp_resp_size >= 15);
+    g_assert_cmpmem(buffer, 13, exp_resp, 13);
+    g_assert_cmpmem(&buffer[14], exp_resp_size - 14,
+                    &exp_resp[14], exp_resp_size - 14);
 }
 
 bool tpm_util_swtpm_has_tpm2(void)
-- 
2.24.1

Re: [PATCH v3 2/2] tests: tpm: Skip over pcrUpdateCounter byte in result comparison

[Marc-André Lureau]

On Wed, Jul 8, 2020 at 12:16 AM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> The TPM 2 code in libtpms was fixed to handle the PCR 'TCB group' according
> to the PCClient profile. The change of the PCRs belonging to the 'TCB group'
> now affects the pcrUpdateCounter in the TPM2_PCRRead() responses where its
> value is now different (typically lower by '1') than what it was before. To
> not fail the tests, we skip the comparison of the 14th byte, which
> represents the pcrUpdateCounter.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  tests/qtest/tpm-util.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/tpm-util.c b/tests/qtest/tpm-util.c
> index 34efae8f18..58a9593745 100644
> --- a/tests/qtest/tpm-util.c
> +++ b/tests/qtest/tpm-util.c
> @@ -139,7 +139,11 @@ void tpm_util_pcrread(QTestState *s, tx_func *tx,
>
>      tx(s, tpm_pcrread, sizeof(tpm_pcrread), buffer, sizeof(buffer));
>
> -    g_assert_cmpmem(buffer, exp_resp_size, exp_resp, exp_resp_size);
> +    /* skip pcrUpdateCounter (14th byte) in comparison */
> +    g_assert(exp_resp_size >= 15);
> +    g_assert_cmpmem(buffer, 13, exp_resp, 13);
> +    g_assert_cmpmem(&buffer[14], exp_resp_size - 14,
> +                    &exp_resp[14], exp_resp_size - 14);
>  }
>
>  bool tpm_util_swtpm_has_tpm2(void)
> --
> 2.24.1
>

Re: [PATCH v3 1/2] tpm: tpm_spapr: Exit on TPM backend failures

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]

Hi

On Wed, Jul 8, 2020 at 12:17 AM Stefan Berger <stefanb@linux.vnet.ibm.com>
wrote:

> Exit on TPM backend failures in the same way as the TPM CRB and TIS device
> models do. With this change we now get an error report when the backend
> did not start up properly:
>
> error: internal error: qemu unexpectedly closed the monitor:
> 2020-07-07T12:49:28.333928Z qemu-system-ppc64: tpm-emulator: \
>   TPM result for CMD_INIT: 0x101 operation failed
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  hw/tpm/tpm_spapr.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
> index cb4dfd1e6a..8288ab0a15 100644
> --- a/hw/tpm/tpm_spapr.c
> +++ b/hw/tpm/tpm_spapr.c
> @@ -306,7 +306,10 @@ static void tpm_spapr_reset(SpaprVioDevice *dev)
>                              TPM_SPAPR_BUFFER_MAX);
>
>      tpm_backend_reset(s->be_driver);
> -    tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
> +
> +    if (tpm_spapr_do_startup_tpm(s, s->be_buffer_size) < 0) {
> +        exit(1);
> +    }
>

Not ideal, but consistent with CRB & TIS.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

 }
>
>  static enum TPMVersion tpm_spapr_get_version(TPMIf *ti)
> --
> 2.24.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2133 bytes --]