[Qemu-devel] [PULL 0/3] Merge tpm 2018/12/04 v1

[Qemu-devel] [PULL 0/3] Merge tpm 2018/12/04 v1

[Stefan Berger]

This series of patches removes an unnecessary parameter from tpm_tis_abort()
and adds a locality range check (using assert()) to tpm_tis_prep_abort() and
tpm_tis_request_completed().

   Stefan

The following changes since commit 83ea23cd207a03c5736be0231acbf7f8b05dbf52:

  i386: hvf: Fix overrun of _decode_tbl1 (2018-12-03 15:09:55 +0000)

are available in the Git repository at:

  git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2018-12-04-1

for you to fetch changes up to a639f96111eadb3b8e3021fd3f27e2948ad1c640:

  tpm: Make sure the locality received from backend is valid (2018-12-04 10:21:25 -0500)

----------------------------------------------------------------
Stefan Berger (3):
      tpm: Remove unused locty parameter from tpm_tis_abort()
      tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid
      tpm: Make sure the locality received from backend is valid

 hw/tpm/tpm_tis.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.17.1

[Qemu-devel] [PULL 1/3] tpm: Remove unused locty parameter from tpm_tis_abort()

[Stefan Berger]

Remove the unused locty parameter from tpm_tis_abort() function.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_tis.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d9322692ee..176d424ed9 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -233,7 +233,7 @@ static void tpm_tis_new_active_locality(TPMState *s, uint8_t new_active_locty)
 }
 
 /* abort -- this function switches the locality */
-static void tpm_tis_abort(TPMState *s, uint8_t locty)
+static void tpm_tis_abort(TPMState *s)
 {
     s->rw_offset = 0;
 
@@ -281,7 +281,7 @@ static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, uint8_t newlocty)
         }
     }
 
-    tpm_tis_abort(s, locty);
+    tpm_tis_abort(s);
 }
 
 /*
@@ -311,7 +311,7 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
     }
 
     if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
-        tpm_tis_abort(s, locty);
+        tpm_tis_abort(s);
     }
 
     tpm_tis_raise_irq(s, locty,
-- 
2.17.1

[Qemu-devel] [PULL 2/3] tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid

[Stefan Berger]

Make sure that the new locality passed to tpm_tis_prep_abort()
is valid.

Add a comment to aborting_locty that it may be any locality, including
TPM_TIS_NO_LOCALITY.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/tpm/tpm_tis.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 176d424ed9..04e4ad9212 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -263,7 +263,9 @@ static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, uint8_t newlocty)
 {
     uint8_t busy_locty;
 
-    s->aborting_locty = locty;
+    assert(TPM_TIS_IS_VALID_LOCTY(newlocty));
+
+    s->aborting_locty = locty; /* may also be TPM_TIS_NO_LOCALITY */
     s->next_locty = newlocty;  /* locality after successful abort */
 
     /*
-- 
2.17.1

[Qemu-devel] [PULL 3/3] tpm: Make sure the locality received from backend is valid

[Stefan Berger]

Make sure that the locality passed from the backend to
tpm_tis_request_completed() is valid.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/tpm/tpm_tis.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 04e4ad9212..2563d7501f 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -295,6 +295,8 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
     uint8_t locty = s->cmd.locty;
     uint8_t l;
 
+    assert(TPM_TIS_IS_VALID_LOCTY(locty));
+
     if (s->cmd.selftest_done) {
         for (l = 0; l < TPM_TIS_NUM_LOCALITIES; l++) {
             s->loc[l].sts |= TPM_TIS_STS_SELFTEST_DONE;
-- 
2.17.1

Re: [Qemu-devel] [PULL 0/3] Merge tpm 2018/12/04 v1

[Peter Maydell]

On Tue, 4 Dec 2018 at 15:34, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
> This series of patches removes an unnecessary parameter from tpm_tis_abort()
> and adds a locality range check (using assert()) to tpm_tis_prep_abort() and
> tpm_tis_request_completed().
>
>    Stefan
>
> The following changes since commit 83ea23cd207a03c5736be0231acbf7f8b05dbf52:
>
>   i386: hvf: Fix overrun of _decode_tbl1 (2018-12-03 15:09:55 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2018-12-04-1
>
> for you to fetch changes up to a639f96111eadb3b8e3021fd3f27e2948ad1c640:
>
>   tpm: Make sure the locality received from backend is valid (2018-12-04 10:21:25 -0500)

Hi -- can you provide a justification for why this is rc4 material,
please? This wasn't on the list of "things that maybe or must go
into rc4", and the commit messages and pull request cover letter
don't suggest that there's anything particularly urgent here
that couldn't wait for 4.0.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/3] Merge tpm 2018/12/04 v1

[Peter Maydell]

On Tue, 4 Dec 2018 at 15:43, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 4 Dec 2018 at 15:34, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >
> > This series of patches removes an unnecessary parameter from tpm_tis_abort()
> > and adds a locality range check (using assert()) to tpm_tis_prep_abort() and
> > tpm_tis_request_completed().
> >
> >    Stefan
> >
> > The following changes since commit 83ea23cd207a03c5736be0231acbf7f8b05dbf52:
> >
> >   i386: hvf: Fix overrun of _decode_tbl1 (2018-12-03 15:09:55 +0000)
> >
> > are available in the Git repository at:
> >
> >   git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2018-12-04-1
> >
> > for you to fetch changes up to a639f96111eadb3b8e3021fd3f27e2948ad1c640:
> >
> >   tpm: Make sure the locality received from backend is valid (2018-12-04 10:21:25 -0500)
>
> Hi -- can you provide a justification for why this is rc4 material,
> please? This wasn't on the list of "things that maybe or must go
> into rc4", and the commit messages and pull request cover letter
> don't suggest that there's anything particularly urgent here
> that couldn't wait for 4.0.

I looked through the patchset again, looked back at the history
of it on the mailing list, and got some second opinions on IRC
from Marc-André Lureau and Philippe Mathieu-Daudé. We don't think
this is 3.1 material at this point in the release process. So
I'm not going to apply it -- please resubmit as a pullreq for
4.0 once the tree reopens for development.

thanks
-- PMM