Re: [SeaBIOS] [PATCH v2 1/3] tcgbios: pass returnCode through many functions

[Stefan Berger]

On 01/16/2018 01:16 PM, Kevin O'Connor wrote:
> On Tue, Jan 16, 2018 at 11:41:01AM -0500, Stefan Berger wrote:
> > Pass the returnCode parameter through many of the functions so
> > that we can get the TPM return code from hwtpm_transmit, if needed.
> > Signed-off-by: Stefan Berger
> >
> > src/hw/tpm_drivers.c |   8 ++-
> >   src/hw/tpm_drivers.h |   2 +-
> >   src/tcgbios.c        | 147 +++++++++++++++++++++++++++------------------------
> >   3 files changed, 86 insertions(+), 71 deletions(-)
> >
> > diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
> > index a137e62..25f4f36 100644
> > --- a/src/hw/tpm_drivers.c
> > +++ b/src/hw/tpm_drivers.c
> > @@ -377,7 +377,7 @@ tpmhw_is_present(void)
> >   int
> >   tpmhw_transmit(u8 locty, struct tpm_req_header *req,
> >                  void *respbuffer, u32 *respbufferlen,
> >           enum tpmDurationType to_t)
> >           enum tpmDurationType to_t, u32 *returnCode)
> > {
> >     if (TPMHW_driver_to_use == TPM_INVALID_DRIVER)
> >         return -1;
> >
> > @@ -408,6 +408,12 @@ tpmhw_transmit(u8 locty, struct tpm_req_header *req,
> >   td->ready();
> >
> > if (returnCode && *respbufferlen >= sizeof(struct tpm_rsp_header)) {
> >    struct tpm_rsp_header *r = respbuffer;
> >    *returnCode = be32_to_cpu(r->errcode);
> > }
> > return 0;
> > }
>
> [...]
> > diff --git a/src/tcgbios.c b/src/tcgbios.c
> > index 40b3028..730b5e7 100644
> > --- a/src/tcgbios.c
> > +++ b/src/tcgbios.c
> > @@ -350,7 +350,8 @@ tpm_build_digest(struct tpm_log_entry *le, const u8 *sha1, int bigEndian)
> >   // optional parameter (0, 1, or 2 bytes) and have no special response.
> >   static int
> >   tpm_simple_cmd(u8 locty, u32 ordinal
> >           , int param_size, u16 param, enum tpmDurationType to_t)
> >           , int param_size, u16 param, enum tpmDurationType to_t
> >           , u32 *returnCode)
> > {
> >     struct {
> >         struct tpm_req_header trqh;
> >
> > @@ -374,7 +375,8 @@ tpm_simple_cmd(u8 locty, u32 ordinal
> >       u32 obuffer_len = sizeof(obuffer);
> >       memset(obuffer, 0x0, sizeof(obuffer));
> > int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t);
> > int ret = tpmhw_transmit(locty, &req.trqh, obuffer, &obuffer_len, to_t,
> >                         returnCode);
> > ret = ret ? -1 : be32_to_cpu(trsh->errcode);
>
> I'm not sure why this patch is necessary.  The callers of
> tpmhw_transmit() can already extract the errcode.  Indeed,
> tpm_simple_cmd() does just that and returns it to its callers via its
> return code (see the last line of code above).

We sometimes have code like this:

ret = tpm12...
if (ret)
     return -1;

This just maps a potential TPM error code to -1.

I guess the right thing to do is to avoid this mapping.

Stefan
> -Kevin