Patchwork [v2,2/3] efi: call get_event_log before ExitBootServices

login
register
mail settings
Submitter Thiebaud Weksteen via tpmdd-devel
Date Sept. 11, 2017, 10 a.m.
Message ID <20170911100022.7251-3-tweek@google.com>
Download mbox | patch
Permalink /patch/335675/
State New
Headers show

Comments

Thiebaud Weksteen via tpmdd-devel - Sept. 11, 2017, 10 a.m.
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 <tweek@google.com>
---
 arch/x86/boot/compressed/eboot.c      |  1 +
 drivers/firmware/efi/Makefile         |  2 +-
 drivers/firmware/efi/efi.c            |  2 +
 drivers/firmware/efi/libstub/Makefile |  3 +-
 drivers/firmware/efi/libstub/tpm.c    | 81 +++++++++++++++++++++++++++++++++++
 drivers/firmware/efi/tpm.c            | 39 +++++++++++++++++
 include/linux/efi.h                   | 49 +++++++++++++++++++++
 7 files changed, 174 insertions(+), 3 deletions(-)
 create mode 100644 drivers/firmware/efi/tpm.c
kbuild test robot - Sept. 13, 2017, 2:49 p.m.
Hi Thiebaud,

[auto build test ERROR on efi/next]
[also build test ERROR on next-20170913]
[cannot apply to char-misc/char-misc-testing linus/master v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

Note: the linux-review/Thiebaud-Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312 HEAD 58dc8ee5de3bb5fb1ef216b80a76102d2de2b141 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/firmware/efi/tpm.c: In function 'efi_tpm_eventlog_init':
>> drivers/firmware/efi/tpm.c:24:9: error: 'struct efi' has no member named 'tpm_log'
     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
            ^
   In file included from arch/ia64/include/asm/io.h:23:0,
                    from arch/ia64/include/asm/smp.h:20,
                    from include/linux/smp.h:59,
                    from include/linux/percpu.h:6,
                    from include/linux/percpu-rwsem.h:6,
                    from include/linux/fs.h:32,
                    from include/linux/proc_fs.h:8,
                    from include/linux/efi.h:18,
                    from drivers/firmware/efi/tpm.c:10:
   drivers/firmware/efi/tpm.c:27:26: error: 'struct efi' has no member named 'tpm_log'
     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
                             ^
   arch/ia64/include/asm/early_ioremap.h:5:62: note: in definition of macro 'early_memremap'
    #define early_memremap(phys_addr, size)        early_ioremap(phys_addr, size)
                                                                 ^~~~~~~~~
   In file included from include/linux/kernel.h:13:0,
                    from include/linux/list.h:8,
                    from include/linux/preempt.h:10,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/efi.h:16,
                    from drivers/firmware/efi/tpm.c:10:
   drivers/firmware/efi/tpm.c:30:7: error: 'struct efi' has no member named 'tpm_log'
       efi.tpm_log);
          ^
   include/linux/printk.h:301:33: note: in definition of macro 'pr_err'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
                                    ^~~~~~~~~~~
   drivers/firmware/efi/tpm.c:35:22: error: 'struct efi' has no member named 'tpm_log'
     memblock_reserve(efi.tpm_log, tbl_size);
                         ^

vim +24 drivers/firmware/efi/tpm.c

    15	
    16	/*
    17	 * Reserve the memory associated with the TPM Event Log configuration table.
    18	 */
    19	int __init efi_tpm_eventlog_init(void)
    20	{
    21		struct linux_efi_tpm_eventlog *tbl;
    22		unsigned int tbl_size;
    23	
  > 24		if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
    25			return 0;
    26	
    27		tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
    28		if (!tbl) {
    29			pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
    30				efi.tpm_log);
    31			return -ENOMEM;
    32		}
    33	
    34		tbl_size = sizeof(*tbl) + tbl->size;
    35		memblock_reserve(efi.tpm_log, tbl_size);
    36		early_memunmap(tbl, sizeof(*tbl));
    37		return 0;
    38	}
    39	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Thiebaud Weksteen via tpmdd-devel - Sept. 13, 2017, 4:28 p.m.
efi_tpm_eventlog_init should be in patch 0003 and not 0002.
I'll send a new version of the patch set once I've received more feedback.

