Hi Shahriyar,
On 2026-07-24T10:34:14, shj <[email protected]> wrote:
> tpm: bounds-check the device-controlled response length
>
> The length of a TPM reply is set by the device: tpm_sendrecv_command()
> stores the number of bytes received, caps it only at the command buffer
> size, and reports success whenever the reply's return code is 0. Callers
> then use that length without checking it:
>
> tpm2_get_capability() copies response_len - 15 bytes into the caller's
> buffer. A reply shorter than 15 bytes underflows the subtraction into a
> huge memcpy; a reply longer than the buffer (sized for prop_count
> properties) overruns it.
>
> tpm1_load_key2_oiap() and tpm1_get_pub_key_oiap() pass response_length
> - 41 to verify_response_auth(), which a reply shorter than 41 bytes
> underflows.
>
> A TPM sits on a discrete SPI/I2C/LPC bus that is physically accessible, so
> a reply this malformed is reachable by a bus interposer or a faulty part;
> on the TPM2 path this parsing runs during measured boot.
>
> [...]
>
> lib/tpm-v1.c | 6 ++++++
> lib/tpm-v2.c | 8 ++++++++
> 2 files changed, 14 insertions(+)
> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> @@ -519,6 +519,14 @@ u32 tpm2_get_capability(struct udevice *dev, u32
> capability, u32 property,
> + if (capability == TPM2_CAP_TPM_PROPERTIES &&
> + response_len - properties_off >
> + sizeof(u32) + prop_count * sizeof(struct tpms_tagged_property))
> + return TPM_LIB_ERROR;
> +
> memcpy(buf, &response[properties_off], response_len - properties_off);
This bounds the copy against a size derived from prop_count, not against
the real size of buf, which the function is never told. It is safe today
only because every caller happens to pass a buffer of at least
sizeof(u32) + prop_count * sizeof(struct tpms_tagged_property) bytes. The
kerneldoc does not spell that out - it describes @prop_count only as
'Size of output buffer', which reads as a byte count rather than a
property count:
@prop_count Size of output buffer
So the memcpy() safety rests on an undocumented contract. Since you are
hardening this path, please can you pass the caller's actual buffer
length and bound the copy against that, and document what @prop_count
means? That also sidesteps what the bound should be for capabilities
other than TPM2_CAP_TPM_PROPERTIES, where the copy is still unbounded.
What do you think?
Regards,
Simon
