Re: [PATCH] efi_loader: add EFI_TCG2_PROTOCOL.SubmitCommand
On Fri, 29 Oct 2021 at 18:43, Masahisa Kojima
wrote:
>
> On Fri, 29 Oct 2021 at 15:26, Ilias Apalodimas
> 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
> > > ---
> > > 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.
tpm_sendrecv_command() does not fill *recv_size with the received size
if the *recv_size is smaller than the actual received size, it just
return -ENOSPC.
So instead of checking resp_buf_size, check return code and
if rc is -ENOSPC then return EFI_BUFFER_TOO_SMALL.
Thanks,
Masahisa Kojima
>
> >
> > > + 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 b
Re: [PATCH] efi_loader: add EFI_TCG2_PROTOCOL.SubmitCommand
On Fri, 29 Oct 2021 at 15:26, Ilias Apalodimas
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
> > ---
> > 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
Re: [PATCH] efi_loader: add EFI_TCG2_PROTOCOL.SubmitCommand
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
> ---
> 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?
> + 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?
> +{
> + return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size);
> +}
> --
> 2.17.1
>
Thanks!
/Ilias
[PATCH] efi_loader: add EFI_TCG2_PROTOCOL.SubmitCommand
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
---
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
+ *
+ * @devTPM 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);
+ 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)
+{
+ return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size);
+}
--
2.17.1
