Re: [PATCH] efi: expose TPM event log to userspace via sysfs

[Jarkko Sakkinen]

On Fri Apr 26, 2024 at 11:19 AM EEST, Mikko Rapeli wrote:
> Hi,
>
> On Fri, Apr 26, 2024 at 10:40:20AM +0300, Jarkko Sakkinen wrote:
> > On Fri Apr 26, 2024 at 10:35 AM EEST, Jarkko Sakkinen wrote:
> > > On Thu Apr 25, 2024 at 5:01 PM EEST, Jarkko Sakkinen wrote:
> > > > On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> > > > > General purpose distros typically don't build all TPM drivers into the
> > > > > kernel, but ship some in the initrd instead. Then, udev is responsible
> > > > > for iterating all buses/devices and auto-loading the necessary
> > > > > drivers. Each loaded bus driver might make more devices available for
> > > >
> > > > I've had since day 0 that I've worked with TPM driver (i.e. since 2013
> > >
> > > - had the opinion (typo)
> > 
> > Tbh, I have zero idea what this discussion is about anyway because the
> > original thread *was not* CC'd to linux-integrity and I'm not subscribed
> > to linux-efi. So next time put all the relevant mailing lists. I.e.
> > definitive NAK for this patch.
>
> Sorry for not including linux-integrity. I added maintainers and lists
> proposed by scripts/get_maintainers.pl for the change which did not touch
> drivers/char/tpm/ though TPM event log APIs are clearly there.
>
> The full thread starts from here:
>
> https://lore.kernel.org/all/20240422112711.362779-1-mikko.rap...@linaro.org/T/#u

NP, just add CC to the next version.

I'll download it and check through once bandwidth.

> Cheers,
>
> -Mikko

BR, Jarkko

Re: [PATCH] efi: expose TPM event log to userspace via sysfs

[Jarkko Sakkinen]

On Fri Apr 26, 2024 at 10:35 AM EEST, Jarkko Sakkinen wrote:
> On Thu Apr 25, 2024 at 5:01 PM EEST, Jarkko Sakkinen wrote:
> > On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> > > General purpose distros typically don't build all TPM drivers into the
> > > kernel, but ship some in the initrd instead. Then, udev is responsible
> > > for iterating all buses/devices and auto-loading the necessary
> > > drivers. Each loaded bus driver might make more devices available for
> >
> > I've had since day 0 that I've worked with TPM driver (i.e. since 2013
>
> - had the opinion (typo)

Tbh, I have zero idea what this discussion is about anyway because the
original thread *was not* CC'd to linux-integrity and I'm not subscribed
to linux-efi. So next time put all the relevant mailing lists. I.e.
definitive NAK for this patch.

BR, Jarkko

Re: [PATCH] efi: expose TPM event log to userspace via sysfs

[Jarkko Sakkinen]

On Thu Apr 25, 2024 at 5:01 PM EEST, Jarkko Sakkinen wrote:
> On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> > General purpose distros typically don't build all TPM drivers into the
> > kernel, but ship some in the initrd instead. Then, udev is responsible
> > for iterating all buses/devices and auto-loading the necessary
> > drivers. Each loaded bus driver might make more devices available for
>
> I've had since day 0 that I've worked with TPM driver (i.e. since 2013

- had the opinion (typo)

BR, Jarkko

Re: [PATCH] efi: expose TPM event log to userspace via sysfs

[Jarkko Sakkinen]

On Thu Apr 25, 2024 at 12:58 PM EEST, Lennart Poettering wrote:
> General purpose distros typically don't build all TPM drivers into the
> kernel, but ship some in the initrd instead. Then, udev is responsible
> for iterating all buses/devices and auto-loading the necessary
> drivers. Each loaded bus driver might make more devices available for

I've had since day 0 that I've worked with TPM driver (i.e. since 2013
or 2014) that module support should be removed.

I've kept the module compilation only because huge turnback from the
community.

It does not make sense:

1. Because it makes sense as part of "TCB".
2. "TCB" is should in be vmlinux.
3. TPM is also a subsystem with other clients in the kernel.

At minimum the main TPM driver should IMHO just in vmlinux e.g. because
it is rare to see distro kernel with TPM enabled and IMA disabled, I
don't know any.

That said, I would not mind either if TPM subsystem drivers were only
y/n *except* tpm_vtpm_proxy.

BR, Jarkko

Re: [PATCH] efi: expose TPM event log to userspace via sysfs

[Jarkko Sakkinen]

On Thu Apr 25, 2024 at 11:56 AM EEST, Mikko Rapeli wrote:
> 1) is there a TPM device

Translates to "Does /sys/class/tpm/tpm0 exists?"

TPM version can be determined with
/sys/class/tpm/tpm0/tpm_version_major

BR, Jarkko

Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

[Jarkko Sakkinen]

On Wed, Oct 16, 2019 at 08:23:56AM -0700, Joe Perches wrote:
> On Wed, 2019-10-16 at 18:20 +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 14, 
> 2019 at 11:18:25PM +0200, Borislav Petkov wrote:
> > > On Mon, Oct 14, 2019 at 11:21:11PM +0300, Jarkko Sakkinen wrote:
> > > > Was there a section in the patch submission documentation to point out
> > > > when people send patches with all the possible twists for an acronym?
> > > 
> > > I don't think so.
> > > 
> > > > This is giving me constantly gray hairs with TPM patches.
> > > 
> > > Well, I'm slowly getting tired of repeating the same crap over and over
> > > again about how important it is to document one's changes and to write
> > > good commit messages. The most repeated answers I'm simply putting into
> > > canned reply templates because, well, saying it once or twice is not
> > > enough anymore. :-\
> > > 
> > > And yeah, I see your pain. Same here, actually.
> > > 
> > > In the acronym case, I'd probably add a regex to my patch massaging
> > > script and convert those typos automatically and be done with it.
> > 
> > Wonder if checkpatch.pl could be extended to know acronyms e.g. have a
> > db of known acronyms.
> 
> ?  examples please.
> 
> checkpatch has a db for misspellings, I supposed another for
> acronyms could be added, but how would false positives be avoided?

TPM should be always TPM, e.g. not tpm. EFI should be always, e.g.
not efi.

/Jarkko

Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

[Jarkko Sakkinen]

On Mon, Oct 14, 2019 at 11:18:25PM +0200, Borislav Petkov wrote:
> On Mon, Oct 14, 2019 at 11:21:11PM +0300, Jarkko Sakkinen wrote:
> > Was there a section in the patch submission documentation to point out
> > when people send patches with all the possible twists for an acronym?
> 
> I don't think so.
> 
> > This is giving me constantly gray hairs with TPM patches.
> 
> Well, I'm slowly getting tired of repeating the same crap over and over
> again about how important it is to document one's changes and to write
> good commit messages. The most repeated answers I'm simply putting into
> canned reply templates because, well, saying it once or twice is not
> enough anymore. :-\
> 
> And yeah, I see your pain. Same here, actually.
> 
> In the acronym case, I'd probably add a regex to my patch massaging
> script and convert those typos automatically and be done with it.

Wonder if checkpatch.pl could be extended to know acronyms e.g. have a
db of known acronyms.

/Jarkko

Re: [PATCH] efi/tpm: return -EINVAL when determining tpm final events log size fails

[Jarkko Sakkinen]

On Mon, Oct 14, 2019 at 10:21:45AM -0700, Jerry Snitselaar wrote:
> Currently nothing checks the return value of efi_tpm_eventlog_init,
> but in case that changes in the future make sure an error is
> returned when it fails to determine the tpm final events log
> size.
> 
> Cc: Ard Biesheuvel 
> Cc: Jarkko Sakkinen 
> Cc: linux-efi@vger.kernel.org
> Cc: linux-integr...@vger.kernel.org
> Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size' after 
> successful event log parsing")
> Suggested-by: Dan Carpenter 
> Signed-off-by: Jerry Snitselaar 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

[Jarkko Sakkinen]

On Mon, Oct 14, 2019 at 12:14:19PM +0200, Borislav Petkov wrote:
> Your spelling of "EFI" is like a random number generator in this
> paragraph: "Efi", "efi" and "EFI". Can you please be more careful when
> writing your commit messages? They're not some random text you hurriedly
> jot down before sending the patch but a most important description of
> why a change is being done.

Was there a section in the patch submission documentation to point out
when people send patches with all the possible twists for an acronym?

This is giving me constantly gray hairs with TPM patches.

/Jarkko

Re: [PATCH v3] tpm: only set efi_tpm_final_log_size after successful event log parsing

[Jarkko Sakkinen]

DR6: fffe0ff0 DR7: 
> 0400
> [0.774788] Kernel panic - not syncing: Fatal exception
> [0.774788] Kernel Offset: 0x1d00 from 0x8100 (relocation 
> range: 0x8000-0xbfff)
> [0.774788] ---[ end Kernel panic - not syncing: Fatal exception ]---
> 
> The root cause of the issue that caused the failure of event parsing
> in this case is resolved by Peter Jone's patchset dealing with large
> event logs where crossing over a page boundary causes the page with
> the event count to be unmapped.
> 
> Fixes: c46f3405692de ("tpm: Reserve the TPM final events table")
> Cc: linux-efi@vger.kernel.org
> Cc: linux-integr...@vger.kernel.org
> Cc: sta...@vger.kernel.org
> Cc: Matthew Garrett 
> Cc: Ard Biesheuvel 
> Cc: Jarkko Sakkinen 
> Signed-off-by: Jerry Snitselaar 

Reviewed-by:  

/Jarkko

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

[Jarkko Sakkinen]

On Wed, Sep 25, 2019 at 09:41:33AM -0700, Jerry Snitselaar wrote:
> 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
> > > >  wrote:
> > > > > 
> > > > > From: Peter Jones 
> > > > > 
> > > > > 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@vger.kernel.org
> > > > > Cc: linux-integr...@vger.kernel.org
> > > > > Cc: sta...@vger.kernel.org
> > > > > Signed-off-by: Peter Jones 
> > > > > Tested-by: Lyude Paul 
> > > > > Reviewed-by: Jarkko Sakkinen 
> > > > > Acked-by: Matthew Garrett 
> > > > > Acked-by: Ard Biesheuvel 
> > > > > Signed-off-by: Jarkko Sakkinen 
> > > > 
> > > > 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.

You could sent a follow up patch for that I think. The current
ones are kind of already "went through the process" and do right
things but I do agree that a sanity check would make sense just
in case.

/Jarkko

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

[Jarkko Sakkinen]

On Wed, Sep 25, 2019 at 08:16:16AM -0700, 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
> > >  wrote:
> > > >
> > > > From: Peter Jones 
> > > >
> > > > 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@vger.kernel.org
> > > > Cc: linux-integr...@vger.kernel.org
> > > > Cc: sta...@vger.kernel.org
> > > > Signed-off-by: Peter Jones 
> > > > Tested-by: Lyude Paul 
> > > > Reviewed-by: Jarkko Sakkinen 
> > > > Acked-by: Matthew Garrett 
> > > > Acked-by: Ard Biesheuvel 
> > > > Signed-off-by: Jarkko Sakkinen 
> > > 
> > > 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.

Great, thanks.

/Jarkko

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

[Jarkko Sakkinen]

On Wed, Sep 25, 2019 at 12:25:05PM +0200, Ard Biesheuvel wrote:
> On Wed, 25 Sep 2019 at 12:16, Jarkko Sakkinen
>  wrote:
> >
> > From: Peter Jones 
> >
> > 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@vger.kernel.org
> > Cc: linux-integr...@vger.kernel.org
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Peter Jones 
> > Tested-by: Lyude Paul 
> > Reviewed-by: Jarkko Sakkinen 
> > Acked-by: Matthew Garrett 
> > Acked-by: Ard Biesheuvel 
> > Signed-off-by: Jarkko Sakkinen 
> 
> 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

Re: [RFC PATCH] tpm: only set efi_tpm_final_log_size after successful event log parsing

[Jarkko Sakkinen]

On Wed, Sep 18, 2019 at 12:16:26PM -0700, Jerry Snitselaar wrote:
> + if (tbl_size < 0) {
> + pr_err("Failed to parse event in TPM Final Event log\n");

FW_BUG?

> + goto calc_out;
> + }
> +
>   memblock_reserve((unsigned long)final_tbl,
>tbl_size + sizeof(*final_tbl));
> - early_memunmap(final_tbl, sizeof(*final_tbl));
>   efi_tpm_final_log_size = tbl_size;
>  
> +calc_out:
> + early_memunmap(final_tbl, sizeof(*final_tbl));

out_calc

>  out:
>   early_memunmap(log_tbl, sizeof(*log_tbl));
>   return ret;
> -- 
> 2.23.0
> 

/Jarkko

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

[Jarkko Sakkinen]

From: Peter Jones 

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@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Cc: sta...@vger.kernel.org
Signed-off-by: Peter Jones 
Tested-by: Lyude Paul 
Reviewed-by: Jarkko Sakkinen 
Acked-by: Matthew Garrett 
Acked-by: Ard Biesheuvel 
Signed-off-by: Jarkko Sakkinen 
---
 include/linux/tpm_eventlog.h | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 63238c84dc0b..12584b69a3f3 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -170,6 +170,7 @@ static inline int __calc_tpm2_event_size(struct 
tcg_pcr_event2_head *event,
u16 halg;
int i;
int j;
+   u32 count, event_type;
 
marker = event;
marker_start = marker;
@@ -190,16 +191,22 @@ static inline int __calc_tpm2_event_size(struct 
tcg_pcr_event2_head *event,
}
 
event = (struct tcg_pcr_event2_head *)mapping;
+   /*
+* the loop below will unmap these fields if the log is larger than
+* one page, so save them here for reference.
+*/
+   count = READ_ONCE(event->count);
+   event_type = READ_ONCE(event->event_type);
 
efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
 
/* Check if event is malformed. */
-   if (event->count > efispecid->num_algs) {
+   if (count > efispecid->num_algs) {
size = 0;
goto out;
}
 
-   for (i = 0; i < event->count; i++) {
+   for (i = 0; i < count; i++) {
halg_size = sizeof(event->digests[i].alg_id);
 
/* Map the digest's algorithm identifier */
@@ -256,8 +263,9 @@ static inline int __calc_tpm2_event_size(struct 
tcg_pcr_event2_head *event,
+ event_field->event_size;
size = marker - marker_start;
 
-   if ((event->event_type == 0) && (event_field->event_size == 0))
+   if (event_type == 0 && event_field->event_size == 0)
size = 0;
+
 out:
if (do_mapping)
TPM_MEMUNMAP(mapping, mapping_size);
-- 
2.20.1

[PATCH v2 2/2] efi+tpm: don't traverse an event log with no events

[Jarkko Sakkinen]

From: Peter Jones 

When there are no entries to put into the final event log, some machines
will return the template they would have populated anyway.  In this case
the nr_events field is 0, but the rest of the log is just garbage.

This patch stops us from trying to iterate the table with
__calc_tpm2_event_size() when the number of events in the table is 0.

Fixes: c46f3405692d ("tpm: Reserve the TPM final events table")
Cc: linux-efi@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Cc: sta...@vger.kernel.org
Signed-off-by: Peter Jones 
Tested-by: Lyude Paul 
Reviewed-by: Jarkko Sakkinen 
Acked-by: Matthew Garrett 
Acked-by: Ard Biesheuvel 
Signed-off-by: Jarkko Sakkinen 
---
 drivers/firmware/efi/tpm.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 1d3f5ca3eaaf..b9ae5c6f9b9c 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -75,11 +75,16 @@ int __init efi_tpm_eventlog_init(void)
goto out;
}
 
-   tbl_size = tpm2_calc_event_log_size((void *)efi.tpm_final_log
-   + sizeof(final_tbl->version)
-   + sizeof(final_tbl->nr_events),
-   final_tbl->nr_events,
-   log_tbl->log);
+   tbl_size = 0;
+   if (final_tbl->nr_events != 0) {
+   void *events = (void *)efi.tpm_final_log
+   + sizeof(final_tbl->version)
+   + sizeof(final_tbl->nr_events);
+
+   tbl_size = tpm2_calc_event_log_size(events,
+   final_tbl->nr_events,
+   log_tbl->log);
+   }
memblock_reserve((unsigned long)final_tbl,
 tbl_size + sizeof(*final_tbl));
early_memunmap(final_tbl, sizeof(*final_tbl));
-- 
2.20.1

Re: [PATCH v2] x86, efi: never relocate kernel below lowest acceptable address

[Jarkko Sakkinen]

On Fri, Sep 20, 2019 at 12:05:21AM +0800, Kairui Song wrote:
> Currently, kernel fails to boot on some HyperV VMs when using EFI.
> And it's a potential issue on all platforms.
> 
> It's caused a broken kernel relocation on EFI systems, when below three
> conditions are met:
> 
> 1. Kernel image is not loaded to the default address (LOAD_PHYSICAL_ADDR)
>by the loader.
> 2. There isn't enough room to contain the kernel, starting from the
>default load address (eg. something else occupied part the region).
> 3. In the memmap provided by EFI firmware, there is a memory region
>starts below LOAD_PHYSICAL_ADDR, and suitable for containing the
>kernel.
> 
> Efi stub will perform a kernel relocation when condition 1 is met. But
> due to condition 2, efi stub can't relocate kernel to the preferred
> address, so it fallback to query and alloc from EFI firmware for lowest
> usable memory region.
> 
> It's incorrect to use the lowest memory address. In later stage, kernel
> will assume LOAD_PHYSICAL_ADDR as the minimal acceptable relocate address,
> but efi stub will end up relocating kernel below it.
> 
> Then before the kernel decompressing. Kernel will do another relocation
> to address not lower than LOAD_PHYSICAL_ADDR, this time the relocate will
> over write the blockage at the default load address, which efi stub tried
> to avoid, and lead to unexpected behavior. Beside, the memory region it
> writes to is not allocated from EFI firmware, which is also wrong.
> 
> To fix it, just don't let efi stub relocate the kernel to any address
> lower than lowest acceptable address.
> 
> Signed-off-by: Kairui Song 

Acked-by:  

/Jarkko

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

[Jarkko Sakkinen]

On Tue, Aug 27, 2019 at 06:11:58PM -0400, Peter Jones wrote:
> On Tue, Aug 27, 2019 at 04:41:55PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Aug 27, 2019 at 02:03:44PM +0300, Jarkko Sakkinen wrote:
> > > > Jarkko, these two should probably go to 5.3 if possible - I
> > > > independently had a report of a system hitting this issue last week
> > > > (Intel apparently put a surprising amount of data in the event logs on
> > > > the NUCs).
> > > 
> > > OK, I can try to push them. I'll do PR today.
> > 
> > Ard, how do you wish these to be managed?
> > 
> > I'm asking this because:
> > 
> > 1. Neither patch was CC'd to linux-integrity.
> > 2. Neither patch has your tags ATM.
> 
> I think Ard's not back until September.  I can just to re-send them with
> the accumulated tags and Cc linux-integrity, if you think that would
> help?

I take the risk. If possible, add all the cumulated tags to those
patches...

/Jarkko

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

[Jarkko Sakkinen]

On Tue, Aug 27, 2019 at 02:03:44PM +0300, Jarkko Sakkinen wrote:
> > Jarkko, these two should probably go to 5.3 if possible - I
> > independently had a report of a system hitting this issue last week
> > (Intel apparently put a surprising amount of data in the event logs on
> > the NUCs).
> 
> OK, I can try to push them. I'll do PR today.

Ard, how do you wish these to be managed?

I'm asking this because:

1. Neither patch was CC'd to linux-integrity.
2. Neither patch has your tags ATM.

/Jarkko

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

[Jarkko Sakkinen]

On Mon, Aug 26, 2019 at 10:44:31AM -0700, Matthew Garrett wrote:
> On Mon, Aug 26, 2019 at 9:28 AM Jarkko Sakkinen
>  wrote:
> >
> > On Mon, Aug 26, 2019 at 11:30:27AM -0400, Peter Jones wrote:
> > > 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.
> > >
> > > Signed-off-by: Peter Jones 
> > > Tested-by: Lyude Paul 
> >
> > Reviewed-by: Jarkko Sakkinen 
> Acked-by: Matthew Garrett 
> 
> Jarkko, these two should probably go to 5.3 if possible - I
> independently had a report of a system hitting this issue last week
> (Intel apparently put a surprising amount of data in the event logs on
> the NUCs).

OK, I can try to push them. I'll do PR today.

/Jarkko

Re: [PATCH 2/2] efi+tpm: don't traverse an event log with no events

[Jarkko Sakkinen]

On Mon, Aug 26, 2019 at 11:30:28AM -0400, Peter Jones wrote:
> When there are no entries to put into the final event log, some machines
> will return the template they would have populated anyway.  In this case
> the nr_events field is 0, but the rest of the log is just garbage.
> 
> This patch stops us from trying to iterate the table with
> __calc_tpm2_event_size() when the number of events in the table is 0.
> 
> Signed-off-by: Peter Jones 
> Tested-by: Lyude Paul 
> ---
>  drivers/firmware/efi/tpm.c | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
> index 1d3f5ca3eaa..be51ed17c6e 100644
> --- a/drivers/firmware/efi/tpm.c
> +++ b/drivers/firmware/efi/tpm.c
> @@ -75,11 +75,15 @@ int __init efi_tpm_eventlog_init(void)
>   goto out;
>   }
>  
> - tbl_size = tpm2_calc_event_log_size((void *)efi.tpm_final_log
> - + sizeof(final_tbl->version)
> - + sizeof(final_tbl->nr_events),
> - final_tbl->nr_events,
> - log_tbl->log);
> + tbl_size = 0;
> + if (final_tbl->nr_events != 0) {
> + void *events = (void *)efi.tpm_final_log
> + + sizeof(final_tbl->version)
> + + sizeof(final_tbl->nr_events);
> + tbl_size = tpm2_calc_event_log_size(events,
> +     final_tbl->nr_events,
> + log_tbl->log);
> + }

Reviewed-by: Jarkko Sakkinen 

/Jarkko

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

[Jarkko Sakkinen]

On Mon, Aug 26, 2019 at 11:30:27AM -0400, Peter Jones wrote:
> 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.
> 
> Signed-off-by: Peter Jones 
> Tested-by: Lyude Paul 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH 5.3 regression fix] efi-stub: Fix get_efi_config_table on mixed-mode setups

[Jarkko Sakkinen]

On Wed, Aug 07, 2019 at 11:59:03PM +0200, Hans de Goede wrote:
> Fix get_efi_config_table using the wrong structs when booting a
> 64 bit kernel on 32 bit firmware.
> 
> Cc: Matthew Garrett 
> Cc: Ard Biesheuvel 
> Cc: Jarkko Sakkinen 
> Fixes: 82d736ac56d7 ("Abstract out support for locating an EFI config table")
> Signed-off-by: Hans de Goede 

Acked-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH] drivers: firmware: efi: fix gcc warning -Wint-conversion

[Jarkko Sakkinen]

On Thu, 2019-06-20 at 15:00 -0700, Matthew Garrett wrote:
> On Thu, Jun 20, 2019 at 2:37 PM Jarkko Sakkinen
>  wrote:
> > Right! OK, I squashed just the fix to the earlier patch. Master and
> > next are updated. Can you take a peek of [1] and see if it looks
> > legit given all the fuzz around these changes? Then I'm confident
> > enough to do the 5.3 PR.
> 
> All looks good to me. Thanks!

Thank you! I'll send the PR now.

/Jarkko

Re: [PATCH] drivers: firmware: efi: fix gcc warning -Wint-conversion

[Jarkko Sakkinen]

On Wed, Jun 19, 2019 at 03:48:23PM -0700, Matthew Garrett wrote:
> On Wed, Jun 19, 2019 at 2:55 AM Ard Biesheuvel
>  wrote:
> >
> > (+ Jarkko, tpmdd, Matthew)
> >
> > On Sat, 15 Jun 2019 at 06:02, Hariprasad Kelam
> >  wrote:
> > >
> > > This patch fixes below warning
> > >
> > > drivers/firmware/efi/tpm.c:78:38: warning: passing argument 1 of
> > > ‘tpm2_calc_event_log_size’ makes pointer from integer without a cast
> > > [-Wint-conversion]
> > >
> > > Signed-off-by: Hariprasad Kelam 
> >
> > I think we already have a fix queued for this, no?
> 
> It looks like I fixed this in "Don't duplicate events from the final
> event log in the TCG2 log" rather than a separate patch - I'm fine
> merging this, based on Jarkko's preferences.

Right! OK, I squashed just the fix to the earlier patch. Master and
next are updated. Can you take a peek of [1] and see if it looks
legit given all the fuzz around these changes? Then I'm confident
enough to do the 5.3 PR.

[1] git://git.infradead.org/users/jjs/linux-tpmdd.git

/Jarkko

Re: [PATCH V2 1/2] Abstract out support for locating an EFI config table

[Jarkko Sakkinen]

On Fri, Jun 07, 2019 at 01:51:46PM -0700, Matthew Garrett wrote:
> We want to grab a pointer to the TPM final events table, so abstract out
> the existing code for finding an FDT table and make it generic.
> 
> Signed-off-by: Matthew Garrett 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH V2 2/2] tpm: Don't duplicate events from the final event log in the TCG2 log

[Jarkko Sakkinen]

On Fri, Jun 07, 2019 at 01:51:47PM -0700, Matthew Garrett wrote:
> After the first call to GetEventLog() on UEFI systems using the TCG2
> crypto agile log format, any further log events (other than those
> triggered by ExitBootServices()) will be logged in both the main log and
> also in the Final Events Log. While the kernel only calls GetEventLog()
> immediately before ExitBootServices(), we can't control whether earlier
> parts of the boot process have done so. This will result in log entries
> that exist in both logs, and so the current approach of simply appending
> the Final Event Log to the main log will result in events being
> duplicated.
> 
> We can avoid this problem by looking at the size of the Final Event Log
> just before we call ExitBootServices() and exporting this to the main
> kernel. The kernel can then skip over all events that occured before
> ExitBootServices() and only append events that were not also logged to
> the main log.
> 
> Signed-off-by: Matthew Garrett 
> Reported-by: Joe Richey 
> Suggested-by: Joe Richey 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH V2 2/2] tpm: Don't duplicate events from the final event log in the TCG2 log

[Jarkko Sakkinen]

On Fri, Jun 07, 2019 at 11:11:21PM +0200, Ard Biesheuvel wrote:
> Acked-by: Ard Biesheuvel 

Ard, is it cool if I include these to my next TPM PR along with the
other Matthew's changes? Just sanity checking given that crossing
subsystems...

/Jarkko

Re: [PATCH V2 1/2] Abstract out support for locating an EFI config table

[Jarkko Sakkinen]

On Mon, Jun 10, 2019 at 10:46:35AM -0700, Matthew Garrett wrote:
> On Mon, Jun 10, 2019 at 9:58 AM Jarkko Sakkinen
>  wrote:
> >
> > On Fri, Jun 07, 2019 at 01:51:46PM -0700, Matthew Garrett wrote:
> > > We want to grab a pointer to the TPM final events table, so abstract out
> > > the existing code for finding an FDT table and make it generic.
> > >
> > > Signed-off-by: Matthew Garrett 
> >
> > Just to clarify are these extensions to what you did before and not
> > something that needs be squashed your commits pipelined for v5.3?
> 
> Correct - they handle a corner case. Ideally they'd hit 5.3 as well,
> but if you'd prefer I'm ok waiting.

Probably makes sense to have them in the PR.

/Jarkko

Re: [PATCH V2 1/2] Abstract out support for locating an EFI config table

[Jarkko Sakkinen]

On Fri, Jun 07, 2019 at 01:51:46PM -0700, Matthew Garrett wrote:
> We want to grab a pointer to the TPM final events table, so abstract out
> the existing code for finding an FDT table and make it generic.
> 
> Signed-off-by: Matthew Garrett 

Just to clarify are these extensions to what you did before and not
something that needs be squashed your commits pipelined for v5.3?

/Jarkko

Re: [PATCH] efi: Fix TPM code build failure on ARM

[Jarkko Sakkinen]

On Wed, Jun 05, 2019 at 11:11:40AM -0700, Matthew Garrett wrote:
> asm/early_ioremap.h needs to be #included before tpm_eventlog.h in order
> to ensure that early_memremap is available.
> 
> Signed-off-by: Matthew Garrett 

Thanks, squashed to "tpm: Reserve the TPM final events table".

/Jarkko

Re: [PATCH V2] tpm: Don't duplicate events from the final event log in the TCG2 log

[Jarkko Sakkinen]

On Wed, Jun 05, 2019 at 11:05:15AM -0700, Matthew Garrett wrote:
> After the first call to GetEventLog() on UEFI systems using the TCG2
> crypto agile log format, any further log events (other than those
> triggered by ExitBootServices()) will be logged in both the main log and
> also in the Final Events Log. While the kernel only calls GetEventLog()
> immediately before ExitBootServices(), we can't control whether earlier
> parts of the boot process have done so. This will result in log entries
> that exist in both logs, and so the current approach of simply appending
> the Final Event Log to the main log will result in events being
> duplicated.
> 
> We can avoid this problem by looking at the size of the Final Event Log
> just before we call ExitBootServices() and exporting this to the main
> kernel. The kernel can then skip over all events that occured before
> ExitBootServices() and only append events that were not also logged to
> the main log.
> 
> Signed-off-by: Matthew Garrett 
> Reported-by: Joe Richey 
> Suggested-by: Joe Richey 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH 1/1] tpm: Don't dereference event after it's unmapped in __calc_tpm2_event_size

[Jarkko Sakkinen]

On Wed, Jun 05, 2019 at 12:04:33AM +0100, Chris Coulson wrote:
> The pointer to the event header is dereferenced on each loop iteration in
> order to obtain the digest count, but when called from
> tpm2_calc_event_log_size, the event header is unmapped on the first
> iteration of the loop. This results in an invalid access for on subsequent
> loop iterations for log entries that have more than one digest.
> 
> Signed-off-by: Chris Coulson 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH 0/1] Fix crash in __calc_tpm2_event_size

[Jarkko Sakkinen]

On Wed, Jun 05, 2019 at 12:04:32AM +0100, Chris Coulson wrote:
> I've been testing the latest code in the linux-tpmdd branch and I'm
> experiencing a crash in __calc_tpm2_event_size when it's called to
> calculate the size of events in the final log. I hope I'm not stepping on
> anyone's toes, but this small change seems to fix it.

Oh no, thank you for reporting this.

/Jarkko

Re: [PATCH] tpm: Don't duplicate events from the final event log in the TCG2 log

[Jarkko Sakkinen]

On Tue, Jun 04, 2019 at 12:35:11PM -0700, Matthew Garrett wrote:
> After the first call to GetEventLog() on UEFI systems using the TCG2
> crypto agile log format, any further log events (other than those
> triggered by ExitBootServices()) will be logged in both the main log and
> also in the Final Events Log. While the kernel only calls GetEventLog()
> immediately before ExitBootServices(), we can't control whether earlier
> parts of the boot process have done so. This will result in log entries
> that exist in both logs, and so the current approach of simply appending
> the Final Event Log to the main log will result in events being
> duplicated.