On Wed, Sep 13, 2017 at 4:49 PM, kbuild test robot <lkp@intel.com> wrote:

> Hi Thiebaud,
>
> [auto build test ERROR on efi/next]
> [also build test ERROR on next-20170913]
> [cannot apply to char-misc/char-misc-testing linus/master v4.13]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Thiebaud-
> Weksteen/Call-GetEventLog-before-ExitBootServices/20170913-221312
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
> config: ia64-allmodconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 6.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/
> sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=ia64
>
> Note: the linux-review/Thiebaud-Weksteen/Call-GetEventLog-
> before-ExitBootServices/20170913-221312 HEAD
> 58dc8ee5de3bb5fb1ef216b80a76102d2de2b141 builds fine.
>       It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
>    drivers/firmware/efi/tpm.c: In function 'efi_tpm_eventlog_init':
> >> drivers/firmware/efi/tpm.c:24:9: error: 'struct efi' has no member
> named 'tpm_log'
>      if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>             ^
>    In file included from arch/ia64/include/asm/io.h:23:0,
>                     from arch/ia64/include/asm/smp.h:20,
>                     from include/linux/smp.h:59,
>                     from include/linux/percpu.h:6,
>                     from include/linux/percpu-rwsem.h:6,
>                     from include/linux/fs.h:32,
>                     from include/linux/proc_fs.h:8,
>                     from include/linux/efi.h:18,
>                     from drivers/firmware/efi/tpm.c:10:
>    drivers/firmware/efi/tpm.c:27:26: error: 'struct efi' has no member
> named 'tpm_log'
>      tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>                              ^
>    arch/ia64/include/asm/early_ioremap.h:5:62: note: in definition of
> macro 'early_memremap'
>     #define early_memremap(phys_addr, size)
> early_ioremap(phys_addr, size)
>                                                                  ^~~~~~~~~
>    In file included from include/linux/kernel.h:13:0,
>                     from include/linux/list.h:8,
>                     from include/linux/preempt.h:10,
>                     from include/linux/spinlock.h:50,
>                     from include/linux/seqlock.h:35,
>                     from include/linux/time.h:5,
>                     from include/linux/efi.h:16,
>                     from drivers/firmware/efi/tpm.c:10:
>    drivers/firmware/efi/tpm.c:30:7: error: 'struct efi' has no member
> named 'tpm_log'
>        efi.tpm_log);
>           ^
>    include/linux/printk.h:301:33: note: in definition of macro 'pr_err'
>      printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>                                     ^~~~~~~~~~~
>    drivers/firmware/efi/tpm.c:35:22: error: 'struct efi' has no member
> named 'tpm_log'
>      memblock_reserve(efi.tpm_log, tbl_size);
>                          ^
>
> vim +24 drivers/firmware/efi/tpm.c
>
>     15
>     16  /*
>     17   * Reserve the memory associated with the TPM Event Log
> configuration table.
>     18   */
>     19  int __init efi_tpm_eventlog_init(void)
>     20  {
>     21          struct linux_efi_tpm_eventlog *tbl;
>     22          unsigned int tbl_size;
>     23
>   > 24          if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>     25                  return 0;
>     26
>     27          tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>     28          if (!tbl) {
>     29                  pr_err("Failed to map TPM Event Log table @
> 0x%lx\n",
>     30                          efi.tpm_log);
>     31                  return -ENOMEM;
>     32          }
>     33
>     34          tbl_size = sizeof(*tbl) + tbl->size;
>     35          memblock_reserve(efi.tpm_log, tbl_size);
>     36          early_memunmap(tbl, sizeof(*tbl));
>     37          return 0;
>     38  }
>     39
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all                   Intel
> Corporation
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Javier Martinez Canillas - Sept. 14, 2017, 10:24 a.m.
On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel 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 <tweek@google.com>
> ---

[snip]

> +void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
> +{

[snip]

> +
> +	/* Allocate space for the logs and copy them. */
> +	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
> +				sizeof(*log_tbl) + log_size,
> +				(void **) &log_tbl);
> +
> +	if (status != EFI_SUCCESS) {
> +		efi_printk(sys_table_arg,
> +			   "Unable to allocate memory for event log\n");
> +		return;
> +	}

