[PATCH] tpm: eventlog: check the payload size before reading tag events

[PATCH] tpm: eventlog: check the payload size before reading tag events

[Guanglei Zhu]

The EVENT_TAG branch of get_event_name() casts the event payload to
struct tcpa_pc_event and reads from it:

	pc_event = (struct tcpa_pc_event *)event_entry;

	switch (do_endian_conversion(pc_event->event_id)) {
	...
		for (i = 0; i < 20; i++)
			d_len += sprintf(&data[2*i], "%02x",
					pc_event->event_data[i]);

The log iterators only verify that the event header plus the declared
event_size stays within the log buffer; they do not require the
payload of an EVENT_TAG entry to be large enough for the fields the
branch reads. A truncated entry makes the branch read up to 20 bytes
of event_data[] past the end of the entry, and, for the last entry in
the log, past the end of the buffer itself.

Skip entries whose payload cannot even hold the event id, and skip
the 20-byte digest dump when the payload cannot hold the full
tcpa_pc_event header plus the digest, so the branch never reads
beyond the entry it is formatting.

Fixes: 55a82ab3181b ("[PATCH] tpm: add bios measurement log")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/char/tpm/eventlog/tpm1.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c
index 0397e3361020..f93a358a706e 100644
--- a/drivers/char/tpm/eventlog/tpm1.c
+++ b/drivers/char/tpm/eventlog/tpm1.c
@@ -178,6 +178,10 @@ static int get_event_name(char *dest, struct tcpa_event *event,
 		}
 		break;
 	case EVENT_TAG:
+		if (do_endian_conversion(event->event_size) <
+		    sizeof(pc_event->event_id))
+			break;
+
 		pc_event = (struct tcpa_pc_event *)event_entry;
 
 		/* ToDo Row data -> Base64 */
@@ -203,6 +207,9 @@ static int get_event_name(char *dest, struct tcpa_event *event,
 			name = tcpa_pc_event_id_strings[do_endian_conversion
 							(pc_event->event_id)];
 			n_len = strlen(name);
+			if (do_endian_conversion(event->event_size) <
+			    sizeof(struct tcpa_pc_event) + 20)
+				break;
 			for (i = 0; i < 20; i++)
 				d_len += sprintf(&data[2*i], "%02x",
 						pc_event->event_data[i]);
-- 
2.50.1

Re: [PATCH] tpm: eventlog: check the payload size before reading tag events

[Jarkko Sakkinen]

On Tue, Nov 03, 2026 at 04:41:55PM +0800, Guanglei Zhu wrote:
> The EVENT_TAG branch of get_event_name() casts the event payload to
> struct tcpa_pc_event and reads from it:
> 
> 	pc_event = (struct tcpa_pc_event *)event_entry;
> 
> 	switch (do_endian_conversion(pc_event->event_id)) {
> 	...
> 		for (i = 0; i < 20; i++)
> 			d_len += sprintf(&data[2*i], "%02x",
> 					pc_event->event_data[i]);
> 
> The log iterators only verify that the event header plus the declared
> event_size stays within the log buffer; they do not require the
> payload of an EVENT_TAG entry to be large enough for the fields the
> branch reads. A truncated entry makes the branch read up to 20 bytes
> of event_data[] past the end of the entry, and, for the last entry in
> the log, past the end of the buffer itself.


What does the spec say about this?

> 
> Skip entries whose payload cannot even hold the event id, and skip
> the 20-byte digest dump when the payload cannot hold the full
> tcpa_pc_event header plus the digest, so the branch never reads
> beyond the entry it is formatting.
> 
> Fixes: 55a82ab3181b ("[PATCH] tpm: add bios measurement log")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
> ---
>  drivers/char/tpm/eventlog/tpm1.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c
> index 0397e3361020..f93a358a706e 100644
> --- a/drivers/char/tpm/eventlog/tpm1.c
> +++ b/drivers/char/tpm/eventlog/tpm1.c
> @@ -178,6 +178,10 @@ static int get_event_name(char *dest, struct tcpa_event *event,
>  		}
>  		break;
>  	case EVENT_TAG:
> +		if (do_endian_conversion(event->event_size) <
> +		    sizeof(pc_event->event_id))
> +			break;
> +
>  		pc_event = (struct tcpa_pc_event *)event_entry;
>  
>  		/* ToDo Row data -> Base64 */
> @@ -203,6 +207,9 @@ static int get_event_name(char *dest, struct tcpa_event *event,
>  			name = tcpa_pc_event_id_strings[do_endian_conversion
>  							(pc_event->event_id)];
>  			n_len = strlen(name);
> +			if (do_endian_conversion(event->event_size) <
> +			    sizeof(struct tcpa_pc_event) + 20)
> +				break;
>  			for (i = 0; i < 20; i++)
>  				d_len += sprintf(&data[2*i], "%02x",
>  						pc_event->event_data[i]);
> -- 
> 2.50.1
> 

Br, Jarkko

Re: [PATCH] tpm: eventlog: check the payload size before reading tag events

[Guanglei Zhu]

The legacy PC Client spec defines the EV_EVENT_TAG payload as a
TCPA_PC_EVENT: event_id, event_size and event-type-specific data.
A conformant entry carries the 8-byte header, and the hash-type
ids carry a full digest.

The catch is that the log is an ACPI table the firmware leaves in
memory: the TPM never measures the log itself, so the kernel cannot
tell a conformant log from a corrupted one.  The iterators already
work on that assumption - they stop once the declared event_size
runs past the buffer end - and this check applies the same rule to
what the EVENT_TAG branch reads off the payload.

Re: [PATCH] tpm: eventlog: check the payload size before reading tag events

[Jarkko Sakkinen]

On Wed, Nov 04, 2026 at 10:58:09AM +0800, Guanglei Zhu wrote:
> The legacy PC Client spec defines the EV_EVENT_TAG payload as a
> TCPA_PC_EVENT: event_id, event_size and event-type-specific data.
> A conformant entry carries the 8-byte header, and the hash-type
> ids carry a full digest.
> 
> The catch is that the log is an ACPI table the firmware leaves in
> memory: the TPM never measures the log itself, so the kernel cannot
> tell a conformant log from a corrupted one.  The iterators already
> work on that assumption - they stop once the declared event_size
> runs past the buffer end - and this check applies the same rule to
> what the EVENT_TAG branch reads off the payload.

Yeah, this is exactly what I wanted to know. Could you refine the
patch and send a new version with this?

It's really good stuff to have in the commit log when looking back
at old changes.


Br, Jarkko

[PATCH v2] tpm: eventlog: check the payload size before reading tag events

[Guanglei Zhu]

The EVENT_TAG branch of get_event_name() casts the event payload to
struct tcpa_pc_event and reads from it:

	pc_event = (struct tcpa_pc_event *)event_entry;

	switch (do_endian_conversion(pc_event->event_id)) {
	...
		for (i = 0; i < 20; i++)
			d_len += sprintf(&data[2*i], "%02x",
					pc_event->event_data[i]);

The log iterators only verify that the event header plus the declared
event_size stays within the log buffer; they do not require the
payload of an EVENT_TAG entry to be large enough for the fields the
branch reads. A truncated entry makes the branch read up to 20 bytes
of event_data[] past the end of the entry, and, for the last entry in
the log, past the end of the buffer itself.

The legacy PC Client spec defines the EV_EVENT_TAG payload as a
TCPA_PC_EVENT - event_id, event_size and event-type-specific data -
so a conformant entry carries the 8-byte header and the hash-type
ids carry a full digest. The log itself, however, is an ACPI table
the firmware leaves in memory: the TPM never measures the log, so
the kernel cannot tell a conformant log from a corrupted one. The
iterators already work on that assumption - they stop once the
declared event_size runs past the buffer end - and the readers
should apply the same rule to what they take off the payload.

Skip entries whose payload cannot even hold the event id, and skip
the 20-byte digest dump when the payload cannot hold the full
tcpa_pc_event header plus the digest, so the branch never reads
beyond the entry it is formatting.

Fixes: 55a82ab3181b ("[PATCH] tpm: add bios measurement log")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
v2: fold the spec rationale from the list discussion into the commit
    log, per review - what the legacy PC Client spec defines for the
    tag payload, why the kernel cannot trust the log to conform, and
    how the iterators already handle it.

 drivers/char/tpm/eventlog/tpm1.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c
index 0397e33..f93a358 100644
--- a/drivers/char/tpm/eventlog/tpm1.c
+++ b/drivers/char/tpm/eventlog/tpm1.c
@@ -178,6 +178,10 @@ static int get_event_name(char *dest, struct tcpa_event *event,
 		}
 		break;
 	case EVENT_TAG:
+		if (do_endian_conversion(event->event_size) <
+		    sizeof(pc_event->event_id))
+			break;
+
 		pc_event = (struct tcpa_pc_event *)event_entry;
 
 		/* ToDo Row data -> Base64 */
@@ -203,6 +207,9 @@ static int get_event_name(char *dest, struct tcpa_event *event,
 			name = tcpa_pc_event_id_strings[do_endian_conversion
 							(pc_event->event_id)];
 			n_len = strlen(name);
+			if (do_endian_conversion(event->event_size) <
+			    sizeof(struct tcpa_pc_event) + 20)
+				break;
 			for (i = 0; i < 20; i++)
 				d_len += sprintf(&data[2*i], "%02x",
 						pc_event->event_data[i]);
-- 
2.43.0