Sounds flakky how UEFI firmaware works. Wonder why the ignition of the
final events log is bound to the invokation of GetEventLog() in the
first place.

> We can avoid this problem by looking at the size of the Final Event Log
> just before we call ExitBootServices() and exporting this to the main
> kernel. The kernel can then skip over all events that occured before
> ExitBootServices() and only append events that were not also logged to
> the main log.
> 
> Signed-off-by: Matthew Garrett 
> Reported-by: Joe Richey 
> Suggested-by: Joe Richey 

Rename final_events_early_size as final_events_preboot_size because it
is a bit more descriptive name. Other than that looks good to me.

/Jarkko

Re: [PATCH V7 0/4] Add support for crypto agile logs

[Jarkko Sakkinen]

On Sat, May 25, 2019 at 05:22:34AM +1000, James Morris wrote:
> On Fri, 24 May 2019, Jarkko Sakkinen wrote:
> 
> > I'm referring to these:
> > 
> > https://lore.kernel.org/linux-integrity/20190329115544.ga27...@linux.intel.com/
> > 
> > I got response from you that those were applied and there is another
> > response in that thread that they are being sent to Linus. That is why I
> > haven't done anything since. Most of them are critical fixes to v5.1
> > changes.
> 
> These are in Linus' tree.  
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a556810d8e06aa2da8bbe22da3d105eb5a0d0c7d
> 
> I initially queued them in the next-tpm branch, but forgot to drop them 
> from there after sending them to Linus as a v5.1 fix. Linus was not happy 
> to see them again in the v5.2 merge window.
> 
> Apologies for the confusion.

OK, just to confirm, my next PR will go straight to Linus?

/Jarkko

Re: [PATCH V7 0/4] Add support for crypto agile logs

[Jarkko Sakkinen]

On Fri, May 24, 2019 at 02:54:20AM +1000, James Morris wrote:
> On Thu, 23 May 2019, Jarkko Sakkinen wrote:
> 
> > On Thu, May 23, 2019 at 03:14:49PM +0300, Jarkko Sakkinen wrote:
> > > On Mon, May 20, 2019 at 01:54:57PM -0700, Matthew Garrett wrote:
> > > > Identical to previous version except without the KSAN workaround - Ard
> > > > has a better solution for that.
> > > 
> > > 
> > > Reviewed-by: Jarkko Sakkinen 
> > > Tested-by: Jarkko Sakkinen 
> > 
> > Only applied to my master at this point becaues the patches from
> > my earlier PRs are not yet mirrored to security/next-general.
> 
> Which PRs are these?
> 
> btw, Linus wants security subsystem maintainers to submit PRs directly to 
> him from now on.
> 
> I'll only be carrying patches for the core LSM and new security mechanisms 
> before they're merged and have a maintainer assigned.

I'm referring to these:

https://lore.kernel.org/linux-integrity/20190329115544.ga27...@linux.intel.com/

I got response from you that those were applied and there is another
response in that thread that they are being sent to Linus. That is why I
haven't done anything since. Most of them are critical fixes to v5.1
changes.

Should I now take the action to send a PR to Linus and tag them to
stable?

/Jarkko

Re: [PATCH V7 0/4] Add support for crypto agile logs

[Jarkko Sakkinen]

On Thu, May 23, 2019 at 03:14:49PM +0300, Jarkko Sakkinen wrote:
> On Mon, May 20, 2019 at 01:54:57PM -0700, Matthew Garrett wrote:
> > Identical to previous version except without the KSAN workaround - Ard
> > has a better solution for that.
> 
> 
> Reviewed-by: Jarkko Sakkinen 
> Tested-by: Jarkko Sakkinen 

Only applied to my master at this point becaues the patches from
my earlier PRs are not yet mirrored to security/next-general.

/Jarkko

Re: [PATCH V7 0/4] Add support for crypto agile logs

[Jarkko Sakkinen]

On Mon, May 20, 2019 at 01:54:57PM -0700, Matthew Garrett wrote:
> Identical to previous version except without the KSAN workaround - Ard
> has a better solution for that.


Reviewed-by: Jarkko Sakkinen 
Tested-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH V7 0/4] Add support for crypto agile logs

[Jarkko Sakkinen]

On Mon, May 20, 2019 at 01:54:57PM -0700, Matthew Garrett wrote:
> Identical to previous version except without the KSAN workaround - Ard
> has a better solution for that.

I'll check in detail through tomorrow but probably will get merged
now that we have Ard's ack's (thanks Ard for all the trouble!) :-)

/Jarkko

Re: [PATCH] x86/boot: provide KASAN compatible aliases for string routines

[Jarkko Sakkinen]

On Sat, May 18, 2019 at 06:11:13PM +0200, Ard Biesheuvel wrote:
> The KASAN subsystem wraps calls to memcpy(), memset() and memmove()
> to sanitize the arguments before invoking the actual routines, which
> have been renamed to __memcpy(), __memset() and __memmove(),
> respectively. When CONFIG_KASAN is enabled for the kernel build but
> KASAN code generation is disabled for the compilation unit (which is
> needed for things like the EFI stub or the decompressor), the string
> routines are just #define'd to their __ prefixed names so that they
> are simply invoked directly.
> 
> This does however rely on those __ prefixed names to exist in the
> symbol namespace, which is not currently the case for the x86
> decompressor, which may lead to errors like
> 
>   drivers/firmware/efi/libstub/tpm.o: In function 
> `efi_retrieve_tpm2_eventlog':
>   tpm.c:(.text+0x2a8): undefined reference to `__memcpy'
> 
> So let's expose the __ prefixed symbols in the decompressor when
> KASAN is enabled.
> 
> Cc: Andrey Konovalov 
> Cc: Matthew Garrett 
> Signed-off-by: Ard Biesheuvel 

Acked-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Fri, May 03, 2019 at 08:02:18AM +0200, Ingo Molnar wrote:
> 
> * Matthew Garrett  wrote:
> 
> > On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
> >  wrote:
> > >
> > > (+ Ingo)
> > >
> > > On Tue, 30 Apr 2019 at 21:52, Matthew Garrett  wrote:
> > > >
> > > > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek  
> > > > wrote:
> > > > >
> > > > > I may be a little late with this comment, but I've just tested these
> > > > > patches on aarch64 platform (from the top of jjs/master) and got
> > > > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > > > mail). I think there's problem with below call to
> > > > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > > > passed as (void *) and never remapped:
> > > >
> > > > Yes, it looks like this is just broken. Can you try with the attached 
> > > > patch?
> > >
> > > I'm a bit uncomfortable with EFI code that is obviously broken and
> > > untested being queued for the next merge window in another tree.
> > 
> > The patchset was Cc:ed to linux-efi@. Is there anything else I should
> > have done to ensure you picked it up rather than Jarkko?
> 
> That's not the workflow rule the Linux kernel is using, if Cc:-ing a 
> patchset was the only condition for upstream inclusion then we'd have a 
> *LOT* of crap in the Linux kernel.
> 
> Just applying those EFI changes without even as much as an Acked-by from 
> the EFI maintainers is a *totally* unacceptable workflow.
> 
> Please revert/rebase and re-try this on the proper submission channels.
> 
> Meanwhile the broken code is NAK-ed by me:
> 
>Nacked-by: Ingo Molnar 