If this fails or any previous error that will prevent the event log table + logs
to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
it will still try to access them even if the EFI allocate_pool did not succeed.

> + */
> +int __init efi_tpm_eventlog_init(void)
> +{
> +	struct linux_efi_tpm_eventlog *tbl;
> +	unsigned int tbl_size;
> +

The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
log_tbl as variable name, so I would use it here too for consistency.

> +	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
> +		return 0;
> +
> +	tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
> +	if (!tbl) {
> +		pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
> +			efi.tpm_log);
> +		return -ENOMEM;
> +	}
> +

Same question than before, if this fails then the table + logs memory won't be
reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?

> +	tbl_size = sizeof(*tbl) + tbl->size;
> +	memblock_reserve(efi.tpm_log, tbl_size);
> +	early_memunmap(tbl, sizeof(*tbl));
> +	return 0;

Best regards,
Jarkko Sakkinen - Sept. 14, 2017, 6:43 p.m.
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 <tweek@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()?

Anyway, great work, thanks for making this effort.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Thiebaud Weksteen via tpmdd-devel - Sept. 14, 2017, 6:48 p.m.
On Thu, Sep 14, 2017 at 11:43 AM, Jarkko Sakkinen
<jarkko.sakkinen@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 <tweek@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
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.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Jarkko Sakkinen - Sept. 14, 2017, 7:02 p.m.
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.sakkinen@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 <tweek@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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Jarkko Sakkinen - Sept. 15, 2017, 4:06 a.m.
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.sakkinen@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 <tweek@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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Thiebaud Weksteen via tpmdd-devel - Sept. 18, 2017, 12:11 p.m.
On Thu, Sep 14, 2017 at 12:24 PM, Javier Martinez Canillas
<javierm@redhat.com> wrote:
> On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel 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 <tweek@google.com>
>> ---
>
> [snip]
>
>> +void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
>> +{
>
> [snip]
>
>> +
>> +     /* Allocate space for the logs and copy them. */
>> +     status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
>> +                             sizeof(*log_tbl) + log_size,
>> +                             (void **) &log_tbl);
>> +
>> +     if (status != EFI_SUCCESS) {
>> +             efi_printk(sys_table_arg,
>> +                        "Unable to allocate memory for event log\n");
>> +             return;
>> +     }
>
> If this fails or any previous error that will prevent the event log table + logs
> to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
> it will still try to access them even if the EFI allocate_pool did not succeed.
>

The implicit part that covers this case is in
drivers/firmware/efi/efi.c. The match_config_table function will go
through all the installed configuration tables and only fill up the
associated member of the efi structure if it exists. In this case,
.tpm_log will remains at EFI_INVALID_TABLE_ADDR unless
efi_call_early(install_configuration_table, ...) is called. So no
further processing is to be expected should the allocation failed.

>> + */
>> +int __init efi_tpm_eventlog_init(void)
>> +{
>> +     struct linux_efi_tpm_eventlog *tbl;
>> +     unsigned int tbl_size;
>> +
>
> The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
> log_tbl as variable name, so I would use it here too for consistency.
>

Done.

>> +     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>> +             return 0;
>> +
>> +     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>> +     if (!tbl) {
>> +             pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
>> +                     efi.tpm_log);
>> +             return -ENOMEM;
>> +     }
>> +
>
> Same question than before, if this fails then the table + logs memory won't be
> reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
> is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
> tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?
>

That's right. To keep it simple, it might be easier to just set
tpm_log to EFI_INVALID_TABLE_ADDR if that happens. Added in the next
version of the patch set.

>> +     tbl_size = sizeof(*tbl) + tbl->size;
>> +     memblock_reserve(efi.tpm_log, tbl_size);
>> +     early_memunmap(tbl, sizeof(*tbl));
>> +     return 0;
>
> Best regards,
> --
> Javier Martinez Canillas
> Software Engineer - Desktop Hardware Enablement
> Red Hat

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Thiebaud Weksteen via tpmdd-devel - Sept. 18, 2017, 12:28 p.m.
On Thu, Sep 14, 2017 at 9:02 PM, Jarkko Sakkinen
<jarkko.sakkinen@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.sakkinen@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 <tweek@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.

