diff mbox series

efi_loader: add EFI_TCG2_PROTOCOL.SubmitCommand

Message ID 20211022112056.24818-1-masahisa.kojima@linaro.org
State Superseded
Headers show
Series efi_loader: add EFI_TCG2_PROTOCOL.SubmitCommand | expand

Commit Message

Masahisa Kojima Oct. 22, 2021, 11:20 a.m. UTC 
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@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(-)

-- 
2.17.1

Comments

Ilias Apalodimas Oct. 29, 2021, 6:26 a.m. UTC | #1 
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@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?

> +	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
Masahisa Kojima Oct. 29, 2021, 9:43 a.m. UTC | #2 
On Fri, 29 Oct 2021 at 15:26, Ilias Apalodimas
<ilias.apalodimas@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@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
Masahisa Kojima Nov. 2, 2021, 5:36 a.m. UTC | #3 
On Fri, 29 Oct 2021 at 18:43, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>

> On Fri, 29 Oct 2021 at 15:26, Ilias Apalodimas

> <ilias.apalodimas@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@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.


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 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
diff mbox series

Patch

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);
+	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);
+}