Re: [PATCH v2 1/2] efi+tpm: Don't access event->count when it isn't mapped.

From: Jerry Snitselaar
Date: Wed Sep 25 2019 - 12:41:54 EST


On Wed Sep 25 19, Jerry Snitselaar wrote:
On Wed Sep 25 19, Jarkko Sakkinen wrote:
On Wed, Sep 25, 2019 at 12:25:05PM +0200, Ard Biesheuvel wrote:
On Wed, 25 Sep 2019 at 12:16, Jarkko Sakkinen
<jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote:

From: Peter Jones <pjones@xxxxxxxxxx>

Some machines generate a lot of event log entries. When we're
iterating over them, the code removes the old mapping and adds a
new one, so once we cross the page boundary we're unmapping the page
with the count on it. Hilarity ensues.

This patch keeps the info from the header in local variables so we don't
need to access that page again or keep track of if it's mapped.

Fixes: 44038bc514a2 ("tpm: Abstract crypto agile event size calculations")
Cc: linux-efi@xxxxxxxxxxxxxxx
Cc: linux-integrity@xxxxxxxxxxxxxxx
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Peter Jones <pjones@xxxxxxxxxx>
Tested-by: Lyude Paul <lyude@xxxxxxxxxx>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
Acked-by: Matthew Garrett <mjg59@xxxxxxxxxx>
Acked-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>

Thanks Jarkko.

Shall I take these through the EFI tree?

Would be great, if you could because I already sent one PR with fixes for
v5.4-rc1 yesterday.

/Jarkko

My patch collides with this, so I will submit a v3 that applies on top of
these once I've run a test with all 3 applied on this t480s.

Tested with Peter's patches, and that was the root cause on this 480s.

I think there should still be a check for tbl_size to make sure we
aren't sticking -1 into efi_tpm_final_log_size though, which will be
the case right now if it fails to parse an event.