Linux-Integrity Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2 0/2] tpm/eventlog/tpm1: Small fixes
@ 2019-01-11  8:59 Jia Zhang
  2019-01-11  8:59 ` [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements Jia Zhang
  2019-01-11  8:59 ` [PATCH 2/2] tpm/eventlog/tpm1: Fix off-by-1 when reading binary_bios_measurements Jia Zhang
  0 siblings, 2 replies; 11+ messages in thread
From: Jia Zhang @ 2019-01-11  8:59 UTC (permalink / raw)
  To: jarkko.sakkinen, peterhuewe, jgg, tweek
  Cc: linux-integrity, linux-kernel, zhang.jia

Change since V1:

- Add test results with LTP.
- Rewrite patch 1's commit header.

Here is the test result with LTP testcase ima_tpm.sh which is used
to verify binary_bios_measurements.

ima_tpm 1 TINFO: timeout per run is 0h 5m 0s
ima_tpm 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-5.0.0-rc1+ root=UUID=c665e92c-736d-4b08-9143-a57396f935f3 ro rootwait crashkernel=auto console=tty0 console=ttyS0,115200 reboot=efi ima_hash=sha1
ima_tpm 1 TINFO: verify boot aggregate
ima_tpm 1 TPASS: bios aggregate matches IMA boot aggregate
ima_tpm 2 TINFO: verify PCR values
ima_tpm 2 TINFO: evmctl version: evmctl 1.1
ima_tpm 2 TCONF: TPM Hardware Support not enabled in kernel or no TPM chip found

Summary:
passed   1
failed   0   
skipped  1   
warnings 0

Note:
The 2nd test in ima_tpm requires /sys/class/tpm/tpm0/device/pcrs but this
interface is not available if TPM2 device used. So the test result showed
above is expected.

Jia

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
  2019-01-11  8:59 [PATCH v2 0/2] tpm/eventlog/tpm1: Small fixes Jia Zhang