There must be some kind of misconception here. None of the changes have
been submitted so far. They are only in my master branch. They briefly
went to linux-next through my next branch but as soon as issues were
reported I wiped them off from there (which happened like 2-3 weeks
ago). They haven't been part off any of my PR's.

There is nothing to revert.

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Thu, May 02, 2019 at 11:03:08AM -0700, Matthew Garrett wrote:
> On Thu, May 2, 2019 at 1:32 AM Jarkko Sakkinen
>  wrote:
> >
> > On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Not late. This is not part of any PR yet. Thank you for the
> > feedback!
> >
> > Matthew, can you send an updated version of the whole patch set
> > with fixes to this issue and also reordering of the includes?
> 
> Yes, I'll resend and let's do this again for 5.3.

Agreed, better not rush but get it right once.

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Thu, May 02, 2019 at 09:14:49AM +0200, Ard Biesheuvel wrote:
> (+ Ingo)
> 
> On Tue, 30 Apr 2019 at 21:52, Matthew Garrett  wrote:
> >
> > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek  
> > wrote:
> > >
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Yes, it looks like this is just broken. Can you try with the attached patch?
> 
> I'm a bit uncomfortable with EFI code that is obviously broken and
> untested being queued for the next merge window in another tree.
> 
> What is currently queued there? Can we revert this change for the time
> being, and resubmit it via the EFI tree for v5.3?

Nothing ATM. The broken code is in my master branch ATM. Nothing
in my next branch nor have I included anything to my PRs. Should
be nothing to worry about in that sense :-)

/Jarkko

Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> I may be a little late with this comment, but I've just tested these
> patches on aarch64 platform (from the top of jjs/master) and got
> kernel panic ("Unable to handle kernel read", full log at the end of
> mail). I think there's problem with below call to
> tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> passed as (void *) and never remapped:

Not late. This is not part of any PR yet. Thank you for the
feedback!

Matthew, can you send an updated version of the whole patch set
with fixes to this issue and also reordering of the includes?

/Jarkko

Re: [PATCH] efi: Include tpm_eventlog.h after asm/efi.h to avoid memcpy breakage

[Jarkko Sakkinen]

On Wed, Apr 03, 2019 at 12:32:37PM -0700, Matthew Garrett wrote:
> 769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch)
> disables the KASAN version of certain memory calls in the EFI boot stub.
> tpm_eventlog.h references memcpy, so must be included after asm/efi.h in
> order to allow 769a8089c1fd2 to take effect on its declarations.
> 
> Signed-off-by: Matthew Garrett 

Applied.

/Jarkko

Re: [PATCH] TCG2 log support build fixes for non-x86_64

[Jarkko Sakkinen]

On Tue, Apr 02, 2019 at 02:55:54PM -0700, Matthew Garrett wrote:
> Couple of patches to fix ktest reported issues with the crypto-agile log
> format support.

Applied and squashed. Should be soon in linux-next.

/Jarkko

Re: [PATCH] efi: Include tpm_eventlog.h after asm/efi.h to avoid memcpy breakage

[Jarkko Sakkinen]

On Wed, Apr 03, 2019 at 12:32:37PM -0700, Matthew Garrett wrote:
> 769a8089c1fd2 (x86, efi, kasan: #undef memset/memcpy/memmove per arch)
> disables the KASAN version of certain memory calls in the EFI boot stub.
> tpm_eventlog.h references memcpy, so must be included after asm/efi.h in
> order to allow 769a8089c1fd2 to take effect on its declarations.
> 
> Signed-off-by: Matthew Garrett 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH] TCG2 log support build fixes for non-x86_64

[Jarkko Sakkinen]

On Tue, Apr 02, 2019 at 02:55:54PM -0700, Matthew Garrett wrote:
> Couple of patches to fix ktest reported issues with the crypto-agile log
> format support.

I guess I squash these to your earlier commits?

/Jarkko

Re: [PATCH 2/2] tpm: Fix builds on platforms that lack early_memremap()

[Jarkko Sakkinen]

On Tue, Apr 02, 2019 at 02:55:56PM -0700, Matthew Garrett wrote:
> On EFI systems, __calc_tpm2_event_size() needs to be able to map tables
> at early boot time in order to extract information from them.
> Unfortunately this interacts badly with other architectures that don't
> provide the early_memremap() interface but which may still have other
> mechanisms for obtaining crypto-agile logs. Abstract this away so we
> can avoid the need for two implementations while still avoiding breakage
> on architectures that don't require remapping of the table.
> 
> Signed-off-by: Matthew Garrett 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code

[Jarkko Sakkinen]

On Tue, Apr 02, 2019 at 02:55:55PM -0700, Matthew Garrett wrote:
> 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373 introduced a cast from
> efi_physical_address_t to (void *), which are different sizes on 32-bit.
> Fix that. Caught by the 0-day test bot.
> 
> Signed-off-by: Matthew Garrett 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Tue, Apr 02, 2019 at 10:15:39AM -0700, Matthew Garrett wrote:
> On Tue, Apr 2, 2019 at 6:07 AM Jarkko Sakkinen
>  wrote:
> > Reviewed-by: Jarkko Sakkinen 
> > Tested-by: Jarkko Sakkinen 
> >
> > I'll apply all patches soonish and include them to the next PR.
> 
> Thanks! Looks like I need some fixes to deal with non-x86
> architectures, I'll get on that today.

Great thanks. I'll check them tomorrow (Thu).

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Mon, Apr 01, 2019 at 08:32:26PM -0700, Matthew Garrett wrote:
> On Mon, Apr 1, 2019 at 4:52 PM Jarkko Sakkinen
>  wrote:
> >
> > On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > > Identical to V4, but based on tpmdd-next
> >
> > OK, so on my GLK NUC I get valid final log and invalid event log
> > after adding some extra klogs.
> >
> > I.e.
> >
> > -   if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> > +   if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
> 
> Just to make sure - are you booting via the EFI boot stub? We need to
> obtain the boot log before ExitBootServices() is called, so if you're
> booting directly into the 32-bit entry point (eg, by using the "linux"
> command in grub) then you won't get a log.

... and I was wondering why it used to work when I tested the first
flush of patches. Ugh, sorry. The only excuse is too much multitasking
lately.

Anyway:

Reviewed-by: Jarkko Sakkinen 
Tested-by: Jarkko Sakkinen 

I'll apply all patches soonish and include them to the next PR.

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> Identical to V4, but based on tpmdd-next

OK, so on my GLK NUC I get valid final log and invalid event log
after adding some extra klogs.

I.e.

-   if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
+   if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
+   pr_err("No event log\n");
return -ENODEV;
+   }

will result

[2.654392] No event log 

but still

[0.00] efi:  ACPI 2.0=0x69ca2000  ACPI=0x69ca2000  
TPMFinalLog=0x69ce4000  SMBIOS=0x69f63000  SMBIOS 3.0=0x69f62000  
ESRT=0x69f3e818  MEMATTR=0x63475118

Tomas, I wonder if you are able to get the log out with some machine?

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Thu, Mar 14, 2019 at 02:04:02PM -0700, Matthew Garrett wrote:
> On Thu, Mar 14, 2019 at 2:35 AM Jarkko Sakkinen
>  wrote:
> >
> > On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > > Identical to V4, but based on tpmdd-next
> >
> > This is not found /sys/kernel/security/tpm0/ascii_bios_measurements
> 
> That's expected - the existing kernel TCG2 log code doesn't expose
> ascii_bios_measurements, only binary_bios_measurements. This patchset
> doesn't change that.

Oops, I meant to point out that the binary_bios_measurents is not found
(i.e. ascii was a typo). The whole tpm0 directory is missing.

I'll try to pinpoint next week where things might go wrong on my system.

/Jarkko

Re: Add support for TCG2 log format on UEFI systems

[Jarkko Sakkinen]

On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> Identical to V4, but based on tpmdd-next

This is not found /sys/kernel/security/tpm0/ascii_bios_measurements

But still

[0.00] efi:  ACPI 2.0=0x69ca2000  ACPI=0x69ca2000  
TPMFinalLog=0x69ce4000  SMBIOS=0x69f63000  SMBIOS 3.0=0x69f62000  
ESRT=0x69f3e818  MEMATTR=0x63448018

Tried this with too machines now.

I wonder if anyone else has had success...

/Jarkko

Re: [PATCH V4 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Wed, Feb 27, 2019 at 11:57:09AM -0800, Matthew Garrett wrote:
> On Wed, Feb 27, 2019 at 6:03 AM Jarkko Sakkinen
>  wrote:
> > My guess is that your patches are based a later 5.0-rcX. Unfortunately I
> > cannot update my master at this point because my 5.1 PR was taken to
> > security tree and rebasing would change the commit IDs of 5.1 content
> > because security/next-general does not yet contain those patches.
> 
> Bother. I'll rebase against your tree.

Thanks!

/Jarkko

Re: [PATCH V4 2/4] tpm: Reserve the TPM final events table

[Jarkko Sakkinen]

On Fri, Feb 22, 2019 at 12:26:04PM -0800, Matthew Garrett wrote:
> From: Matthew Garrett 
> 
> UEFI systems provide a boot services protocol for obtaining the TPM
> event log, but this is unusable after ExitBootServices() is called.
> Unfortunately ExitBootServices() itself triggers additional TPM events
> that then can't be obtained using this protocol. The platform provides a
> mechanism for the OS to obtain these events by recording them to a
> separate UEFI configuration table which the OS can then map.
> 
> Unfortunately this table isn't self describing in terms of providing its
> length, so we need to parse the events inside it to figure out how long
> it is. Since the table isn't mapped at this point, we need to extend the
> length calculation function to be able to map the event as it goes
> along.
> 
> Signed-off-by: Matthew Garrett 

Getting:

Applying: tpm: Reserve the TPM final events table
error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
error: could not build fake ancestor

My tree:

git://git.infradead.org/users/jjs/linux-tpmdd.git

My guess is that your patches are based a later 5.0-rcX. Unfortunately I
cannot update my master at this point because my 5.1 PR was taken to
security tree and rebasing would change the commit IDs of 5.1 content
because security/next-general does not yet contain those patches.

/Jarkko

Re: [PATCH V4 0/4] Add support for TCG2 event logs on EFI systems

[Jarkko Sakkinen]

On Fri, 2019-02-22 at 12:26 -0800, Matthew Garrett wrote:
> This patchset adds support for obtaining the TCG2 format event log on
> EFI systems, along with support for copying up the final event log to
> capture events that occur after the primary log is obtained. V4 is
> identical to previous versions, except for tpm_read_log_efi() in patch 3
> being reworked to reduce nesting.

Looks good.

I'll re-try testing latter part of this weke.

/Jarkko

Re: linux-next: Tree for Feb 20

[Jarkko Sakkinen]

On Wed, Feb 20, 2019 at 11:52:52AM +0200, Jarkko Sakkinen wrote:
> On Wed, Feb 20, 2019 at 05:11:15PM +0800, Zhangshaokun wrote:
> > There is a compiler failure on arm64 platform, as follow:
> > 
> >   AS  arch/arm64/kvm/hyp.o
> >   CC  kernel/trace/ring_buffer.o
> > In file included from security/integrity/ima/ima_fs.c:30:0:
> > security/integrity/ima/ima.h:176:7: error: redeclaration of enumerator 
> > ‘NONE’
> >   hook(NONE)   \
> >^
> > security/integrity/ima/ima.h:188:34: note: in definition of macro 
> > ‘__ima_hook_enumify’
> >  #define __ima_hook_enumify(ENUM) ENUM,
> >   ^
> > security/integrity/ima/ima.h:191:2: note: in expansion of macro 
> > ‘__ima_hooks’
> >   __ima_hooks(__ima_hook_enumify)
> >   ^
> > In file included from ./arch/arm64/include/asm/acpi.h:15:0,
> >  from ./include/acpi/acpi_io.h:7,
> >  from ./include/linux/acpi.h:47,
> >  from ./include/linux/tpm.h:26,
> >  from security/integrity/ima/ima.h:25,
> >  from security/integrity/ima/ima_fs.c:30:
> > ./include/linux/efi.h:1716:2: note: previous definition of ‘NONE’ was here
> >   NONE,
> >   ^
> > scripts/Makefile.build:276: recipe for target 
> > 'security/integrity/ima/ima_fs.o' failed
> > make[3]: *** [security/integrity/ima/ima_fs.o] Error 1
> > 
> > I dug it and it is the commit 901615cb916d ("tpm: move tpm_chip definition 
> > to include/linux/tpm.h")
> 
> This results from a new include in tpm.h:
> 
>   #include 
> 
> Must be fixed either in include/linux/efi.h or security/integrity/ima.h as
> those files have a name collision. Makes me wonder why neither has taken
> care of prefixing the constants properly.

Preferably both subsystems should be fixed with proper 'EFI_' and 'IMA_'
prefixes. Defining a constant named as NONE in a non-generic subsystem
(e.g. not part of the core data structures of Linux) and especially
exporting it to include/linux is not too well considered act.

/Jarkko

Re: [PATCH 4/5] efi/libstub/tpm: Retrieve TPM event log in 2.0 format

[Jarkko Sakkinen]

On Mon, Feb 11, 2019 at 03:30:51PM +0100, b...@semihalf.com wrote:
> From: Bartosz Szczepanek 
> 
> Currently, the only way to get TPM 2.0 event log from firmware is to use
> device tree. Introduce efi_retrieve_tpm2_eventlog_2 function to enable
> retrieving it from EFI structures.
> 
> Include lib/tpm.c into EFI stub to calculate event sizes using helper
> function.
> 
> Signed-off-by: Bartosz Szczepanek 

Collides with Matthew's changes. I want to land those change first
because they are almost production ready.

Maybe you should consider reviewing those changes to make sure that
they make sense to you so that you can build these on top of after
these have landed.

There is not rush as I already made my v5.1 PR. These will be released
earliest in v5.2.

/Jarkko

Re: [PATCH 3/5] tpm: Use library version of calc_tpm2_event_size in sysfs code

[Jarkko Sakkinen]

On Mon, Feb 11, 2019 at 03:30:50PM +0100, b...@semihalf.com wrote:
> From: Bartosz Szczepanek 
> 
> Expect negative values from calc_tpm2_event_size as error codes.
> Pass efispecid instead of event header to calc_tpm2_event_size.
> 
> Also, include tpm library in the build.
> 
> Signed-off-by: Bartosz Szczepanek 

Event log uses securityfs, not sysfs.

/Jarkko

Re: [PATCH 2/5] tpm: Change calc_tpm2_event_size signature

[Jarkko Sakkinen]

On Mon, Feb 11, 2019 at 03:30:49PM +0100, b...@semihalf.com wrote:
> From: Bartosz Szczepanek 
> 
> Pass tcg_efi_specid_event as an argument instead of tcg_pcr_event, as the
> former is what is actually needed to compute event size. tcg_pcr_event
> structure describes TPM event log header (even though its name), from where
> efispecid can be extracted -- it seems cleaner and less misleading to do it
> out of calc_tpm2_event_size function.
> 
> Also, use ssize_t instead of int for event log size.
> 
> Signed-off-by: Bartosz Szczepanek 

Unreviewable without call sites. NAK.

/Jarkko

Re: [PATCH 1/5] tpm: Copy calc_tpm2_event_size() to TPM library

[Jarkko Sakkinen]

On Wed, Feb 13, 2019 at 01:14:43PM +0200, Jarkko Sakkinen wrote:
> On Mon, Feb 11, 2019 at 03:30:48PM +0100, b...@semihalf.com wrote:
> > From: Bartosz Szczepanek 
> > 
> > Function to calculate event size in TPM 2.0 log will also be needed in EFI
> > stub. Separate it to library to make it accessible out of TPM character
> > driver.
> > 
> > It will be removed from tpm2.c in subsequent commit.
> > 
> > Signed-off-by: Bartosz Szczepanek 
> 
> Collides with Matthew's patch set, which has priority over this because
> it was sent earlier.
> 
> Matthew, what you think of this? Maybe could replace 1/4 with this in
> your patch set? Somehow feels a bit cleaner approach.

 This commit is in any case broken. It leaves two versions of the
 function hanging to the code base.

 /Jarkko

Re: [PATCH 1/5] tpm: Copy calc_tpm2_event_size() to TPM library

[Jarkko Sakkinen]

On Mon, Feb 11, 2019 at 03:30:48PM +0100, b...@semihalf.com wrote:
> From: Bartosz Szczepanek 
> 
> Function to calculate event size in TPM 2.0 log will also be needed in EFI
> stub. Separate it to library to make it accessible out of TPM character
> driver.
> 
> It will be removed from tpm2.c in subsequent commit.
> 
> Signed-off-by: Bartosz Szczepanek 

Collides with Matthew's patch set, which has priority over this because
it was sent earlier.

Matthew, what you think of this? Maybe could replace 1/4 with this in
your patch set? Somehow feels a bit cleaner approach.

> ---
>  lib/tpm.c | 80 
> +++
>  1 file changed, 80 insertions(+)
>  create mode 100644 lib/tpm.c
> 
> diff --git a/lib/tpm.c b/lib/tpm.c
> new file mode 100644
> index ..aaeeafe52426
> --- /dev/null
> +++ b/lib/tpm.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2016 IBM Corporation

Do we want copyright statements to new files? I'm sure that this code
would have more copyright holders than just IBM (eg VMWare). Git
documents this anyway. This is something that will be left unmaintained.

> + * Parts of this file based on earlier work by:
> + *  Nayna Jain 
> + *  Petr Vandrovec 

Please remove these three lines. These type of lists are just inaccurate
presentaion of the commit log.

> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.

You already have SPDX identifier. Unncessary repeat.

> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * calc_tpm2_event_size() - calculate the event size, where event
> + * is an entry in the TPM 2.0 event log. The event is of type Crypto
> + * Agile Log Entry Format as defined in TCG EFI Protocol Specification
> + * Family "2.0".
> +
> + * @event: event whose size is to be calculated.
> + * @event_header: the first event in the event log.
> + *
> + * Returns size of the event. If it is an invalid event, returns 0.
> + */
> +int calc_tpm2_event_size(struct tcg_pcr_event2 *event,
> +  struct tcg_pcr_event *event_header)
> +{
> + struct tcg_efi_specid_event *efispecid;
> + struct tcg_event_field *event_field;
> + void *marker;
> + void *marker_start;
> + u32 halg_size;
> + size_t size;
> + u16 halg;
> + int i;
> + int j;
> +
> + marker = event;
> + marker_start = marker;
> + marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
> + + sizeof(event->count);
> +
> + efispecid = (struct tcg_efi_specid_event *)event_header->event;
> +
> + /* Check if event is malformed. */
> + if (event->count > efispecid->num_algs)
> + return 0;
> +
> + for (i = 0; i < event->count; i++) {
> + halg_size = sizeof(event->digests[i].alg_id);
> + memcpy(, marker, halg_size);
> + marker = marker + halg_size;
> + for (j = 0; j < efispecid->num_algs; j++) {
> + if (halg == efispecid->digest_sizes[j].alg_id) {
> + marker +=
> + efispecid->digest_sizes[j].digest_size;
> + break;
> + }
> + }
> + /* Algorithm without known length. Such event is unparseable. */
> + if (j == efispecid->num_algs)
> + return 0;
> + }
> +
> + event_field = (struct tcg_event_field *)marker;
> + marker = marker + sizeof(event_field->event_size)
> + + event_field->event_size;
> + size = marker - marker_start;
> +
> + if ((event->event_type == 0) && (event_field->event_size == 0))
> + return 0;
> +
> + return size;
> +}
> +EXPORT_SYMBOL(calc_tpm2_event_size);
> -- 
> 2.14.4
> 

Re: [PATCH -next] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static

[Jarkko Sakkinen]

On Tue, Apr 24, 2018 at 08:39:09AM +0200, Ard Biesheuvel wrote:
> On 23 April 2018 at 21:38, Jarkko Sakkinen
> <jarkko.sakki...@linux.intel.com> wrote:
> > On Mon, Apr 16, 2018 at 01:05:24PM +0200, Ard Biesheuvel wrote:
> >> On 22 March 2018 at 15:09, Jarkko Sakkinen
> >> <jarkko.sakki...@linux.intel.com> wrote:
> >> > On Thu, 2018-03-22 at 16:06 +0200, Jarkko Sakkinen wrote:
> >> >> On Tue, 2018-03-20 at 14:17 +, Wei Yongjun wrote:
> >> >> > Fixes the following sparse warning:
> >> >> >
> >> >> > drivers/firmware/efi/libstub/tpm.c:62:6: warning:
> >> >> >  symbol 'efi_retrieve_tpm2_eventlog_1_2' was not declared. Should it 
> >> >> > be
> >> >> > static?
> >> >> >
> >> >> > Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
> >> >> > ---
> >> >> >  drivers/firmware/efi/libstub/tpm.c | 2 +-
> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/drivers/firmware/efi/libstub/tpm.c
> >> >> > b/drivers/firmware/efi/libstub/tpm.c
> >> >> > index 9d08cea..caa37a6 100644
> >> >> > --- a/drivers/firmware/efi/libstub/tpm.c
> >> >> > +++ b/drivers/firmware/efi/libstub/tpm.c
> >> >> > @@ -59,7 +59,7 @@ void 
> >> >> > efi_enable_reset_attack_mitigation(efi_system_table_t
> >> >> > *sys_table_arg)
> >> >> >
> >> >> >  #endif
> >> >> >
> >> >> > -void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t 
> >> >> > *sys_table_arg)
> >> >> > +static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t
> >> >> > *sys_table_arg)
> >> >> >  {
> >> >> > efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
> >> >> > efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
> >> >> >
> >> >>
> >> >> Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>
> >> >
> >> > Applied.
> >> >
> >>
> >> Applied to?
> >
> > Mistake, efi maintainers should pick this one up. Sorry (my reviewed tag
> > still holds).
> >
> 
> Thanks. I have queued this up in efi/next

And sorry for the confusion! Had a lot of traffic that point..

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static

[Jarkko Sakkinen]

On Mon, Apr 16, 2018 at 01:05:24PM +0200, Ard Biesheuvel wrote:
> On 22 March 2018 at 15:09, Jarkko Sakkinen
> <jarkko.sakki...@linux.intel.com> wrote:
> > On Thu, 2018-03-22 at 16:06 +0200, Jarkko Sakkinen wrote:
> >> On Tue, 2018-03-20 at 14:17 +, Wei Yongjun wrote:
> >> > Fixes the following sparse warning:
> >> >
> >> > drivers/firmware/efi/libstub/tpm.c:62:6: warning:
> >> >  symbol 'efi_retrieve_tpm2_eventlog_1_2' was not declared. Should it be
> >> > static?
> >> >
> >> > Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
> >> > ---
> >> >  drivers/firmware/efi/libstub/tpm.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/firmware/efi/libstub/tpm.c
> >> > b/drivers/firmware/efi/libstub/tpm.c
> >> > index 9d08cea..caa37a6 100644
> >> > --- a/drivers/firmware/efi/libstub/tpm.c
> >> > +++ b/drivers/firmware/efi/libstub/tpm.c
> >> > @@ -59,7 +59,7 @@ void 
> >> > efi_enable_reset_attack_mitigation(efi_system_table_t
> >> > *sys_table_arg)
> >> >
> >> >  #endif
> >> >
> >> > -void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
> >> > +static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t
> >> > *sys_table_arg)
> >> >  {
> >> > efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
> >> > efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
> >> >
> >>
> >> Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>
> >
> > Applied.
> >
> 
> Applied to?

Mistake, efi maintainers should pick this one up. Sorry (my reviewed tag
still holds).

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static

[Jarkko Sakkinen]

On Thu, 2018-03-22 at 16:06 +0200, Jarkko Sakkinen wrote:
> On Tue, 2018-03-20 at 14:17 +, Wei Yongjun wrote:
> > Fixes the following sparse warning:
> > 
> > drivers/firmware/efi/libstub/tpm.c:62:6: warning:
> >  symbol 'efi_retrieve_tpm2_eventlog_1_2' was not declared. Should it be
> > static?
> > 
> > Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
> > ---
> >  drivers/firmware/efi/libstub/tpm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/firmware/efi/libstub/tpm.c
> > b/drivers/firmware/efi/libstub/tpm.c
> > index 9d08cea..caa37a6 100644
> > --- a/drivers/firmware/efi/libstub/tpm.c
> > +++ b/drivers/firmware/efi/libstub/tpm.c
> > @@ -59,7 +59,7 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t
> > *sys_table_arg)
> >  
> >  #endif
> >  
> > -void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
> > +static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t
> > *sys_table_arg)
> >  {
> > efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
> > efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
> > 
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>

Applied.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static

[Jarkko Sakkinen]

On Tue, 2018-03-20 at 14:17 +, Wei Yongjun wrote:
> Fixes the following sparse warning:
> 
> drivers/firmware/efi/libstub/tpm.c:62:6: warning:
>  symbol 'efi_retrieve_tpm2_eventlog_1_2' was not declared. Should it be
> static?
> 
> Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
> ---
>  drivers/firmware/efi/libstub/tpm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/efi/libstub/tpm.c
> b/drivers/firmware/efi/libstub/tpm.c
> index 9d08cea..caa37a6 100644
> --- a/drivers/firmware/efi/libstub/tpm.c
> +++ b/drivers/firmware/efi/libstub/tpm.c
> @@ -59,7 +59,7 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t
> *sys_table_arg)
>  
>  #endif
>  
> -void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
> +static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
>  {
>   efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
>   efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
> 

Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] efi/libstub: tpm: zero initialize pointer variables for mixed mode

[Jarkko Sakkinen]

On Tue, Mar 13, 2018 at 02:09:21PM +, Ard Biesheuvel wrote:
> As reported by Jeremy, running the new TPM libstub code in mixed mode
> (i.e., 64-bit kernel on 32-bit UEFI) results in hangs when invoking
> the TCG2 protocol, or when accessing the log_tbl pool allocation.
> 
> The reason turns out to be that in both cases, the 64-bit pointer
> variables are not fully initialized by the 32-bit EFI code, and so
> we should take care to zero initialize these variables beforehand,
> or we'll end up dereferencing bogus pointers.
> 
> Reported-by: Jeremy Cline <jer...@jcline.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression from efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Mon, Mar 12, 2018 at 11:41:25AM +0100, Paul Menzel wrote:
> Dear Jarkko,
> 
> 
> On 03/12/18 11:17, Jarkko Sakkinen wrote:
> > On Sat, 2018-03-10 at 10:45 +, Thiebaud Weksteen wrote:
> > > On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline <jer...@jcline.org> wrote:
> > > > and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);"
> > > 
> > > Thanks. Well, it looks like the memory that is supposedly allocated is not
> > > usable. I'm thinking this is a firmware bug.
> > > Ard, would you agree on this assumption? Thoughts on how to proceed?
> > 
> > Check if the BIOS is up to date?
> 
> How would that help? The no regression policy demands, that Linux continues
> working on systems, where it worked before. So upgrading the firmware is no
> solution, is it? Until a solution is found, the commits should be reverted.

Nope. You are right.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression from efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Sat, 2018-03-10 at 10:45 +, Thiebaud Weksteen wrote:
> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline  wrote:
> > and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);"
> 
> Thanks. Well, it looks like the memory that is supposedly allocated is not
> usable. I'm thinking this is a firmware bug.
> Ard, would you agree on this assumption? Thoughts on how to proceed?

Check if the BIOS is up to date?

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:19PM +0200, Thiebaud Weksteen wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
> Area (LAML and LASA). These logs are useful to understand and rebuild the
> final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
> The recommended method is now to call the GetEventLog EFI protocol before
> ExitBootServices.
> 
> Implement this method within the EFI stub and create copy of the logs for the
> TPM device. This will create 
> /sys/kernel/security/tpm0/binary_bios_measurements
> for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).
> 
> ---
> 
> Patchset Changelog:
> 
> Version 2:
> - Move tpm_eventlog.h to top include directory, add commit for this.
> - Use EFI_LOADER_DATA to store the configuration table
> - Whitespace and new lines fixes
> 
> 
> Thiebaud Weksteen (3):
>   tpm: move tpm_eventlog.h outside of drivers folder
>   efi: call get_event_log before ExitBootServices
>   tpm: parse TPM event logs based on EFI table
> 
>  arch/x86/boot/compressed/eboot.c   |  1 +
>  drivers/char/tpm/Makefile  |  2 +-
>  drivers/char/tpm/tpm-chip.c|  3 +-
>  drivers/char/tpm/tpm-interface.c   |  2 +-
>  drivers/char/tpm/tpm.h | 35 --
>  drivers/char/tpm/tpm1_eventlog.c   | 17 +++--
>  drivers/char/tpm/tpm2_eventlog.c   |  2 +-
>  drivers/char/tpm/tpm_acpi.c|  2 +-
>  drivers/char/tpm/tpm_efi.c | 66 ++
>  drivers/char/tpm/tpm_of.c  |  2 +-
>  drivers/firmware/efi/Makefile  |  2 +-
>  drivers/firmware/efi/efi.c |  4 ++
>  drivers/firmware/efi/libstub/Makefile  |  3 +-
>  drivers/firmware/efi/libstub/tpm.c | 81 
> ++
>  drivers/firmware/efi/tpm.c | 39 +++
>  include/linux/efi.h| 50 +
>  {drivers/char/tpm => include/linux}/tpm_eventlog.h | 32 ++---
>  17 files changed, 301 insertions(+), 42 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm_efi.c
>  create mode 100644 drivers/firmware/efi/tpm.c
>  rename {drivers/char/tpm => include/linux}/tpm_eventlog.h (77%)
> 
> -- 
> 2.14.1.581.gf28d330327-goog
> 

Tested-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.cpm>

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Tue, Oct 17, 2017 at 10:00:15AM +0200, Thiebaud Weksteen wrote:
> This patch was mainly developed and tested on Kabylake with PTT as well.
> 
> It could be a few things. Are you booting with the EFI stub? Is the
> TPM enabled within the BIOS? Does tpm_tis get loaded? Does it produce
> any log?

Nope, and it should not get loaded anyway as I'm using PTT. With PTT you
use tpm_crb. TPM is working just fine.

> If the logs are recovered (but not parsed), you should already see an
> entry in the logs like:
> 
> efi:  SMBIOS=0x7fed6000  ACPI=0x7ff0  TPMEventLog=0x.
> 
> Can you see the TPMEventLog part?