>
> /Jrakko
>
> /Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Javier Martinez Canillas - Sept. 18, 2017, 12:49 p.m.
On 09/18/2017 02:11 PM, Thiebaud Weksteen wrote:
> On Thu, Sep 14, 2017 at 12:24 PM, Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> On 09/11/2017 12:00 PM, Thiebaud Weksteen via tpmdd-devel wrote:

[snip]

>>> +
>>> +     if (status != EFI_SUCCESS) {
>>> +             efi_printk(sys_table_arg,
>>> +                        "Unable to allocate memory for event log\n");
>>> +             return;
>>> +     }
>>
>> If this fails or any previous error that will prevent the event log table + logs
>> to be allocated, shouldn't tpm_read_log_efi() be notified somehow? Since AFAICT
>> it will still try to access them even if the EFI allocate_pool did not succeed.
>>
> 
> The implicit part that covers this case is in
> drivers/firmware/efi/efi.c. The match_config_table function will go
> through all the installed configuration tables and only fill up the
> associated member of the efi structure if it exists. In this case,
> .tpm_log will remains at EFI_INVALID_TABLE_ADDR unless
> efi_call_early(install_configuration_table, ...) is called. So no
> further processing is to be expected should the allocation failed.
>

I see, missed that. Thanks a lot for the explanation.

>>> + */
>>> +int __init efi_tpm_eventlog_init(void)
>>> +{
>>> +     struct linux_efi_tpm_eventlog *tbl;
>>> +     unsigned int tbl_size;
>>> +
>>
>> The functions efi_retrieve_tpm2_eventlog_1_2() and tpm_read_log_efi() are using
>> log_tbl as variable name, so I would use it here too for consistency.
>>
> 
> Done.
> 
>>> +     if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
>>> +             return 0;
>>> +
>>> +     tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
>>> +     if (!tbl) {
>>> +             pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
>>> +                     efi.tpm_log);
>>> +             return -ENOMEM;
>>> +     }
>>> +
>>
>> Same question than before, if this fails then the table + logs memory won't be
>> reserved but tpm_read_log_efi() will still try to access it. I'm not sure what
>> is the correct way to notify though, maybe setting efi.tpm_log to 0 and then in
>> tpm_read_log_efi() check efi.tpm_log for 0 or EFI_INVALID_TABLE_ADDR instead?
>>
> 
> That's right. To keep it simple, it might be easier to just set
> tpm_log to EFI_INVALID_TABLE_ADDR if that happens. Added in the next
> version of the patch set.
>

Right. I wasn't sure if you wanted to distinguish between the two cases, but that
is simpler indeed.

Best regards,
Jarkko Sakkinen - Sept. 18, 2017, 5:56 p.m.
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.sakkinen@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.sakkinen@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 <tweek@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-integrity@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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index a1686f3dc295..ef6abe8b3788 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -999,6 +999,7 @@  struct boot_params *efi_main(struct efi_config *c,
 
 	/* Ask the firmware to clear memory on unclean shutdown */
 	efi_enable_reset_attack_mitigation(sys_table);
+	efi_retrieve_tpm2_eventlog(sys_table);
 
 	setup_graphics(boot_params);
 
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 0329d319d89a..2f074b5cde87 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -10,7 +10,7 @@ 
 KASAN_SANITIZE_runtime-wrappers.o	:= n
 
 obj-$(CONFIG_ACPI_BGRT) 		+= efi-bgrt.o
-obj-$(CONFIG_EFI)			+= efi.o vars.o reboot.o memattr.o
+obj-$(CONFIG_EFI)			+= efi.o vars.o reboot.o memattr.o tpm.o
 obj-$(CONFIG_EFI)			+= capsule.o memmap.o
 obj-$(CONFIG_EFI_VARS)			+= efivars.o
 obj-$(CONFIG_EFI_ESRT)			+= esrt.o
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index f97f272e16ee..03fbaf8eb248 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -532,6 +532,8 @@  int __init efi_config_parse_tables(void *config_tables, int count, int sz,
 	if (efi_enabled(EFI_MEMMAP))
 		efi_memattr_init();
 
+	efi_tpm_eventlog_init();
+
 	/* Parse the EFI Properties table if it exists */
 	if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
 		efi_properties_table_t *tbl;
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index dedf9bde44db..2abe6d22dc5f 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -29,8 +29,7 @@  OBJECT_FILES_NON_STANDARD	:= y
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT			:= n
 
-lib-y				:= efi-stub-helper.o gop.o secureboot.o
-lib-$(CONFIG_RESET_ATTACK_MITIGATION) += tpm.o
+lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index 6224cdbc9669..da661bf8cb96 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -4,15 +4,18 @@ 
  * Copyright (C) 2016 CoreOS, Inc
  * Copyright (C) 2017 Google, Inc.
  *     Matthew Garrett <mjg59@google.com>
+ *     Thiebaud Weksteen <tweek@google.com>
  *
  * This file is part of the Linux kernel, and is made available under the
  * terms of the GNU General Public License version 2.
  */
 #include <linux/efi.h>
+#include <linux/tpm_eventlog.h>
 #include <asm/efi.h>
 
 #include "efistub.h"
 
+#ifdef CONFIG_RESET_ATTACK_MITIGATION
 static const efi_char16_t efi_MemoryOverWriteRequest_name[] = {
 	'M', 'e', 'm', 'o', 'r', 'y', 'O', 'v', 'e', 'r', 'w', 'r', 'i', 't',
 	'e', 'R', 'e', 'q', 'u', 'e', 's', 't', 'C', 'o', 'n', 't', 'r', 'o',
@@ -56,3 +59,81 @@  void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
 		    EFI_VARIABLE_BOOTSERVICE_ACCESS |
 		    EFI_VARIABLE_RUNTIME_ACCESS, sizeof(val), &val);
 }
+
+#endif
+
+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;
+	efi_status_t status;
+	efi_physical_addr_t log_location, log_last_entry;
+	struct linux_efi_tpm_eventlog *log_tbl;
+	unsigned long first_entry_addr, last_entry_addr;
+	size_t log_size, last_entry_size;
+	efi_bool_t truncated;
+	void *tcg2_protocol;
+
+	status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
+				&tcg2_protocol);
+	if (status != EFI_SUCCESS)
+		return;
+
+	status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol,
+				EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2,
+				&log_location, &log_last_entry, &truncated);
+	if (status != EFI_SUCCESS)
+		return;
+
+	if (!log_location)
+		return;
+	first_entry_addr = (unsigned long) log_location;
+
+	/*
+	 * We populate the EFI table even if the logs are empty.
+	 */
+	if (!log_last_entry) {
+		log_size = 0;
+	} else {
+		last_entry_addr = (unsigned long) log_last_entry;
+		/*
+		 * get_event_log only returns the address of the last entry.
+		 * We need to calculate its size to deduce the full size of
+		 * the logs.
+		 */
+		last_entry_size = sizeof(struct tcpa_event) +
+			((struct tcpa_event *) last_entry_addr)->event_size;
+		log_size = log_last_entry - log_location + last_entry_size;
+	}
+
+	/* Allocate space for the logs and copy them. */
+	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+				sizeof(*log_tbl) + log_size,
+				(void **) &log_tbl);
+
+	if (status != EFI_SUCCESS) {
+		efi_printk(sys_table_arg,
+			   "Unable to allocate memory for event log\n");
+		return;
+	}
+
+	memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
+	log_tbl->size = log_size;
+	log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+	memcpy(log_tbl->log, (void *) first_entry_addr, log_size);
+
+	status = efi_call_early(install_configuration_table,
+				&linux_eventlog_guid, log_tbl);
+	if (status != EFI_SUCCESS)
+		goto err_free;
+	return;
+
+err_free:
+	efi_call_early(free_pool, log_tbl);
+}
+
+void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg)
+{
+	/* Only try to retrieve the logs in 1.2 format. */
+	efi_retrieve_tpm2_eventlog_1_2(sys_table_arg);
+}
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
new file mode 100644
index 000000000000..07d217d68f47
--- /dev/null
+++ b/drivers/firmware/efi/tpm.c
@@ -0,0 +1,39 @@ 
+/*
+ * Copyright (C) 2017 Google, Inc.
+ *     Thiebaud Weksteen <tweek@google.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/efi.h>
+#include <linux/init.h>
+#include <linux/memblock.h>
+
+#include <asm/early_ioremap.h>
+
+/*
+ * Reserve the memory associated with the TPM Event Log configuration table.
+ */
+int __init efi_tpm_eventlog_init(void)
+{
+	struct linux_efi_tpm_eventlog *tbl;
+	unsigned int tbl_size;
+
+	if (efi.tpm_log == EFI_INVALID_TABLE_ADDR)
+		return 0;
+
+	tbl = early_memremap(efi.tpm_log, sizeof(*tbl));
+	if (!tbl) {
+		pr_err("Failed to map TPM Event Log table @ 0x%lx\n",
+			efi.tpm_log);
+		return -ENOMEM;
+	}
+
+	tbl_size = sizeof(*tbl) + tbl->size;
+	memblock_reserve(efi.tpm_log, tbl_size);
+	early_memunmap(tbl, sizeof(*tbl));
+	return 0;
+}
+
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 8dc3d94a3e3c..e671ea9a462e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -472,6 +472,39 @@  typedef struct {
 	u64 get_all;
 } apple_properties_protocol_64_t;
 
+typedef struct {
+	u32 get_capability;
+	u32 get_event_log;
+	u32 hash_log_extend_event;
+	u32 submit_command;
+	u32 get_active_pcr_banks;
+	u32 set_active_pcr_banks;
+	u32 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_32_t;
+
+typedef struct {
+	u64 get_capability;
+	u64 get_event_log;
+	u64 hash_log_extend_event;
+	u64 submit_command;
+	u64 get_active_pcr_banks;
+	u64 set_active_pcr_banks;
+	u64 get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_64_t;
+
+typedef u32 efi_tcg2_event_log_format;
+
+typedef struct {
+	void *get_capability;
+	efi_status_t (*get_event_log)(efi_handle_t, efi_tcg2_event_log_format,
+		efi_physical_addr_t *, efi_physical_addr_t *, efi_bool_t *);
+	void *hash_log_extend_event;
+	void *submit_command;
+	void *get_active_pcr_banks;
+	void *set_active_pcr_banks;
+	void *get_result_of_set_active_pcr_banks;
+} efi_tcg2_protocol_t;
+
 /*
  * Types and defines for EFI ResetSystem
  */
@@ -622,6 +655,7 @@  void efi_native_runtime_setup(void);
 #define EFI_MEMORY_ATTRIBUTES_TABLE_GUID	EFI_GUID(0xdcfa911d, 0x26eb, 0x469f,  0xa2, 0x20, 0x38, 0xb7, 0xdc, 0x46, 0x12, 0x20)
 #define EFI_CONSOLE_OUT_DEVICE_GUID		EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4,  0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 #define APPLE_PROPERTIES_PROTOCOL_GUID		EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb,  0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
+#define EFI_TCG2_PROTOCOL_GUID			EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID	EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID			EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
@@ -634,6 +668,7 @@  void efi_native_runtime_setup(void);
 #define LINUX_EFI_ARM_SCREEN_INFO_TABLE_GUID	EFI_GUID(0xe03fc20a, 0x85dc, 0x406e,  0xb9, 0x0e, 0x4a, 0xb5, 0x02, 0x37, 0x1d, 0x95)
 #define LINUX_EFI_LOADER_ENTRY_GUID		EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID	EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
+#define LINUX_EFI_TPM_EVENT_LOG_GUID		EFI_GUID(0xb7799cb0, 0xeca2, 0x4943,  0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
 
 typedef struct {
 	efi_guid_t guid;
@@ -1504,6 +1539,8 @@  static inline void
 efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg) { }
 #endif
 
+void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table);
+
 /*
  * Arch code can implement the following three template macros, avoiding
  * reptition for the void/non-void return cases of {__,}efi_call_virt():
@@ -1571,4 +1608,16 @@  struct linux_efi_random_seed {
 	u8	bits[];
 };
 
+
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 0x1
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2   0x2
+
+struct linux_efi_tpm_eventlog {
+	u32	size;
+	u8	version;
+	u8	log[];
+};
+
+extern int efi_tpm_eventlog_init(void);
+
 #endif /* _LINUX_EFI_H */