@ 2019-01-11  8:59 ` Jia Zhang
  2019-01-16 22:09   ` Jarkko Sakkinen
  2019-01-11  8:59 ` [PATCH 2/2] tpm/eventlog/tpm1: Fix off-by-1 when reading binary_bios_measurements Jia Zhang
  1 sibling, 1 reply; 11+ messages in thread
From: Jia Zhang @ 2019-01-11  8:59 UTC (permalink / raw)
  To: jarkko.sakkinen, peterhuewe, jgg, tweek
  Cc: linux-integrity, linux-kernel, zhang.jia

The responsibility of tpm1_bios_measurements_start() is to walk
over the first *pos measurements, ensuring the skipped and
to-be-read measurements are not out-of-boundary.

Current logic is complicated a bit. Just employ a do-while loop
with necessary sanity check, and then get the goal.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] tpm/eventlog/tpm1: Fix off-by-1 when reading binary_bios_measurements
  2019-01-11  8:59 [PATCH v2 0/2] tpm/eventlog/tpm1: Small fixes Jia Zhang
  2019-01-11  8:59 ` [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements Jia Zhang
@ 2019-01-11  8:59 ` Jia Zhang
  2019-01-16 22:17   ` Jarkko Sakkinen
  1 sibling, 1 reply; 11+ messages in thread
From: Jia Zhang @ 2019-01-11  8:59 UTC (permalink / raw)
  To: jarkko.sakkinen, peterhuewe, jgg, tweek
  Cc: linux-integrity, linux-kernel, zhang.jia

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: 55a82ab("tpm: add bios measurement log")
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
  2019-01-11  8:59 ` [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements Jia Zhang
@ 2019-01-16 22:09   ` Jarkko Sakkinen
  2019-01-17  1:32     ` Jia Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Jarkko Sakkinen @ 2019-01-16 22:09 UTC (permalink / raw)
  To: Jia Zhang; +Cc: peterhuewe, jgg, tweek, linux-integrity, linux-kernel

Please use "tpm:" tag for commits, not "tpm/eventlog/tpm1".

On Fri, Jan 11, 2019 at 04:59:32PM +0800, Jia Zhang wrote:
> The responsibility of tpm1_bios_measurements_start() is to walk
> over the first *pos measurements, ensuring the skipped and
> to-be-read measurements are not out-of-boundary.
> 
> Current logic is complicated a bit. Just employ a do-while loop
> with necessary sanity check, and then get the goal.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

What does this fix? Even if the current logic is "complicated", it is
still a pretty simple functiion.

Applying clean ups for fun has the side-effect of making backporting
more difficult. And swapping implementation randomly has the side-effect
of potentially introducing regressions. The current code might be messy
but it is still field tested.

I'm sorry but I have to reject this patch.

/Jarkko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] tpm/eventlog/tpm1: Fix off-by-1 when reading binary_bios_measurements
  2019-01-11  8:59 ` [PATCH 2/2] tpm/eventlog/tpm1: Fix off-by-1 when reading binary_bios_measurements Jia Zhang
@ 2019-01-16 22:17   ` Jarkko Sakkinen
  0 siblings, 0 replies; 11+ messages in thread
From: Jarkko Sakkinen @ 2019-01-16 22:17 UTC (permalink / raw)
  To: Jia Zhang, nayna; +Cc: peterhuewe, jgg, tweek, linux-integrity, linux-kernel

On Fri, Jan 11, 2019 at 04:59:33PM +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.
> 
> Fixes: 55a82ab("tpm: add bios measurement log")
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

LGTM, Nayna?

/Jarkko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
  2019-01-16 22:09   ` Jarkko Sakkinen
@ 2019-01-17  1:32     ` Jia Zhang
  2019-01-18 15:18       ` Jarkko Sakkinen
  0 siblings, 1 reply; 11+ messages in thread
From: Jia Zhang @ 2019-01-17  1:32 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: peterhuewe, jgg, tweek, linux-integrity, linux-kernel



On 2019/1/17 上午6:09, Jarkko Sakkinen wrote:
> Please use "tpm:" tag for commits, not "tpm/eventlog/tpm1".
> 
> On Fri, Jan 11, 2019 at 04:59:32PM +0800, Jia Zhang wrote:
>> The responsibility of tpm1_bios_measurements_start() is to walk
>> over the first *pos measurements, ensuring the skipped and
>> to-be-read measurements are not out-of-boundary.
>>
>> Current logic is complicated a bit. Just employ a do-while loop
>> with necessary sanity check, and then get the goal.
>>
>> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> 
> What does this fix? Even if the current logic is "complicated", it is
> still a pretty simple functiion.


OK. Let me point out the fix part. Here is the original implementation:

 87         /* read over *pos measurements */
 88         for (i = 0; i < *pos; i++) {
 89                 event = addr;
 90
 91                 converted_event_size =
 92                     do_endian_conversion(event->event_size);
 93                 converted_event_type =
 94                     do_endian_conversion(event->event_type);
 95
 96                 if ((addr + sizeof(struct tcpa_event)) < limit) {
 97                         if ((converted_event_type == 0) &&
 98                             (converted_event_size == 0))
 99                                 return NULL;
100                         addr += (sizeof(struct tcpa_event) +
101                                  converted_event_size);
102                 }
103         }

The problem (just ignore all off-by-1 issues) is that accessing to
event_size and event_type is not pre-checked carefully. In the latter
part of tpm1_bios_measurements_start() and
tpm1_bios_measurements_next(), there is a fixed patter to do the sanity
check like this:

136         /* now check if current entry is valid */
137         if ((v + sizeof(struct tcpa_event)) >= limit)
138                 return NULL;

So if we simply change this read-over chunk with sanity check like this:

        /* read over *pos measurements */
        for (i = 0; i < *pos; i++) {
                event = addr;

                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 ((converted_event_type == 0) &&
                    (converted_event_size == 0))
                        return NULL;
                addr += (sizeof(struct tcpa_event) +
                         converted_event_size);
        }

We will get two highly similar code chunks in
tpm1_bios_measurements_start(). Here is the latter part:

106         /* now check if current entry is valid */
107         if ((addr + sizeof(struct tcpa_event)) >= limit)
108                 return NULL;
109
110         event = addr;
111
112         converted_event_size = do_endian_conversion(event->event_size);
113         converted_event_type = do_endian_conversion(event->event_type);
114
115         if (((converted_event_type == 0) && (converted_event_size == 0))
116             || ((addr + sizeof(struct tcpa_event) +
converted_event_size)
117                 >= limit))
118                 return NULL;
119
120         return addr;

So using a do while logic can simply merge them together and thus simply
and optimize the logic of walking over *pos measurements.

Sorry I admit my initial motivation is to fix up the sanity check
problem. If you would like to accept the optimization part, I will split
this patch.

Jia

> 
> Applying clean ups for fun has the side-effect of making backporting
> more difficult. And swapping implementation randomly has the side-effect
> of potentially introducing regressions. The current code might be messy
> but it is still field tested.
> 
> I'm sorry but I have to reject this patch.
> 
> /Jarkko
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
  2019-01-17  1:32     ` Jia Zhang
@ 2019-01-18 15:18       ` Jarkko Sakkinen
  2019-01-19  7:48         ` Jia Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Jarkko Sakkinen @ 2019-01-18 15:18 UTC (permalink / raw)
  To: Jia Zhang; +Cc: peterhuewe, jgg, tweek, linux-integrity, linux-kernel

On Thu, Jan 17, 2019 at 09:32:55AM +0800, Jia Zhang wrote:
> 
> 
> On 2019/1/17 上午6:09, Jarkko Sakkinen wrote:
> > Please use "tpm:" tag for commits, not "tpm/eventlog/tpm1".
> > 
> > On Fri, Jan 11, 2019 at 04:59:32PM +0800, Jia Zhang wrote:
> >> The responsibility of tpm1_bios_measurements_start() is to walk
> >> over the first *pos measurements, ensuring the skipped and
> >> to-be-read measurements are not out-of-boundary.
> >>
> >> Current logic is complicated a bit. Just employ a do-while loop
> >> with necessary sanity check, and then get the goal.
> >>
> >> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> > 
> > What does this fix? Even if the current logic is "complicated", it is
> > still a pretty simple functiion.
> 
> 
> OK. Let me point out the fix part. Here is the original implementation:
> 
>  87         /* read over *pos measurements */
>  88         for (i = 0; i < *pos; i++) {
>  89                 event = addr;
>  90
>  91                 converted_event_size =
>  92                     do_endian_conversion(event->event_size);
>  93                 converted_event_type =
>  94                     do_endian_conversion(event->event_type);
>  95
>  96                 if ((addr + sizeof(struct tcpa_event)) < limit) {
>  97                         if ((converted_event_type == 0) &&
>  98                             (converted_event_size == 0))
>  99                                 return NULL;
> 100                         addr += (sizeof(struct tcpa_event) +
> 101                                  converted_event_size);
> 102                 }
> 103         }
> 
> The problem (just ignore all off-by-1 issues) is that accessing to
> event_size and event_type is not pre-checked carefully. In the latter
> part of tpm1_bios_measurements_start() and
> tpm1_bios_measurements_next(), there is a fixed patter to do the sanity
> check like this:
> 
> 136         /* now check if current entry is valid */
> 137         if ((v + sizeof(struct tcpa_event)) >= limit)
> 138                 return NULL;
> 
> So if we simply change this read-over chunk with sanity check like this:
> 
>         /* read over *pos measurements */
>         for (i = 0; i < *pos; i++) {
>                 event = addr;
> 
>                 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 ((converted_event_type == 0) &&
>                     (converted_event_size == 0))
>                         return NULL;
>                 addr += (sizeof(struct tcpa_event) +
>                          converted_event_size);
>         }
> 
> We will get two highly similar code chunks in
> tpm1_bios_measurements_start(). Here is the latter part:
> 
> 106         /* now check if current entry is valid */
> 107         if ((addr + sizeof(struct tcpa_event)) >= limit)
> 108                 return NULL;
> 109
> 110         event = addr;
> 111
> 112         converted_event_size = do_endian_conversion(event->event_size);
> 113         converted_event_type = do_endian_conversion(event->event_type);
> 114
> 115         if (((converted_event_type == 0) && (converted_event_size == 0))
> 116             || ((addr + sizeof(struct tcpa_event) +
> converted_event_size)
> 117                 >= limit))
> 118                 return NULL;
> 119
> 120         return addr;
> 
> So using a do while logic can simply merge them together and thus simply
> and optimize the logic of walking over *pos measurements.
> 
> Sorry I admit my initial motivation is to fix up the sanity check
> problem. If you would like to accept the optimization part, I will split
> this patch.

OK, got it now. I think I will apply this! Will take a while because
of https://lkml.org/lkml/2019/1/18/485. Will not apply new patches
before that is rooted.

/Jarkko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
  2019-01-18 15:18       ` Jarkko Sakkinen
@ 2019-01-19  7:48         ` Jia Zhang
  0 siblings, 0 replies; 11+ messages in thread
From: Jia Zhang @ 2019-01-19  7:48 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: peterhuewe, jgg, tweek, linux-integrity, linux-kernel



On 2019/1/18 下午11:18, Jarkko Sakkinen wrote:
> On Thu, Jan 17, 2019 at 09:32:55AM +0800, Jia Zhang wrote:
>>
>>
>> On 2019/1/17 上午6:09, Jarkko Sakkinen wrote:
>>> Please use "tpm:" tag for commits, not "tpm/eventlog/tpm1".
>>>

... snipped

> 
> OK, got it now. I think I will apply this! Will take a while because
> of https://lkml.org/lkml/2019/1/18/485. Will not apply new patches
> before that is rooted.

No problem. Thanks for your reviews.

Cheers,
Jia

> 
> /Jarkko
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
  2019-01-10 17:32 ` Jarkko Sakkinen
@ 2019-01-11  8:29   ` Jia Zhang
  0 siblings, 0 replies; 11+ messages in thread
From: Jia Zhang @ 2019-01-11  8:29 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: peterhuewe, jgg, tweek, linux-integrity, linux-kernel



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 <zhang.jia@linux.alibaba.com>
> 
> 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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
  2019-01-06  7:23 [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements Jia Zhang
@ 2019-01-10 17:32 ` Jarkko Sakkinen
  2019-01-11  8:29   ` Jia Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Jarkko Sakkinen @ 2019-01-10 17:32 UTC (permalink / raw)
  To: Jia Zhang; +Cc: peterhuewe, jgg, tweek, linux-integrity, linux-kernel

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 <zhang.jia@linux.alibaba.com>

The cover letter is missing and commit messages do not describe
what kind of change the commit does.

/Jarkko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements
@ 2019-01-06  7:23 Jia Zhang
  2019-01-10 17:32 ` Jarkko Sakkinen
  0 siblings, 1 reply; 11+ messages in thread
From: Jia Zhang @ 2019-01-06  7:23 UTC (permalink / raw)
  To: jarkko.sakkinen, peterhuewe, jgg, tweek
  Cc: linux-integrity, linux-kernel, zhang.jia

The sanity check would be easier, especially for the first read
of binary_bios_measurements from the beginning.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11  8:59 [PATCH v2 0/2] tpm/eventlog/tpm1: Small fixes Jia Zhang
2019-01-11  8:59 ` [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements Jia Zhang
2019-01-16 22:09   ` Jarkko Sakkinen
2019-01-17  1:32     ` Jia Zhang
2019-01-18 15:18       ` Jarkko Sakkinen
2019-01-19  7:48         ` Jia Zhang
2019-01-11  8:59 ` [PATCH 2/2] tpm/eventlog/tpm1: Fix off-by-1 when reading binary_bios_measurements Jia Zhang
2019-01-16 22:17   ` Jarkko Sakkinen
  -- strict thread matches above, loose matches on Subject: below --
2019-01-06  7:23 [PATCH 1/2] tpm/eventlog/tpm1: Simplify walking over *pos measurements Jia Zhang
2019-01-10 17:32 ` Jarkko Sakkinen
2019-01-11  8:29   ` Jia Zhang

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org linux-integrity@archiver.kernel.org
	public-inbox-index linux-integrity


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/ public-inbox