[PATCH 0/1] Fix crash in __calc_tpm2_event_size

[PATCH 0/1] Fix crash in __calc_tpm2_event_size

[Chris Coulson]

I've been testing the latest code in the linux-tpmdd branch and I'm
experiencing a crash in __calc_tpm2_event_size when it's called to
calculate the size of events in the final log. I hope I'm not stepping on
anyone's toes, but this small change seems to fix it.

What seems to happen is that the event header is mapped here:

    /* Map the event header */
    if (do_mapping) {
        mapping_size = marker - marker_start;
        mapping = TPM_MEMREMAP((unsigned long)marker_start,
                       mapping_size);

    ...

    event = (struct tcg_pcr_event2_head *)mapping;

When calculating the cumulative size of the digests, the event header is
dereferenced here on each loop iteration in order to obtain the digest
count:

    for (i = 0; i < event->count; i++) {
        halg_size = sizeof(event->digests[i].alg_id);

But the first iteration of the loop unmaps the event header:

        /* Map the digest's algorithm identifier */
        if (do_mapping) {
            TPM_MEMUNMAP(mapping, mapping_size);
            mapping_size = halg_size;
            mapping = TPM_MEMREMAP((unsigned long)marker,
                         mapping_size);

Subsequent loop iterations then dereference a pointer to unmapped memory.

Chris Coulson (1):
  tpm: Don't dereference event after it's unmapped in
    __calc_tpm2_event_size

 include/linux/tpm_eventlog.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/1] tpm: Don't dereference event after it's unmapped in __calc_tpm2_event_size

[Chris Coulson]

The pointer to the event header is dereferenced on each loop iteration in
order to obtain the digest count, but when called from
tpm2_calc_event_log_size, the event header is unmapped on the first
iteration of the loop. This results in an invalid access for on subsequent
loop iterations for log entries that have more than one digest.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
---
 include/linux/tpm_eventlog.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 63238c84dc0b..7b76abbff7d8 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -165,6 +165,7 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	int mapping_size;
 	void *marker;
 	void *marker_start;
+	u32 count;
 	u32 halg_size;
 	size_t size;
 	u16 halg;
@@ -190,16 +191,17 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	}
 
 	event = (struct tcg_pcr_event2_head *)mapping;
+	count = event->count;
 
 	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
 
 	/* Check if event is malformed. */
-	if (event->count > efispecid->num_algs) {
+	if (count > efispecid->num_algs) {
 		size = 0;
 		goto out;
 	}
 
-	for (i = 0; i < event->count; i++) {
+	for (i = 0; i < count; i++) {
 		halg_size = sizeof(event->digests[i].alg_id);
 
 		/* Map the digest's algorithm identifier */
-- 
2.17.1

Re: [PATCH 0/1] Fix crash in __calc_tpm2_event_size

[Jarkko Sakkinen]

On Wed, Jun 05, 2019 at 12:04:32AM +0100, Chris Coulson wrote:
> I've been testing the latest code in the linux-tpmdd branch and I'm
> experiencing a crash in __calc_tpm2_event_size when it's called to
> calculate the size of events in the final log. I hope I'm not stepping on
> anyone's toes, but this small change seems to fix it.

Oh no, thank you for reporting this.

/Jarkko

Re: [PATCH 1/1] tpm: Don't dereference event after it's unmapped in __calc_tpm2_event_size

[Jarkko Sakkinen]

On Wed, Jun 05, 2019 at 12:04:33AM +0100, Chris Coulson wrote:
> The pointer to the event header is dereferenced on each loop iteration in
> order to obtain the digest count, but when called from
> tpm2_calc_event_log_size, the event header is unmapped on the first
> iteration of the loop. This results in an invalid access for on subsequent
> loop iterations for log entries that have more than one digest.
> 
> Signed-off-by: Chris Coulson <chris.coulson@canonical.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko