The sanity check would be easier, especially for the first read
of binary_bios_measurements from the beginning.
Signed-off-by: Jia Zhang <[email protected]>
---
drivers/char/tpm/eventlog/tpm1.c | 37 ++++++++++++++-----------------------
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c
index 58c8478..4cf8303 100644
--- a/drivers/char/tpm/eventlog/tpm1.c
+++ b/drivers/char/tpm/eventlog/tpm1.c
@@ -74,7 +74,7 @@
/* returns pointer to start of pos. entry of tcg log */
static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos)
{
- loff_t i;
+ loff_t i = 0;
struct tpm_chip *chip = m->private;
struct tpm_bios_log *log = &chip->log;
void *addr = log->bios_event_log;
@@ -83,38 +83,29 @@ static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos)
u32 converted_event_size;
u32 converted_event_type;
-
/* read over *pos measurements */
- for (i = 0; i < *pos; i++) {
+ do {
event = addr;
+ /* check if current entry is valid */
+ if (addr + sizeof(struct tcpa_event) >= limit)
+ return NULL;
+
converted_event_size =
do_endian_conversion(event->event_size);
converted_event_type =
do_endian_conversion(event->event_type);
- if ((addr + sizeof(struct tcpa_event)) < limit) {
- if ((converted_event_type == 0) &&
- (converted_event_size == 0))
- return NULL;
- addr += (sizeof(struct tcpa_event) +
- converted_event_size);
- }
- }
-
- /* now check if current entry is valid */
- if ((addr + sizeof(struct tcpa_event)) >= limit)
- return NULL;
-
- event = addr;
+ if (((converted_event_type == 0) && (converted_event_size == 0))
+ || ((addr + sizeof(struct tcpa_event) + converted_event_size)
+ >= limit))
+ return NULL;
- converted_event_size = do_endian_conversion(event->event_size);
- converted_event_type = do_endian_conversion(event->event_type);
+ if (i++ == *pos)
+ break;
- if (((converted_event_type == 0) && (converted_event_size == 0))
- || ((addr + sizeof(struct tcpa_event) + converted_event_size)
- >= limit))
- return NULL;
+ addr += (sizeof(struct tcpa_event) + converted_event_size);
+ } while (1);
return addr;
}
--
1.8.3.1
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.
Fix: 55a82ab("tpm: add bios measurement log")
Signed-off-by: Jia Zhang <[email protected]>
---
drivers/char/tpm/eventlog/tpm1.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c
index 4cf8303..bfdff92 100644
--- a/drivers/char/tpm/eventlog/tpm1.c
+++ b/drivers/char/tpm/eventlog/tpm1.c
@@ -88,7 +88,7 @@ static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos)
event = addr;
/* check if current entry is valid */
- if (addr + sizeof(struct tcpa_event) >= limit)
+ if (addr + sizeof(struct tcpa_event) > limit)
return NULL;
converted_event_size =
@@ -98,7 +98,7 @@ static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos)
if (((converted_event_type == 0) && (converted_event_size == 0))
|| ((addr + sizeof(struct tcpa_event) + converted_event_size)
- >= limit))
+ > limit))
return NULL;
if (i++ == *pos)
@@ -125,7 +125,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v,
v += sizeof(struct tcpa_event) + converted_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;
@@ -134,7 +134,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v,
converted_event_type = do_endian_conversion(event->event_type);
if (((converted_event_type == 0) && (converted_event_size == 0)) ||
- ((v + sizeof(struct tcpa_event) + converted_event_size) >= limit))
+ ((v + sizeof(struct tcpa_event) + converted_event_size) > limit))
return NULL;
(*pos)++;
--
1.8.3.1
On Sun, Jan 06, 2019 at 03:23:18PM +0800, Jia Zhang wrote:
> The sanity check would be easier, especially for the first read
> of binary_bios_measurements from the beginning.
>
> Signed-off-by: Jia Zhang <[email protected]>
The cover letter is missing and commit messages do not describe
what kind of change the commit does.
/Jarkko
On Sun, Jan 06, 2019 at 03:23:19PM +0800, Jia Zhang wrote:
> 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.
>
> Fix: 55a82ab("tpm: add bios measurement log")
Fixes:
/Jarkko
On 2019/1/11 上午1:32, Jarkko Sakkinen wrote:
> On Sun, Jan 06, 2019 at 03:23:18PM +0800, Jia Zhang wrote:
>> The sanity check would be easier, especially for the first read
>> of binary_bios_measurements from the beginning.
>>
>> Signed-off-by: Jia Zhang <[email protected]>
>
> The cover letter is missing and commit messages do not describe
> what kind of change the commit does.
Thanks for the comments. I will send V2.
Cheers,
Jia
>
> /Jarkko
>