[PATCH] efi_loader: add EFI_TCG2_PROTOCOL.SubmitCommand

[Masahisa Kojima]

On Fri, 29 Oct 2021 at 15:26, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
>
> On Fri, Oct 22, 2021 at 08:20:55PM +0900, Masahisa Kojima wrote:
> > This commit adds the EFI_TCG2_PROTOCOL.SubmitCommand
> > required in the TCG PC Client PFP spec.
> > SubmitCommand enables to send the raw command to the TPM device.
> >
> > To implement this api, tpm2_submit_command() is added
> > into tpm-v2.c.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> >  include/tpm-v2.h          | 15 +++++++++++++++
> >  lib/efi_loader/efi_tcg2.c | 37 +++++++++++++++++++++++++++++++------
> >  lib/tpm-v2.c              |  6 ++++++
> >  3 files changed, 52 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > index e6b68769f3..6b04a637ca 100644
> > --- a/include/tpm-v2.h
> > +++ b/include/tpm-v2.h
> > @@ -642,4 +642,19 @@ u32 tpm2_write_lock(struct udevice *dev, u32 index);
> >   */
> >  u32 tpm2_disable_platform_hierarchy(struct udevice *dev);
> >
> > +/**
> > + * submit user specified data to the TPM and get response
> > + *
> > + * @dev              TPM device
> > + * @sendbuf: Buffer of the data to send
> > + * @send_size:       Size of the data to send
> > + * @recvbuf: Buffer to save the response to
> > + * @recv_size:       Pointer to the size of the response buffer
> > + *
> > + * Returns 0 on success (and places the number of response bytes at
> > + * recv_size) or -ve on failure.
> > + */
> > +u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf,
> > +                     size_t send_size, u8 *recvbuf, size_t *recv_size);
> > +
> >  #endif /* __TPM_V2_H */
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index da02e281e7..a0728b555e 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -1033,13 +1033,38 @@ out:
> >   * Return:   status code
> >   */
> >  static efi_status_t EFIAPI
> > -efi_tcg2_submit_command(__maybe_unused struct efi_tcg2_protocol *this,
> > -                     u32 __maybe_unused input_param_block_size,
> > -                     u8 __maybe_unused *input_param_block,
> > -                     u32 __maybe_unused output_param_block_size,
> > -                     u8 __maybe_unused *output_param_block)
> > +efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
> > +                     u32 input_param_block_size,
> > +                     u8 *input_param_block,
> > +                     u32 output_param_block_size,
> > +                     u8 *output_param_block)
> >  {
> > -     return EFI_UNSUPPORTED;
> > +     struct udevice *dev;
> > +     efi_status_t ret;
> > +     u32 rc;
> > +     size_t resp_buf_size = output_param_block_size;
> > +
> > +     EFI_ENTRY("%p, %u, %p, %u, %p", this, input_param_block_size,
> > +               input_param_block, output_param_block_size, output_param_block);
> > +
> > +     if (!this || !input_param_block || !input_param_block_size) {
> > +             ret = EFI_INVALID_PARAMETER;
> > +             goto out;
> > +     }
> > +
> > +     ret = platform_get_tpm2_device(&dev);
> > +     if (ret != EFI_SUCCESS)
> > +             goto out;
> > +
> > +     rc = tpm2_submit_command(dev, input_param_block, input_param_block_size,
> > +                              output_param_block, &resp_buf_size);
>
> Shouldn't you check resp_buf_size against output_param_block_size here and
> return EFI_BUFFER_TOO_SMALL if the output_param_block_size is smaller?

It should be checked, thank you.

>
> > +     if (rc) {
> > +             ret = EFI_DEVICE_ERROR;
> > +             goto out;
> > +     }
> > +
> > +out:
> > +     return EFI_EXIT(ret);
> >  }
> >
> >  /**
> > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> > index 235f8c20d4..ee08948ec1 100644
> > --- a/lib/tpm-v2.c
> > +++ b/lib/tpm-v2.c
> > @@ -659,3 +659,9 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev)
> >
> >       return 0;
> >  }
> > +
> > +u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf,
> > +                     size_t send_size, u8 *recvbuf, size_t *recv_size)
>
> Why do we keep send_size in the arg list? tpm_sendrecv_command is exported
> anyway, so we can just use that?

Yes, send_size is not used, should be removed.

tpm_sendrecv_command() is exported, but declared in lib/tpm-utils.h,
it means this function can only be called under u-boot/lib, can not be
called from ./lib/efi_loader/*.
Also the tpm stack seems to be designed having following layers.
 tpm-v1/2.c -> tpm-common.c -> device-dependent driver

Thanks,
Masahisa Kojima

>
> > +{
> > +     return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size);
> > +}
> > --
> > 2.17.1
> >
>
> Thanks!
> /Ilias