On 12 March 2018 at 14:30, Jeremy Cline [off-list ref] wrote:

quoted

On 03/12/2018 07:08 AM, Ard Biesheuvel wrote:

quoted

On 10 March 2018 at 10:45, Thiebaud Weksteen [off-list ref] wrote:

quoted

On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline [off-list ref] wrote:

quoted

On Fri, Mar 09, 2018 at 10:43:50AM +0000, Thiebaud Weksteen wrote:

quoted

Thanks a lot for trying out the patch!

Please don't modify your install at this stage, I think we are hitting a
firmware bug and that would be awesome if we can fix how we are

handling it.

quoted

quoted

So, if we reach that stage in the function it could either be that:
* The allocation did not succeed, somehow, but the firmware still

returned

quoted

quoted

EFI_SUCCEED.
* The size requested is incorrect (I'm thinking something like a 1G of
log). This would be due to either a miscalculation of log_size

(possible)

quoted

quoted

or; the returned values of GetEventLog are not correct.
I'm sending a patch to add checks for these. Could you please apply and
retest?
Again, thanks for helping debugging this.

quoted

No problem, thanks for the help :)

quoted

With the new patch:

quoted

Locating the TCG2Protocol
Calling GetEventLog on TCG2Protocol
Log returned
log_location is not empty
log_size != 0
log_size < 1M
Allocating memory for storing the logs
Returned from memory allocation
Copying log to new location

quoted

And then it hangs. I added a couple more print statements:

quoted

diff --git a/drivers/firmware/efi/libstub/tpm.c

b/drivers/firmware/efi/libstub/tpm.c

quoted

index ee3fac109078..1ab5638bc50e 100644

--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c

@@ -148,8 +148,11 @@ void

efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)

quoted

         efi_printk(sys_table_arg, "Copying log to new location\n");

quoted

         memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
+       efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n");
         log_tbl->size = log_size;
+       efi_printk(sys_table_arg, "Set log_tbl->size\n");
         log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+       efi_printk(sys_table_arg, "Set log_tbl-version\n");
         memcpy(log_tbl->log, (void *) first_entry_addr, log_size);

quoted

         efi_printk(sys_table_arg, "Installing the log into the

configuration table\n");

quoted

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?

I am rather puzzled why the allocate_pool() should succeed and the
subsequent memset() should fail. This does not look like an issue that
is intimately related to TPM2 support, rather an issue in the firmware
that happens to get tickled after the change.

Would you mind trying replacing EFI_LOADER_DATA with
EFI_BOOT_SERVICES_DATA in the allocate_pool() call?

Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the
memset() call.

Could you try the following please? (attached as well in case gmail mangles it)

diff --git a/drivers/firmware/efi/libstub/tpm.c

b/drivers/firmware/efi/libstub/tpm.c
index 2298560cea72..30d960a344b7 100644

--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c

@@ -70,6 +70,8 @@ void

efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
        size_t log_size, last_entry_size;
        efi_bool_t truncated;
        void *tcg2_protocol;
+       unsigned long num_pages;
+       efi_physical_addr_t log_tbl_alloc;

        status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
                                &tcg2_protocol);

@@ -104,9 +106,12 @@ void

efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
        }

        /* 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);
+       num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE);
+       status = efi_call_early(allocate_pages,
+                               EFI_ALLOCATE_ANY_PAGES,
+                               EFI_LOADER_DATA,
+                               num_pages,
+                               &log_tbl_alloc);

        if (status != EFI_SUCCESS) {
                efi_printk(sys_table_arg,

@@ -114,6 +119,7 @@ void

efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
                return;
        }

+       log_tbl = (struct linux_efi_tpm_eventlog *)(unsigned long)log_tbl_alloc;
        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;

@@ -126,7 +132,7 @@ void

efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
        return;

 err_free:
-       efi_call_early(free_pool, log_tbl);
+       efi_call_early(free_pages, log_tbl_alloc, num_pages);
 }

 void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg)