* [PATCH 1/2] tpm: bounds-check the device-controlled response length
2026-07-23 14:01 [PATCH 0/2] tpm: bounds-check device-reported response lengths shj
@ 2026-07-23 14:01 ` shj
2026-07-23 14:01 ` [PATCH 2/2] test: tpm: check malformed capability responses are rejected shj
2026-07-24 9:47 ` [PATCH 0/2] tpm: bounds-check device-reported response lengths Miquel Raynal via U-Boot
2 siblings, 0 replies; 5+ messages in thread
From: shj @ 2026-07-23 14:01 UTC (permalink / raw)
To: u-boot, Ilias Apalodimas, Simon Glass, u-boot, Andy Fleming
Cc: Miquel Raynal, Reinhard Pfau, Che-Liang Chiou, Dirk Eibach,
Tom Rini, Argus, shj
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.
Check the reported length against the header before the subtraction, and
against the caller's buffer before the GetCapability copy.
Fixes: 69cd8f0681f4 ("tpm: add TPM2_GetCapability command support")
Fixes: be6c1529c1ce ("tpm: add AUTH1 cmds for LoadKey2 and GetPubKey")
Signed-off-by: shj <shahriyar@byteray.co.uk>
---
lib/tpm-v1.c | 6 ++++++
lib/tpm-v2.c | 8 ++++++++
2 files changed, 14 insertions(+)
diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
index a6727c575fd..5fd22924b28 100644
--- a/lib/tpm-v1.c
+++ b/lib/tpm-v1.c
@@ -757,6 +757,9 @@ u32 tpm1_load_key2_oiap(struct udevice *dev, u32 parent_handle, const void *key,
return err;
}
+ if (response_length < TPM_RESPONSE_AUTH_LENGTH)
+ return TPM_LIB_ERROR;
+
err = verify_response_auth(0x00000041, response,
response_length - TPM_RESPONSE_AUTH_LENGTH,
4, &oiap_session,
@@ -817,6 +820,9 @@ u32 tpm1_get_pub_key_oiap(struct udevice *dev, u32 key_handle,
oiap_session.valid = 0;
return err;
}
+ if (response_length < TPM_RESPONSE_AUTH_LENGTH)
+ return TPM_LIB_ERROR;
+
err = verify_response_auth(0x00000021, response,
response_length - TPM_RESPONSE_AUTH_LENGTH,
0, &oiap_session,
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index f443b738f82..aa4d3866d7b 100644
--- 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,
*/
properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
sizeof(u8) + sizeof(u32);
+ if (response_len < properties_off)
+ return TPM_LIB_ERROR;
+
+ 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);
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] test: tpm: check malformed capability responses are rejected
2026-07-23 14:01 [PATCH 0/2] tpm: bounds-check device-reported response lengths shj
2026-07-23 14:01 ` [PATCH 1/2] tpm: bounds-check the device-controlled response length shj
@ 2026-07-23 14:01 ` shj
2026-07-24 9:47 ` [PATCH 0/2] tpm: bounds-check device-reported response lengths Miquel Raynal via U-Boot
2 siblings, 0 replies; 5+ messages in thread
From: shj @ 2026-07-23 14:01 UTC (permalink / raw)
To: u-boot, Ilias Apalodimas, Simon Glass, u-boot, Andy Fleming
Cc: Miquel Raynal, Reinhard Pfau, Che-Liang Chiou, Dirk Eibach,
Tom Rini, Argus, shj
The length of a TPM reply comes from the device and cannot be trusted to
be well-formed. Add tests for two malformed replies: one too short to hold
the data it advertises, and one advertising more data than the request can
hold. The sandbox TPM emulator gains two test-only properties that produce
these replies, and two DM tests ask tpm2_get_capability() for them and
check the call is rejected instead of parsing past the end of the reply or
past the caller's buffer.
Signed-off-by: shj <shahriyar@byteray.co.uk>
---
drivers/tpm/tpm2_tis_sandbox.c | 27 +++++++++++++++++++++++++++
include/tpm-v2.h | 10 ++++++++++
test/dm/tpm.c | 41 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 78 insertions(+)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c
index 50e308e7116..9205781fafc 100644
--- a/drivers/tpm/tpm2_tis_sandbox.c
+++ b/drivers/tpm/tpm2_tis_sandbox.c
@@ -542,6 +542,33 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const u8 *sendbuf,
property_count = get_unaligned_be32(sent);
sent += sizeof(property_count);
+ /*
+ * Test hook: reply with a truncated (header-only) success
+ * response so the response parser can be exercised against a
+ * reply too short to hold the data it promises.
+ */
+ if (capability == TPM2_CAP_TPM_PROPERTIES &&
+ property == TPM2_PT_SANDBOX_SHORT_RESPONSE)
+ return sandbox_tpm2_fill_buf(recv, recv_len, tag,
+ TPM2_RC_SUCCESS);
+
+ /*
+ * Test hook: reply with a success response advertising more
+ * property data than a single-property request can hold, to
+ * exercise the caller-buffer bound in the parser.
+ */
+ if (capability == TPM2_CAP_TPM_PROPERTIES &&
+ property == TPM2_PT_SANDBOX_LONG_RESPONSE) {
+ *recv_len = TPM2_HDR_LEN + sizeof(u8) + sizeof(u32) +
+ sizeof(u32) + TPM2_PROPERTY_NB *
+ sizeof(struct tpms_tagged_property);
+ put_unaligned_be16(tag, recv);
+ put_unaligned_be32(*recv_len, recv + sizeof(tag));
+ put_unaligned_be32(TPM2_RC_SUCCESS,
+ recv + sizeof(tag) + sizeof(u32));
+ return 0;
+ }
+
switch (capability) {
case TPM2_CAP_PCRS:
break;
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index a776d24d71f..8bcc4caeb06 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -43,6 +43,16 @@ struct udevice;
#define TPM2_CAP_PCRS 0x00000005U
#define TPM2_CAP_TPM_PROPERTIES 0x00000006U
+/*
+ * Sandbox emulator test hooks: a TPM2_GetCapability for these properties makes
+ * the emulated TPM reply with, respectively, a header-only response and one
+ * advertising more property data than a single-property request can hold. They
+ * let tests drive the response parser with a reply shorter or longer than the
+ * data it should carry, as a tampered or faulty TPM on the bus could.
+ */
+#define TPM2_PT_SANDBOX_SHORT_RESPONSE 0x00ffffff
+#define TPM2_PT_SANDBOX_LONG_RESPONSE 0x00fffffe
+
/* Definition of (UINT32) TPM2_PT Constants */
#define TPM2_PT_GROUP (u32)(0x00000100)
#define TPM2_PT_FIXED (u32)(TPM2_PT_GROUP * 1)
diff --git a/test/dm/tpm.c b/test/dm/tpm.c
index 87c5c416daa..a8645cd1601 100644
--- a/test/dm/tpm.c
+++ b/test/dm/tpm.c
@@ -6,6 +6,7 @@
#include <dm.h>
#include <tpm_api.h>
+#include <tpm-v2.h>
#include <dm/test.h>
#include <test/test.h>
#include <test/ut.h>
@@ -197,3 +198,43 @@ static int dm_test_tpm_autostart_reinit(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_tpm_autostart_reinit, UTF_SCAN_FDT);
+
+/*
+ * A TPM sits on a bus a physical attacker can reach, so its responses cannot be
+ * trusted to be well-formed. Check that a GetCapability reply too short to hold
+ * the data it advertises is rejected, rather than parsed with an underflowed
+ * length.
+ */
+static int dm_test_tpm2_get_capability_short(struct unit_test_state *uts)
+{
+ struct udevice *dev;
+ u8 buf[64];
+
+ ut_assertok(get_tpm_version(TPM_V2, &dev));
+ ut_assertok(tpm_auto_start(dev));
+
+ ut_assert(tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
+ TPM2_PT_SANDBOX_SHORT_RESPONSE, buf, 1));
+
+ return 0;
+}
+DM_TEST(dm_test_tpm2_get_capability_short, UTF_SCAN_FDT);
+
+/*
+ * Check that a GetCapability reply advertising more data than was requested is
+ * rejected, rather than copied past the end of the caller's buffer.
+ */
+static int dm_test_tpm2_get_capability_long(struct unit_test_state *uts)
+{
+ struct udevice *dev;
+ u8 buf[64];
+
+ ut_assertok(get_tpm_version(TPM_V2, &dev));
+ ut_assertok(tpm_auto_start(dev));
+
+ ut_assert(tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES,
+ TPM2_PT_SANDBOX_LONG_RESPONSE, buf, 1));
+
+ return 0;
+}
+DM_TEST(dm_test_tpm2_get_capability_long, UTF_SCAN_FDT);
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] tpm: bounds-check device-reported response lengths
2026-07-23 14:01 [PATCH 0/2] tpm: bounds-check device-reported response lengths shj
2026-07-23 14:01 ` [PATCH 1/2] tpm: bounds-check the device-controlled response length shj
2026-07-23 14:01 ` [PATCH 2/2] test: tpm: check malformed capability responses are rejected shj
@ 2026-07-24 9:47 ` Miquel Raynal via U-Boot
2026-07-24 10:41 ` shj
2 siblings, 1 reply; 5+ messages in thread
From: Miquel Raynal via U-Boot @ 2026-07-24 9:47 UTC (permalink / raw)
To: shj
Cc: u-boot, Ilias Apalodimas, Simon Glass, u-boot, Andy Fleming,
Reinhard Pfau, Che-Liang Chiou, Dirk Eibach, Tom Rini, Argus
Hello,
On 23/07/2026 at 16:01:27 +02, shj <shahriyar@byteray.co.uk> wrote:
> A TPM reply's length is taken from the device and only upper-capped, then
> several callers use it unchecked: the TPM1 OIAP helpers and the TPM2
> GetCapability parser subtract a fixed header length from it, so a reply too
> short underflows the subtraction into a huge memcpy; the GetCapability
> parser then copies that many bytes into the caller's buffer, so a reply
> longer than the buffer overruns it. Because a TPM sits on a physically
> accessible bus, a cheap bus interposer (the kind used to sniff
> disk-encryption keys) can inject such a reply, and on the TPM2 path this
> parsing runs during measured boot, which is exactly the physical attacker
> that measured boot is meant to resist.
>
> Patch 1 bounds the reported length against the header and, for a properties
> query, the caller's buffer. Patch 2 adds regression tests driving the TPM2
> parser with a truncated and an over-long reply through the sandbox emulator.
>
> Based on v2026.07 (fdfe2ec48d5c). A reproducer is available on request.
>
> Signed-off-by: shj <shahriyar@byteray.co.uk>
Thanks for the contribution. Unfortunately, this is not a proper SoB
line, you must put your real identity there.
With this fixed, I am fine with the approach,
Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] tpm: bounds-check device-reported response lengths
2026-07-24 9:47 ` [PATCH 0/2] tpm: bounds-check device-reported response lengths Miquel Raynal via U-Boot
@ 2026-07-24 10:41 ` shj
0 siblings, 0 replies; 5+ messages in thread
From: shj @ 2026-07-24 10:41 UTC (permalink / raw)
To: Miquel Raynal
Cc: u-boot, Ilias Apalodimas, Simon Glass, u-boot, Andy Fleming,
Reinhard Pfau, Che-Liang Chiou, Dirk Eibach, Tom Rini, Argus
On 24.07.26 11:47, Miquel Raynal wrote:
> Hello,
>
> On 23/07/2026 at 16:01:27 +02, shj <shahriyar@byteray.co.uk> wrote:
>
>> A TPM reply's length is taken from the device and only upper-capped, then
>> several callers use it unchecked: the TPM1 OIAP helpers and the TPM2
>> GetCapability parser subtract a fixed header length from it, so a reply too
>> short underflows the subtraction into a huge memcpy; the GetCapability
>> parser then copies that many bytes into the caller's buffer, so a reply
>> longer than the buffer overruns it. Because a TPM sits on a physically
>> accessible bus, a cheap bus interposer (the kind used to sniff
>> disk-encryption keys) can inject such a reply, and on the TPM2 path this
>> parsing runs during measured boot, which is exactly the physical attacker
>> that measured boot is meant to resist.
>>
>> Patch 1 bounds the reported length against the header and, for a properties
>> query, the caller's buffer. Patch 2 adds regression tests driving the TPM2
>> parser with a truncated and an over-long reply through the sandbox emulator.
>>
>> Based on v2026.07 (fdfe2ec48d5c). A reproducer is available on request.
>>
>> Signed-off-by: shj <shahriyar@byteray.co.uk>
> Thanks for the contribution. Unfortunately, this is not a proper SoB
> line, you must put your real identity there.
>
> With this fixed, I am fine with the approach,
>
> Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> Thanks,
> Miquèl
Thanks Miquèl, fixed the Signed-off-by in v2.
Regards,
Shah
^ permalink raw reply [flat|nested] 5+ messages in thread