diff mbox series

[v2,2/2] tpm: tpm2_bios_measurements_next should increase position index

Message ID 16bde2d6-4208-e478-0ac3-163b5c3a1eaa@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] tpm: tpm1_bios_measurements_next should increase position index | expand

Commit Message

Vasily Averin Jan. 30, 2020, 10:23 a.m. UTC
If seq_file .next function does not change position index,
read after non-zero lseek can generate unexpected output.
For /sys/kernel/security/tpm0/binary_bios_measurements:
1) read after lseek beyond end of file generates whole last line.
2) read after lseek to middle of last line generates
expected end of last line and unexpected whole last line once again.

Cc: stable@vger.kernel.org
Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code ...")

https://bugzilla.kernel.org/show_bug.cgi?id=206283
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 drivers/char/tpm/eventlog/tpm2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jarkko Sakkinen Feb. 1, 2020, 5:03 p.m. UTC | #1
On Thu, Jan 30, 2020 at 01:23:21PM +0300, Vasily Averin wrote:
> If seq_file .next function does not change position index,
> read after non-zero lseek can generate unexpected output.

Is it unwanted or unexpected? Unexpected would be mean random
output. I don't think that is the case. Please describe more
throughly.

> For /sys/kernel/security/tpm0/binary_bios_measurements:
> 1) read after lseek beyond end of file generates whole last line.
> 2) read after lseek to middle of last line generates
> expected end of last line and unexpected whole last line once again.
> 
> Cc: stable@vger.kernel.org
> Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code ...")
> 
No empty line here.

> https://bugzilla.kernel.org/show_bug.cgi?id=206283

"Link: https://bugzilla.kernel.org/show_bug.cgi?id=206283"

/Jarkko
Vasily Averin Feb. 3, 2020, 5:14 a.m. UTC | #2
On 2/1/20 8:03 PM, Jarkko Sakkinen wrote:
> On Thu, Jan 30, 2020 at 01:23:21PM +0300, Vasily Averin wrote:
>> If seq_file .next function does not change position index,
>> read after non-zero lseek can generate unexpected output.
> 
> Is it unwanted or unexpected? Unexpected would be mean random
> output. I don't think that is the case. Please describe more
> throughly.

If .next function does not change position index, 
following .show function will repeat output related to current position index.

>> For /sys/kernel/security/tpm0/binary_bios_measurements:
>> 1) read after lseek beyond end of file generates whole last line.
>> 2) read after lseek to middle of last line generates
>> expected end of last line and unexpected whole last line once again.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code ...")
>>
> No empty line here.
> 
>> https://bugzilla.kernel.org/show_bug.cgi?id=206283
> 
> "Link: https://bugzilla.kernel.org/show_bug.cgi?id=206283"
> 
> /Jarkko
>
Jarkko Sakkinen Feb. 5, 2020, 10:03 p.m. UTC | #3
On Mon, Feb 03, 2020 at 08:14:53AM +0300, Vasily Averin wrote:
> On 2/1/20 8:03 PM, Jarkko Sakkinen wrote:
> > On Thu, Jan 30, 2020 at 01:23:21PM +0300, Vasily Averin wrote:
> >> If seq_file .next function does not change position index,
> >> read after non-zero lseek can generate unexpected output.
> > 
> > Is it unwanted or unexpected? Unexpected would be mean random
> > output. I don't think that is the case. Please describe more
> > throughly.
> 
> If .next function does not change position index, 
> following .show function will repeat output related to current position index.

Thank you. That is clear and concise.

Then, please put that to the commit message instead of "unexpected
output".

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index b9aeda1..e741b11 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -94,6 +94,7 @@  static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
 	size_t event_size;
 	void *marker;
 
+	(*pos)++;
 	event_header = log->bios_event_log;
 
 	if (v == SEQ_START_TOKEN) {
@@ -118,7 +119,6 @@  static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
 	if (((v + event_size) >= limit) || (event_size == 0))
 		return NULL;
 
-	(*pos)++;
 	return v;
 }