Hello Shahriyar,
> + void *buf, size_t buf_size, size_t prop_count);
>
> /**
> * tpm2_get_pcr_info() - get the supported, active PCRs and number of banks
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 5f9bbe97455..e15a2340e83 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -154,7 +154,8 @@ static int tpm2_get_max_command_size(struct udevice *dev,
> u16 *max_command_size)
>
> memset(response, 0, sizeof(response));
> ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> - TPM2_PT_MAX_COMMAND_SIZE, response, 1);
> + TPM2_PT_MAX_COMMAND_SIZE, response,
> + sizeof(response), 1);
> if (ret)
> return -1;
>
> @@ -180,7 +181,8 @@ static int tpm2_get_max_response_size(struct udevice *dev,
>
> memset(response, 0, sizeof(response));
> ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> - TPM2_PT_MAX_RESPONSE_SIZE, response, 1);
> + TPM2_PT_MAX_RESPONSE_SIZE, response,
> + sizeof(response), 1);
> if (ret)
> return -1;
>
> @@ -205,7 +207,8 @@ static int tpm2_get_manufacturer_id(struct udevice *dev,
> u32 *manufacturer_id)
>
> memset(response, 0, sizeof(response));
> ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
> - TPM2_PT_MANUFACTURER, response, 1);
> + TPM2_PT_MANUFACTURER, response,
> + sizeof(response), 1);
> if (ret)
> return -1;
>
> diff --git a/lib/tpm-common.c b/lib/tpm-common.c
> index b592c22bfc1..e4a620990fa 100644
> --- a/lib/tpm-common.c
> +++ b/lib/tpm-common.c
> @@ -159,7 +159,8 @@ u32 tpm_return_code(const void *response)
> }
>
> u32 tpm_sendrecv_command(struct udevice *dev, const void *command,
> - void *response, size_t *size_ptr)
> + void *response, size_t *size_ptr,
> + size_t min_response_len)
> {
> int err, ret;
> u8 response_buffer[COMMAND_BUFFER_SIZE];
> @@ -190,11 +191,20 @@ u32 tpm_sendrecv_command(struct udevice *dev, const
> void *command,
> if (err < 0)
> return err;
>
> + ret = tpm_return_code(response);
> +
> + /*
> + * The response length is reported by the (untrusted) TPM. Reject a
> + * successful reply too short to hold what the caller will parse, so it
> + * cannot read past the end of a truncated response. A TPM error reply
> + * is a bare header with no body; let its return code reach the caller.
> + */
> + if (!ret && size_ptr && response_length < min_response_len)
> + return TPM_LIB_ERROR;
The documentation you added says that min_response_len of 0 is ignored. But
here we seem to rely
only on size_ptr != NULL.
> - ret = tpm_return_code(response);
> -
> log_debug("TPM response [ret:%d]: ", ret);
> for (i = 0; i < response_length; i++)
> log_debug("%02x ", ((u8 *)response)[i]);
[...]
> @@ -519,6 +522,11 @@ u32 tpm2_get_capability(struct udevice *dev, u32
> capability, u32 property,
> */
> properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
> sizeof(u8) + sizeof(u32);
> +
> + /* the reply length is set by the TPM; do not overrun the caller's buf
> */
> + if (response_len - properties_off > buf_size)
> + return TPM_LIB_ERROR;
> +
Isn't that covered by tpm_sendrecv_command() now?
> memcpy(buf, &response[properties_off], response_len - properties_off);
>
> return 0;
[...]
Thanks
/Ilias