I can check this when I'm back in Finland. Still in Prague. Tried to
test this with my work laptop (XPS13 with dTPM) now but the USB stick I
have with seems to be broken :-(

This is anyway almost guaranteed to go to 4.16  and I don't want to push
this to 4.15 so there is no rush right now (already sent my PR).

> The issue with extra logging is that the log recovery happens within
> the EFI stub phase where limited logging is available (which I think
> has been limited to error and fatal message only).
> For now, it cannot be a version mismatch as the stub will only request
> the version 1.2 format.

Right, I see.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Fri, Oct 13, 2017 at 10:47:46PM +0300, Jarkko Sakkinen wrote:
> On Thu, Oct 12, 2017 at 05:03:38PM +0200, Javier Martinez Canillas wrote:
> > On Thu, Oct 12, 2017 at 1:38 PM, Jarkko Sakkinen
> > <jarkko.sakki...@linux.intel.com> wrote:
> > 
> > [snip]
> > 
> > >
> > > Now all Thiebaud's patches have been applied to the master of
> > >
> > >   git://git.infradead.org/users/jjs/linux-tpmdd.git
> > >
> > > Testing is still pending.
> > >
> > 
> > I provided my reviewed and tested by tags for the patches but I
> > noticed that weren't picked. Probably my fault though since I answered
> > to the cover letter instead of the individual patches.
> > 
> > > /Jarkko
> > 
> > Best regards,
> > Javier
> 
> I will add it. The master branch is bleeding edge where tags might be
> sometimes (*not* usually) missing. The next branch is the one that goes
> to linux-next.
> 
> I'll check all tags from patchwork before moving any of these to next.
> 
> /Jarkko

Updated.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Wed, Oct 11, 2017 at 02:52:54PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 11, 2017 at 12:54:26PM +1100, James Morris wrote:
> > On Tue, 10 Oct 2017, Jarkko Sakkinen wrote:
> > 
> > > The way I've agreed with James Morris to have my tree is to be rooted to
> > > security trees next branch.
> > > 
> > > James, what actions should we take?
> > 
> > This process has changed recently -- I posted to lsm but forgot to post to 
> > linux-integrity.
> > 
> > http://kernsec.org/pipermail/linux-security-module-archive/2017-September/003356.html
> > 
> > Summary: please track the next-general branch in my tree for your 
> > development, it replaces 'next'.
> > 
> > 
> > - James
> > -- 
> > James Morris
> > <jmor...@namei.org>
> 
> Ah I'm subscribed to that list but lately been busy getting a huge patch
> set to platform-driver-x86 [1] for review, which has prioritized out
> reading much else than linux-integrity.
> 
> Thank you. I'll retry the patches tomorrow.
> 
> /Jarkko

Cannot observer binary_bios_measuremens file.

What kind of hardware was used to develop/test this?

I tried it with Kabylake and PTT (firmware TPM).

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Wed, Oct 11, 2017 at 02:53:18PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 11, 2017 at 02:52:54PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Oct 11, 2017 at 12:54:26PM +1100, James Morris wrote:
> > > On Tue, 10 Oct 2017, Jarkko Sakkinen wrote:
> > > 
> > > > The way I've agreed with James Morris to have my tree is to be rooted to
> > > > security trees next branch.
> > > > 
> > > > James, what actions should we take?
> > > 
> > > This process has changed recently -- I posted to lsm but forgot to post 
> > > to 
> > > linux-integrity.
> > > 
> > > http://kernsec.org/pipermail/linux-security-module-archive/2017-September/003356.html
> > > 
> > > Summary: please track the next-general branch in my tree for your 
> > > development, it replaces 'next'.
> > > 
> > > 
> > > - James
> > > -- 
> > > James Morris
> > > <jmor...@namei.org>
> > 
> > Ah I'm subscribed to that list but lately been busy getting a huge patch
> > set to platform-driver-x86 [1] for review, which has prioritized out
> > reading much else than linux-integrity.
> > 
> > Thank you. I'll retry the patches tomorrow.
> > 
> > /Jarkko
> 
> [1] http://www.spinics.net/lists/platform-driver-x86/msg13260.html
> 
> /Jarkko

Now all Thiebaud's patches have been applied to the master of

  git://git.infradead.org/users/jjs/linux-tpmdd.git

Testing is still pending.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Wed, Oct 11, 2017 at 12:54:26PM +1100, James Morris wrote:
> On Tue, 10 Oct 2017, Jarkko Sakkinen wrote:
> 
> > The way I've agreed with James Morris to have my tree is to be rooted to
> > security trees next branch.
> > 
> > James, what actions should we take?
> 
> This process has changed recently -- I posted to lsm but forgot to post to 
> linux-integrity.
> 
> http://kernsec.org/pipermail/linux-security-module-archive/2017-September/003356.html
> 
> Summary: please track the next-general branch in my tree for your 
> development, it replaces 'next'.
> 
> 
> - James
> -- 
> James Morris
> <jmor...@namei.org>

Ah I'm subscribed to that list but lately been busy getting a huge patch
set to platform-driver-x86 [1] for review, which has prioritized out
reading much else than linux-integrity.

Thank you. I'll retry the patches tomorrow.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Wed, Oct 11, 2017 at 02:52:54PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 11, 2017 at 12:54:26PM +1100, James Morris wrote:
> > On Tue, 10 Oct 2017, Jarkko Sakkinen wrote:
> > 
> > > The way I've agreed with James Morris to have my tree is to be rooted to
> > > security trees next branch.
> > > 
> > > James, what actions should we take?
> > 
> > This process has changed recently -- I posted to lsm but forgot to post to 
> > linux-integrity.
> > 
> > http://kernsec.org/pipermail/linux-security-module-archive/2017-September/003356.html
> > 
> > Summary: please track the next-general branch in my tree for your 
> > development, it replaces 'next'.
> > 
> > 
> > - James
> > -- 
> > James Morris
> > <jmor...@namei.org>
> 
> Ah I'm subscribed to that list but lately been busy getting a huge patch
> set to platform-driver-x86 [1] for review, which has prioritized out
> reading much else than linux-integrity.
> 
> Thank you. I'll retry the patches tomorrow.
> 
> /Jarkko

[1] http://www.spinics.net/lists/platform-driver-x86/msg13260.html

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Wed, Oct 04, 2017 at 01:12:27PM +0200, Thiebaud Weksteen wrote:
> On Wed, Oct 4, 2017 at 12:51 PM, Jarkko Sakkinen
> <jarkko.sakki...@linux.intel.com> wrote:
> > On Fri, Sep 29, 2017 at 08:16:17PM +0300, Jarkko Sakkinen wrote:
> >> On Tue, Sep 26, 2017 at 02:49:31PM +0200, Thiebaud Weksteen wrote:
> >> > On Tue, Sep 26, 2017 at 1:45 PM, Jarkko Sakkinen
> >> > <jarkko.sakki...@linux.intel.com> wrote:
> >> > > On Wed, Sep 20, 2017 at 10:13:39AM +0200, Thiebaud Weksteen wrote:
> >> > >> With TPM 2.0 specification, the event logs may only be accessible by
> >> > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area 
> >> > >> to
> >> > >> a new Linux-specific EFI configuration table so it remains accessible
> >> > >> once booted.
> >> > >>
> >> > >> When calling this service, it is possible to specify the expected 
> >> > >> format
> >> > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, 
> >> > >> only the
> >> > >> first format is retrieved.
> >> > >>
> >> > >> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> >> > >
> >> > > Does not apply:
> >> > >
> >> > > Applying: tpm: move tpm_eventlog.h outside of drivers folder
> >> > > Applying: tpm: rename event log provider files
> >> > > Applying: tpm: add event log format version
> >> > > Applying: efi: call get_event_log before ExitBootServices
> >> > > error: sha1 information is lacking or useless 
> >> > > (drivers/firmware/efi/efi.c).
> >> > > error: could not build fake ancestor
> >> > > Patch failed at 0004 efi: call get_event_log before ExitBootServices
> >> > > The copy of the patch that failed is found in: .git/rebase-apply/patch
> >> > > When you have resolved this problem, run "git am --continue".
> >> > > If you prefer to skip this patch, run "git am --skip" instead.
> >> > > To restore the original branch and stop patching, run "git am --abort".
> >> > >
> >> > > Just rebased my tree to the latest security-next.
> >> >
> >> > It applies fine on security/next-general which is more up-to-date.
> >> > (security/next does not include
> >> > ccc829ba3624beb9a703fc995d016b836d9eead8 on which this patch set is
> >> > based)
> >>
> >> Thanks, my bad, I though that I had it updated.
> >>
> >> I'll update my tree and retry.
> >>
> >> /Jarkko
> >
> > My master is up to date with security/next.
> >
> > Still get the same result:
> >
> > $ git am -3 
> > ~/Downloads/v3-4-5-efi-call-get_event_log-before-ExitBootServices.patch
> > Applying: efi: call get_event_log before ExitBootServices
> > error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
> > error: could not build fake ancestor
> > Patch failed at 0001 efi: call get_event_log before ExitBootServices
> > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> > Maybe you have some other trees fetched in your local GIT so that it
> > finds the ancestors? Anyway, cannot test this at this point.
> >
> > /Jarkko
> 
> The security/next branch still does not contain the commit I mentioned
> (ccc829ba3624beb9a703fc995d016b836d9eead8), which is already part of
> torvalds/master now.
> 
>  $ git branch -a --contains ccc829ba3624beb9a703fc995d016b836d9eead8
>   efi_tpm2_eventlog
>   master
>   remotes/linux-next/akpm
>   remotes/linux-next/akpm-base
>   remotes/linux-next/master
>   remotes/linux-next/stable
>   remotes/security/fixes-v4.14-rc3
>   remotes/security/fixes-v4.14-rc4
>   remotes/security/next-general
>   remotes/security/next-testing
>   remotes/torvalds/master
> 
> Is there any reason why you are trying to merge on that specific
> branch and not next-general or next-testing? Would you know the
> purpose of all these next-* branches?
> 
> Thanks,
> Thiebaud

The way I've agreed with James Morris to have my tree is to be rooted to
security trees next branch.

James, what actions should we take?

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Wed, Oct 04, 2017 at 01:51:13PM +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 29, 2017 at 08:16:17PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Sep 26, 2017 at 02:49:31PM +0200, Thiebaud Weksteen wrote:
> > > On Tue, Sep 26, 2017 at 1:45 PM, Jarkko Sakkinen
> > > <jarkko.sakki...@linux.intel.com> wrote:
> > > > On Wed, Sep 20, 2017 at 10:13:39AM +0200, Thiebaud Weksteen wrote:
> > > >> With TPM 2.0 specification, the event logs may only be accessible by
> > > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area 
> > > >> to
> > > >> a new Linux-specific EFI configuration table so it remains accessible
> > > >> once booted.
> > > >>
> > > >> When calling this service, it is possible to specify the expected 
> > > >> format
> > > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only 
> > > >> the
> > > >> first format is retrieved.
> > > >>
> > > >> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> > > >
> > > > Does not apply:
> > > >
> > > > Applying: tpm: move tpm_eventlog.h outside of drivers folder
> > > > Applying: tpm: rename event log provider files
> > > > Applying: tpm: add event log format version
> > > > Applying: efi: call get_event_log before ExitBootServices
> > > > error: sha1 information is lacking or useless 
> > > > (drivers/firmware/efi/efi.c).
> > > > error: could not build fake ancestor
> > > > Patch failed at 0004 efi: call get_event_log before ExitBootServices
> > > > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > > > When you have resolved this problem, run "git am --continue".
> > > > If you prefer to skip this patch, run "git am --skip" instead.
> > > > To restore the original branch and stop patching, run "git am --abort".
> > > >
> > > > Just rebased my tree to the latest security-next.
> > > 
> > > It applies fine on security/next-general which is more up-to-date.
> > > (security/next does not include
> > > ccc829ba3624beb9a703fc995d016b836d9eead8 on which this patch set is
> > > based)
> > 
> > Thanks, my bad, I though that I had it updated.
> > 
> > I'll update my tree and retry.
> > 
> > /Jarkko
> 
> My master is up to date with security/next.
> 
> Still get the same result:
> 
> $ git am -3 
> ~/Downloads/v3-4-5-efi-call-get_event_log-before-ExitBootServices.patch
> Applying: efi: call get_event_log before ExitBootServices
> error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
> error: could not build fake ancestor
> Patch failed at 0001 efi: call get_event_log before ExitBootServices
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> Maybe you have some other trees fetched in your local GIT so that it
> finds the ancestors? Anyway, cannot test this at this point.
> 
> /Jarkko

I pushed the first three patches to my master as they looked OK. You
should still consider them unreviewed.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Fri, Sep 29, 2017 at 08:16:17PM +0300, Jarkko Sakkinen wrote:
> On Tue, Sep 26, 2017 at 02:49:31PM +0200, Thiebaud Weksteen wrote:
> > On Tue, Sep 26, 2017 at 1:45 PM, Jarkko Sakkinen
> > <jarkko.sakki...@linux.intel.com> wrote:
> > > On Wed, Sep 20, 2017 at 10:13:39AM +0200, Thiebaud Weksteen wrote:
> > >> With TPM 2.0 specification, the event logs may only be accessible by
> > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> > >> a new Linux-specific EFI configuration table so it remains accessible
> > >> once booted.
> > >>
> > >> When calling this service, it is possible to specify the expected format
> > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only 
> > >> the
> > >> first format is retrieved.
> > >>
> > >> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> > >
> > > Does not apply:
> > >
> > > Applying: tpm: move tpm_eventlog.h outside of drivers folder
> > > Applying: tpm: rename event log provider files
> > > Applying: tpm: add event log format version
> > > Applying: efi: call get_event_log before ExitBootServices
> > > error: sha1 information is lacking or useless 
> > > (drivers/firmware/efi/efi.c).
> > > error: could not build fake ancestor
> > > Patch failed at 0004 efi: call get_event_log before ExitBootServices
> > > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > > When you have resolved this problem, run "git am --continue".
> > > If you prefer to skip this patch, run "git am --skip" instead.
> > > To restore the original branch and stop patching, run "git am --abort".
> > >
> > > Just rebased my tree to the latest security-next.
> > 
> > It applies fine on security/next-general which is more up-to-date.
> > (security/next does not include
> > ccc829ba3624beb9a703fc995d016b836d9eead8 on which this patch set is
> > based)
> 
> Thanks, my bad, I though that I had it updated.
> 
> I'll update my tree and retry.
> 
> /Jarkko

My master is up to date with security/next.

Still get the same result:

$ git am -3 
~/Downloads/v3-4-5-efi-call-get_event_log-before-ExitBootServices.patch
Applying: efi: call get_event_log before ExitBootServices
error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
error: could not build fake ancestor
Patch failed at 0001 efi: call get_event_log before ExitBootServices
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Maybe you have some other trees fetched in your local GIT so that it
finds the ancestors? Anyway, cannot test this at this point.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Tue, Sep 26, 2017 at 02:49:31PM +0200, Thiebaud Weksteen wrote:
> On Tue, Sep 26, 2017 at 1:45 PM, Jarkko Sakkinen
> <jarkko.sakki...@linux.intel.com> wrote:
> > On Wed, Sep 20, 2017 at 10:13:39AM +0200, Thiebaud Weksteen wrote:
> >> With TPM 2.0 specification, the event logs may only be accessible by
> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> >> a new Linux-specific EFI configuration table so it remains accessible
> >> once booted.
> >>
> >> When calling this service, it is possible to specify the expected format
> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> >> first format is retrieved.
> >>
> >> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> >
> > Does not apply:
> >
> > Applying: tpm: move tpm_eventlog.h outside of drivers folder
> > Applying: tpm: rename event log provider files
> > Applying: tpm: add event log format version
> > Applying: efi: call get_event_log before ExitBootServices
> > error: sha1 information is lacking or useless (drivers/firmware/efi/efi.c).
> > error: could not build fake ancestor
> > Patch failed at 0004 efi: call get_event_log before ExitBootServices
> > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> > Just rebased my tree to the latest security-next.
> 
> It applies fine on security/next-general which is more up-to-date.
> (security/next does not include
> ccc829ba3624beb9a703fc995d016b836d9eead8 on which this patch set is
> based)

Thanks, my bad, I though that I had it updated.

I'll update my tree and retry.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] tpm: rename event log provider files

[Jarkko Sakkinen]

On Wed, Sep 20, 2017 at 10:13:37AM +0200, Thiebaud Weksteen wrote:
> Rename the current TPM Event Log provider files (ACPI and OF)
> for clarity.
> 
> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> ---
>  drivers/char/tpm/Makefile| 4 ++--
>  drivers/char/tpm/{tpm_acpi.c => tpm_eventlog_acpi.c} | 0
>  drivers/char/tpm/{tpm_of.c => tpm_eventlog_of.c} | 0
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename drivers/char/tpm/{tpm_acpi.c => tpm_eventlog_acpi.c} (100%)
>  rename drivers/char/tpm/{tpm_of.c => tpm_eventlog_of.c} (100%)

Reviewed-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com>

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/5] Call GetEventLog before ExitBootServices

[Jarkko Sakkinen]

On Wed, Sep 20, 2017 at 10:13:35AM +0200, Thiebaud Weksteen wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event
> Log Area (LAML and LASA). These logs are useful to understand and rebuild
> the final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields
> anymore. The recommended method is now to call the GetEventLog EFI
> protocol before ExitBootServices.
> 
> Implement this method within the EFI stub and create a copy of the logs
> for the TPM device using a Linux-specific EFI configuration table
> (LINUX_EFI_TPM_EVENT_LOG). This will create
> /sys/kernel/security/tpm0/binary_bios_measurements for TPM 2.0 devices
> (similarly to the current behaviour for TPM 1.2 devices).
> 
> Two formats for the log entries exist: TPM 1.2 (SHA1) and TPM 2.0 (Crypto
> Agile). This patch set only retrieves the first type of logs. The second
> type will be implemented in a subsequent patch set.
> 
> According to the specifications[1], once GetEventLog has been called,
> future events shall be stored in a separate EFI configuration table
> (EFI_TCG2_FINAL_EVENTS_TABLE). Events stored in this table are not
> processed in this patch set as they are stored in the Crypto Agile format.
> These could eventually be merged with the new table for a unified view
> of the logs from userspace.
> 
> [1] TCG EFI Protocol Specification, Revision 00.13, March 30, 2016
> 
> https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
> 
> ---
> 
> Patchset Changelog:
> 
> Version 3:
> - Move event log providers (acpi and of) to tpm_eventlog_*.c
> - Move efi changes from PATCH 3 to PATCH 2
> - Change return value of tpm_read_log_acpi and tpm_read_log_of
> - Change iounmap to memunmap calls
> - Use log_tbl as variable name for consistency
> - Fix kbuild failures
> 
> Version 2:
> - Move tpm_eventlog.h to top include directory, add commit for this.
> - Use EFI_LOADER_DATA to store the configuration table
> - Whitespace and new lines fixes
> 
> Thiebaud Weksteen (5):
>   tpm: move tpm_eventlog.h outside of drivers folder
>   tpm: rename event log provider files
>   tpm: add event log format version
>   efi: call get_event_log before ExitBootServices
>   tpm: parse TPM event logs based on EFI table
> 
>  arch/x86/boot/compressed/eboot.c   |  1 +
>  drivers/char/tpm/Makefile  |  5 +-
>  drivers/char/tpm/tpm-chip.c|  3 +-
>  drivers/char/tpm/tpm-interface.c   |  2 +-
>  drivers/char/tpm/tpm.h | 35 --
>  drivers/char/tpm/tpm1_eventlog.c   | 13 +++-
>  drivers/char/tpm/tpm2_eventlog.c   |  2 +-
>  .../char/tpm/{tpm_acpi.c => tpm_eventlog_acpi.c}   |  4 +-
>  drivers/char/tpm/tpm_eventlog_efi.c| 66 ++
>  drivers/char/tpm/{tpm_of.c => tpm_eventlog_of.c}   |  6 +-
>  drivers/firmware/efi/Makefile  |  2 +-
>  drivers/firmware/efi/efi.c |  4 ++
>  drivers/firmware/efi/libstub/Makefile  |  3 +-
>  drivers/firmware/efi/libstub/tpm.c | 81 
> ++
>  drivers/firmware/efi/tpm.c | 40 +++
>  include/linux/efi.h| 46 
>  {drivers/char/tpm => include/linux}/tpm_eventlog.h | 35 +++---
>  17 files changed, 304 insertions(+), 44 deletions(-)
>  rename drivers/char/tpm/{tpm_acpi.c => tpm_eventlog_acpi.c} (97%)
>  create mode 100644 drivers/char/tpm/tpm_eventlog_efi.c
>  rename drivers/char/tpm/{tpm_of.c => tpm_eventlog_of.c} (93%)
>  create mode 100644 drivers/firmware/efi/tpm.c
>  rename {drivers/char/tpm => include/linux}/tpm_eventlog.h (77%)
> 
> -- 
> 2.14.1.821.g8fa685d3b7-goog
> 

