linux-kernel - [PATCH 3.16 066/129] tpm: Fix off-by-one when reading binary_bios

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 07 Jul 2019 17:54:17 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
        "Jarkko Sakkinen" <jarkko.sakkinen@...ux.intel.com>,
        "Jia Zhang" <zhang.jia@...ux.alibaba.com>
Subject: [PATCH 3.16 066/129] tpm: Fix off-by-one when reading
 binary_bios_measurements

3.16.70-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Jia Zhang <zhang.jia@...ux.alibaba.com>

commit 64494d39ff630a63b5308042b20132b491e3706b upstream.

It is unable to read the entry when it is the only one in
binary_bios_measurements:

00000000  00 00 00 00 08 00 00 00  c4 2f ed ad 26 82 00 cb
00000010  1d 15 f9 78 41 c3 44 e7  9d ae 33 20 00 00 00 00
00000020

This is obviously a firmware problem on my linux machine:

	Manufacturer: Inspur
	Product Name: SA5212M4
	Version: 01

However, binary_bios_measurements should return it any way,
rather than nothing, after all its content is completely
valid.

Fixes: 55a82ab3181b ("tpm: add bios measurement log")
Signed-off-by: Jia Zhang <zhang.jia@...ux.alibaba.com>
Reviewd-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
[bwh: Backported to 3.16:
 - Fix an additional comparison in tpm1_bios_measurements_start()
 - Adjust filename, context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -81,7 +81,7 @@ static void *tpm_bios_measurements_start
 	for (i = 0; i < *pos; i++) {
 		event = addr;
 
-		if ((addr + sizeof(struct tcpa_event)) < limit) {
+		if ((addr + sizeof(struct tcpa_event)) <= limit) {
 			if (event->event_type == 0 && event->event_size == 0)
 				return NULL;
 			addr += sizeof(struct tcpa_event) + event->event_size;
@@ -89,13 +89,13 @@ static void *tpm_bios_measurements_start
 	}
 
 	/* now check if current entry is valid */
-	if ((addr + sizeof(struct tcpa_event)) >= limit)
+	if ((addr + sizeof(struct tcpa_event)) > limit)
 		return NULL;
 
 	event = addr;
 
 	if ((event->event_type == 0 && event->event_size == 0) ||
-	    ((addr + sizeof(struct tcpa_event) + event->event_size) >= limit))
+	    ((addr + sizeof(struct tcpa_event) + event->event_size) > limit))
 		return NULL;
 
 	return addr;
@@ -111,7 +111,7 @@ static void *tpm_bios_measurements_next(
 	v += sizeof(struct tcpa_event) + event->event_size;
 
 	/* now check if current entry is valid */
-	if ((v + sizeof(struct tcpa_event)) >= limit)
+	if ((v + sizeof(struct tcpa_event)) > limit)
 		return NULL;
 
 	event = v;
@@ -120,7 +120,7 @@ static void *tpm_bios_measurements_next(
 		return NULL;
 
 	if ((event->event_type == 0 && event->event_size == 0) ||
-	    ((v + sizeof(struct tcpa_event) + event->event_size) >= limit))
+	    ((v + sizeof(struct tcpa_event) + event->event_size) > limit))
 		return NULL;
 
 	(*pos)++;