Hi Shahriyar,
On Wed, 29 Jul 2026 at 21:25, Shahriyar Jalayeri <[email protected]> wrote: > > 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 unchecked. The clearest damage is in the parsers that > compute response_length - <header> and index the result (the TPM1 OIAP > helpers and tpm2_get_capability): a reply shorter than the header > underflows the subtraction into a huge memcpy. > > Rather than guard each caller, check the length once at the choke point. > tpm_sendrecv_command() gains a min_response_len argument and rejects a > reply shorter than the caller says it needs, before the caller parses it; > every command that reads a response now declares its minimum. > > tpm2_get_capability() additionally copies response_len - 15 bytes into the > caller's buffer, but was never told how big that buffer is, so a reply > longer than it overruns it. Give the function a buf_size argument and > reject a reply that would not fit, so the copy is bounded for every > capability rather than for the properties query alone. > > 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. > [...] > } > > u32 tpm1_end_oiap(struct udevice *dev) > @@ -695,7 +702,8 @@ u32 tpm1_oiap(struct udevice *dev, u32 *auth_handle) > if (oiap_session.valid) > tpm1_terminate_auth_session(dev, oiap_session.handle); > > - err = tpm_sendrecv_command(dev, command, response, &response_length); > + /* tag(2) + size(4) + rc(4) + authHandle(4) + nonceEven(20) = 34 */ > + err = tpm_sendrecv_command(dev, command, response, &response_length, > 34); > if (err) > return err; > if (unpack_byte_string(response, response_length, "ds", > @@ -750,7 +758,8 @@ u32 tpm1_load_key2_oiap(struct udevice *dev, u32 > parent_handle, const void *key, > parent_key_usage_auth); > if (err) > return err; > - err = tpm_sendrecv_command(dev, request, response, &response_length); > + /* nonceEven(20) + continueAuthSession(1) + resAuth(20) = 41 */ > + err = tpm_sendrecv_command(dev, request, response, &response_length, > 41); The load doesn't seem to factor in the header len which the commit message mentions. The code later on does "response_length - TPM_RESPONSE_AUTH_LENGTH", so it should cause any issues, but we should keep the same policy everywhere. Do you know if loading the key includes the header in the response? [...] Cheers /Ilias