Thank you. I'll have to postpone testing this at some point next week.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 18, 2017 at 02:28:45PM +0200, Thiebaud Weksteen wrote:
> On Thu, Sep 14, 2017 at 9:02 PM, Jarkko Sakkinen
> <jarkko.sakki...@linux.intel.com> wrote:
> > On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> >> On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> >> <jarkko.sakki...@linux.intel.com> wrote:
> >> > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> >> >> With TPM 2.0 specification, the event logs may only be accessible by
> >> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> >> >> a new Linux-specific EFI configuration table so it remains accessible
> >> >> once booted.
> >> >>
> >> >> When calling this service, it is possible to specify the expected format
> >> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only 
> >> >> the
> >> >> first format is retrieved.
> >> >>
> >> >> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> >> >
> >> > With a quick skim the code change looks good but I remember from
> >> > Matthew's talk that there was this issue that ExitBootServices() would
> >> > cause a yet another event?
> >> >
> >> > I guess you could manually synthetize that event by reading the PCR
> >> > values right after ExitBootServices()?
> >>
> >> I think that would involve breaking SHA1… the information should be
> >
> > You are absolutely right, was not thinking clearly :-)
> >
> >> available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> >> /should/ just be a matter of merging the events from that into the
> >> event log.
> >
> > Right, it is available through runtime services. Why this isn't part
> > of the patch set?
> 
> This is not included yet as this table
> (EFI_TCG2_FINAL_EVENTS_TABLE_GUID) relies on the TPM2 format for the
> log entries (TCG_PCR_EVENT2, "Crypto Agile"). I first plan to add the
> parsing of this log version (ie, efi_retrieve_tpm2_eventlog_2) before
> adding the merging of both tables. But these will be separate patch
> sets.

OK, this should be documented to the commit message to make it clear.

linux-integr...@vger.kernel.org is now up and running. I'm still
surviving from jetlag etc. so testing might be postponed either near end
of the week or next week.

Thanks for doing this. This is really important stuff in order to get the
Linux TPM 2.0 support feature complete.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Thu, Sep 14, 2017 at 12:02:47PM -0700, Jarkko Sakkinen wrote:
> On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> > On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> > <jarkko.sakki...@linux.intel.com> wrote:
> > > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> > >> With TPM 2.0 specification, the event logs may only be accessible by
> > >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> > >> a new Linux-specific EFI configuration table so it remains accessible
> > >> once booted.
> > >>
> > >> When calling this service, it is possible to specify the expected format
> > >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only 
> > >> the
> > >> first format is retrieved.
> > >>
> > >> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> > >
> > > With a quick skim the code change looks good but I remember from
> > > Matthew's talk that there was this issue that ExitBootServices() would
> > > cause a yet another event?
> > >
> > > I guess you could manually synthetize that event by reading the PCR
> > > values right after ExitBootServices()?
> > 
> > I think that would involve breaking SHA1… the information should be
> 
> You are absolutely right, was not thinking clearly :-)
> 
> > available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> > /should/ just be a matter of merging the events from that into the
> > event log.
> 
> Right, it is available through runtime services. Why this isn't part
> of the patch set?

Anyway, I'll try this out out when I get back to Finland. Still before
landing this to mainline I think it would make sense to make it complete
wouldn't it?

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Thu, Sep 14, 2017 at 11:48:54AM -0700, Matthew Garrett wrote:
> On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
> <jarkko.sakki...@linux.intel.com> wrote:
> > On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> >> With TPM 2.0 specification, the event logs may only be accessible by
> >> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> >> a new Linux-specific EFI configuration table so it remains accessible
> >> once booted.
> >>
> >> When calling this service, it is possible to specify the expected format
> >> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> >> first format is retrieved.
> >>
> >> Signed-off-by: Thiebaud Weksteen <tw...@google.com>
> >
> > With a quick skim the code change looks good but I remember from
> > Matthew's talk that there was this issue that ExitBootServices() would
> > cause a yet another event?
> >
> > I guess you could manually synthetize that event by reading the PCR
> > values right after ExitBootServices()?
> 
> I think that would involve breaking SHA1… the information should be

You are absolutely right, was not thinking clearly :-)

> available in the TCG_TREE_FINAL_EVENTS configuration table, so it
> /should/ just be a matter of merging the events from that into the
> event log.

Right, it is available through runtime services. Why this isn't part
of the patch set?

/Jrakko

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] tpm: parse TPM event logs based on EFI table

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:22PM +0200, Thiebaud Weksteen wrote:
> If we are not able to retrieve the TPM event logs from the ACPI table,
> check the EFI configuration table (Linux-specific GUID).
> 
> The format version of the log may be returned by the function. If not
> specified (by previous implementation: tpm_acpi and tpm_of), we default
> to the version of the chip (previous behaviour).
> 
> Signed-off-by: Thiebaud Weksteen 

You saw my comment about file naming. I.e. tpm_eventlog_efi.c would be
a more senseful name.

> ---
>  drivers/char/tpm/Makefile|  2 +-
>  drivers/char/tpm/tpm.h   |  8 +
>  drivers/char/tpm/tpm1_eventlog.c | 15 +++--
>  drivers/char/tpm/tpm_efi.c   | 66 
> 
>  drivers/firmware/efi/efi.c   |  2 ++
>  include/linux/efi.h  |  1 +
>  6 files changed, 90 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm_efi.c
> 
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 23681f01f95a..74182a63eef2 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -4,7 +4,7 @@
>  obj-$(CONFIG_TCG_TPM) += tpm.o
>  tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
>tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
> - tpm2-space.o
> + tpm2-space.o tpm_efi.o
>  tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
>  tpm-$(CONFIG_OF) += tpm_of.o
>  obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 46caccf6fd1a..1bd97e01df50 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -597,6 +597,14 @@ static inline int tpm_read_log_of(struct tpm_chip *chip)
>   return -ENODEV;
>  }
>  #endif
> +#if defined(CONFIG_EFI)
> +int tpm_read_log_efi(struct tpm_chip *chip);
> +#else
> +static inline int tpm_read_log_efi(struct tpm_chip *chip)
> +{
> + return -ENODEV;
> +}
> +#endif
>  
>  int tpm_bios_log_setup(struct tpm_chip *chip);
>  void tpm_bios_log_teardown(struct tpm_chip *chip);
> diff --git a/drivers/char/tpm/tpm1_eventlog.c 
> b/drivers/char/tpm/tpm1_eventlog.c
> index d6f70f365443..7e25e6bff6ce 100644
> --- a/drivers/char/tpm/tpm1_eventlog.c
> +++ b/drivers/char/tpm/tpm1_eventlog.c
> @@ -21,6 +21,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -371,6 +372,10 @@ static int tpm_read_log(struct tpm_chip *chip)
>   if (rc != -ENODEV)
>   return rc;
>  
> + rc = tpm_read_log_efi(chip);
> + if (rc != -ENODEV)
> + return rc;
> +
>   return tpm_read_log_of(chip);
>  }
>  
> @@ -388,11 +393,13 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  {
>   const char *name = dev_name(>dev);
>   unsigned int cnt;
> - int rc = 0;
> + int rc = 0, log_version;

A tid bit, one declaration per line.

> +
>  
>   rc = tpm_read_log(chip);
> - if (rc)
> + if (rc < 0)
>   return rc;
> + log_version = rc;
>  
>   cnt = 0;
>   chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
> @@ -404,7 +411,9 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>   cnt++;
>  
>   chip->bin_log_seqops.chip = chip;
> - if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +
> + if (log_version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 ||
> + (!log_version && (chip->flags & TPM_CHIP_FLAG_TPM2)))
>   chip->bin_log_seqops.seqops =
>   _binary_b_measurements_seqops;
>   else
> diff --git a/drivers/char/tpm/tpm_efi.c b/drivers/char/tpm/tpm_efi.c
> new file mode 100644
> index ..c8247fc45bb0
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_efi.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (C) 2017 Google
> + *
> + * Authors:
> + *  Thiebaud Weksteen 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + */
> +
> +#include 
> +#include 
> +
> +#include "tpm.h"
> +
> +/* read binary bios log from EFI configuration table */
> +int tpm_read_log_efi(struct tpm_chip *chip)
> +{
> +
> + struct linux_efi_tpm_eventlog *log_tbl;
> + struct tpm_bios_log *log;
> + u32 log_size;
> + u8 tpm_log_version;
> +
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> + return -ENODEV;
> +
> + if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> + return -ENODEV;
> +
> + log = >log;
> +
> + log_tbl = memremap(efi.tpm_log, sizeof(*log_tbl), MEMREMAP_WB);
> + if (!log_tbl) {
> + pr_err("Could not map UEFI TPM log table !\n");
> + return -ENOMEM;
> + }
> +
> + log_size = log_tbl->size;
> + iounmap(log_tbl);
> +
> + log_tbl = memremap(efi.tpm_log, sizeof(*log_tbl) + 

Re: [PATCH v2 2/3] efi: call get_event_log before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:21PM +0200, Thiebaud Weksteen wrote:
> With TPM 2.0 specification, the event logs may only be accessible by
> calling an EFI Boot Service. Modify the EFI stub to copy the log area to
> a new Linux-specific EFI configuration table so it remains accessible
> once booted.
> 
> When calling this service, it is possible to specify the expected format
> of the logs: TPM 1.2 (SHA1) or TPM 2.0 ("Crypto Agile"). For now, only the
> first format is retrieved.
> 
> Signed-off-by: Thiebaud Weksteen 

With a quick skim the code change looks good but I remember from
Matthew's talk that there was this issue that ExitBootServices() would
cause a yet another event?

I guess you could manually synthetize that event by reading the PCR
values right after ExitBootServices()?

Anyway, great work, thanks for making this effort.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] Call GetEventLog before ExitBootServices

[Jarkko Sakkinen]

On Mon, Sep 11, 2017 at 12:00:19PM +0200, Thiebaud Weksteen wrote:
> With TPM 1.2, the ACPI table ("TCPA") has two fields to recover the Event Log
> Area (LAML and LASA). These logs are useful to understand and rebuild the
> final values of PCRs.
> 
> With TPM 2.0, the ACPI table ("TPM2") does not contain these fields anymore.
> The recommended method is now to call the GetEventLog EFI protocol before
> ExitBootServices.
> 
> Implement this method within the EFI stub and create copy of the logs for the
> TPM device. This will create 
> /sys/kernel/security/tpm0/binary_bios_measurements
> for TPM 2.0 devices (similarly to the current behaviour for TPM 1.2 devices).
> 
> ---
> 
> Patchset Changelog:
> 
> Version 2:
> - Move tpm_eventlog.h to top include directory, add commit for this.
> - Use EFI_LOADER_DATA to store the configuration table
> - Whitespace and new lines fixes
> 
> 
> Thiebaud Weksteen (3):
>   tpm: move tpm_eventlog.h outside of drivers folder
>   efi: call get_event_log before ExitBootServices
>   tpm: parse TPM event logs based on EFI table
> 
>  arch/x86/boot/compressed/eboot.c   |  1 +
>  drivers/char/tpm/Makefile  |  2 +-
>  drivers/char/tpm/tpm-chip.c|  3 +-
>  drivers/char/tpm/tpm-interface.c   |  2 +-
>  drivers/char/tpm/tpm.h | 35 --
>  drivers/char/tpm/tpm1_eventlog.c   | 17 +++--
>  drivers/char/tpm/tpm2_eventlog.c   |  2 +-
>  drivers/char/tpm/tpm_acpi.c|  2 +-
>  drivers/char/tpm/tpm_efi.c | 66 ++
>  drivers/char/tpm/tpm_of.c  |  2 +-

I think these filenames are just awful. Now that you are introducing
completely a new file, it would make sense rename these as

* tpm_eventlog_acpi.c
* tpm_eventlog_efi.c
* tpm_eventlog_of.c

Please wait for further review comments before sending a refined patch
set. Please have renames for tpm_acpi.c and tpm_of.c in its own commit
before introducing other changes.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html