[PATCH 1/2] tpm: Make response length of tpm2_get_capability() configurable

[Heinrich Schuchardt]

On 04.11.20 14:47, Ilias Apalodimas wrote:
> A following patch introduces EFI_TCG2_PROTOCOL.
> One of the functions of that protocol is GetCapability().
> In order to parse device capabilities we need to access a u32
> before the properties which the current implementation ignores
> while reading device properties.
>
> So let's make the response length configurable and prepare the
> functions for EFI_TCG2_PROTOCOL.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>  cmd/tpm-v2.c     |  2 +-
>  include/tpm-v2.h | 12 +++++++-----
>  lib/tpm-v2.c     | 10 +++++++---
>  3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> index e6742656f578..c2df1c34043a 100644
> --- a/cmd/tpm-v2.c
> +++ b/cmd/tpm-v2.c
> @@ -183,7 +183,7 @@ static int do_tpm_get_capability(struct cmd_tbl *cmdtp, int flag, int argc,
>  	data = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
>  	count = simple_strtoul(argv[4], NULL, 0);
>
> -	rc = tpm2_get_capability(dev, capability, property, data, count);
> +	rc = tpm2_get_capability(dev, capability, property, data, count, false);
>  	if (rc)
>  		goto unmap_data;
>
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index f6c045d35480..ee74028ca83b 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -257,15 +257,17 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
>   * to query property index that is 4-byte wide.
>   *
>   * @dev		TPM device
> - * @capability	Partition of capabilities
> - * @property	Further definition of capability, limited to be 4 bytes wide
> - * @buf		Output buffer for capability information
> - * @prop_count	Size of output buffer
> + * @capability		Partition of capabilities
> + * @property		Further definition of capability, limited to be 4 bytes
> + *			wide
> + * @buf			Output buffer for capability information
> + * @prop_count		Size of output buffer
> + * @get_count		Include tpmu property count
>   *
>   * @return code of the operation
>   */
>  u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property,
> -			void *buf, size_t prop_count);
> +			void *buf, size_t prop_count, bool get_count);
>
>  /**
>   * Issue a TPM2_DictionaryAttackLockReset command.
> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index a4c352e3ef75..b58c1057995b 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -161,7 +161,7 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
>  }
>
>  u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property,
> -			void *buf, size_t prop_count)
> +			void *buf, size_t prop_count, bool get_count)

The implementation would be more stable if we would derive the offset
from field property instead of adding get_count.

>  {
>  	u8 command_v2[COMMAND_BUFFER_SIZE] = {

Shouldn't COMMAND_BUFFER_SIZE be changed to something with TPM in the
name, e.g TPM_COMMAND_BUFFER_SIZE?

>  		tpm_u16(TPM2_ST_NO_SESSIONS),		/* TAG */
> @@ -181,13 +181,17 @@ u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property,
>  	if (ret)
>  		return ret;
>
> +	/* When reading PCR properties we need the count */
> +	properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
> +			 sizeof(u8) + sizeof(u32);
>  	/*
>  	 * In the response buffer, the properties are located after the:
>  	 * tag (u16), response size (u32), response code (u32),
>  	 * YES/NO flag (u8), TPM_CAP (u32) and TPMU_CAPABILITIES (u32).
>  	 */

This comment should be above 'properties_off ='. 'get_count' related
field should be mentioned.

Best regards

Heinrich

> -	properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
> -			 sizeof(u8) + sizeof(u32) + sizeof(u32);
> +	if (!get_count)
> +		properties_off += sizeof(u32);
> +
>  	memcpy(buf, &response[properties_off], response_len - properties_off);
>
>  	return 0;
>