Re: 5.3 boot regression caused by 5.3 TPM changes

2019-08-04 Thread Hans de Goede

Hi,

On 04-08-19 17:33, Ard Biesheuvel wrote:

Hi Hans,

On Sun, 4 Aug 2019 at 13:00, Hans de Goede  wrote:


Hi All,

While testing 5.3-rc2 on an Irbis TW90 Intel Cherry Trail based
tablet I noticed that it does not boot on this device.

A git bisect points to commit 166a2809d65b ("tpm: Don't duplicate
events from the final event log in the TCG2 log")

And I can confirm that reverting just that single commit makes
the TW90 boot again.

This machine uses AptIO firmware with base component versions
of: UEFI 2.4 PI 1.3. I've tried to reproduce the problem on
a Teclast X80 Pro which is also CHT based and also uses AptIO
firmware with the same base components. But it does not reproduce
there. Neither does the problem reproduce on a CHT tablet using
InsideH20 based firmware.

Note that these devices have a software/firmware TPM-2.0
implementation, they do not have an actual TPM chip.

Comparing TPM firmware setting between the 2 AptIO based
tablets the settings are identical, but the troublesome
TW90 does have some more setting then the X80, it has
the following settings which are not shown on the X80:

Active PCR banks:   SHA-1 (read only)
Available PCR banks:SHA-1,SHA256  (read only)
TPM2.0 UEFI SPEC version:   TCG_2 (other possible setting: TCG_1_2
Physical Presence SPEC ver: 1.2   (other possible setting: 1.3)

I have the feeling that at least the first 2 indicate that
the previous win10 installation has actually used the
TPM, where as on the X80 the TPM is uninitialized.
Note this is just a hunch I could be completely wrong.

I would be happy to run any commands to try and debug this
or to build a kernel with some patches to gather more info.

Note any kernel patches to printk some debug stuff need
to be based on 5.3 with 166a2809d65b reverted, without that
reverted the device will not boot, and thus I cannot collect
logs without it reverted.



Are you booting a 64-bit kernel on 32-bit firmware?


Yes you are right, I must say that this is somewhat surprising
most Cherry Trail devices do use 64 bit firmware (where as Bay Trail
typically uses 32 bit). But I just checked efibootmgr output and it
says it is booting: \EFI\FEDORA\SHIMIA32.EFI so yeah 32 bit firmware.

Recent Fedora releases take care of this so seamlessly I did not
even realize...

Regards,

Hans


Re: 5.3 boot regression caused by 5.3 TPM changes

2019-08-04 Thread Ard Biesheuvel
Hi Hans,

On Sun, 4 Aug 2019 at 13:00, Hans de Goede  wrote:
>
> Hi All,
>
> While testing 5.3-rc2 on an Irbis TW90 Intel Cherry Trail based
> tablet I noticed that it does not boot on this device.
>
> A git bisect points to commit 166a2809d65b ("tpm: Don't duplicate
> events from the final event log in the TCG2 log")
>
> And I can confirm that reverting just that single commit makes
> the TW90 boot again.
>
> This machine uses AptIO firmware with base component versions
> of: UEFI 2.4 PI 1.3. I've tried to reproduce the problem on
> a Teclast X80 Pro which is also CHT based and also uses AptIO
> firmware with the same base components. But it does not reproduce
> there. Neither does the problem reproduce on a CHT tablet using
> InsideH20 based firmware.
>
> Note that these devices have a software/firmware TPM-2.0
> implementation, they do not have an actual TPM chip.
>
> Comparing TPM firmware setting between the 2 AptIO based
> tablets the settings are identical, but the troublesome
> TW90 does have some more setting then the X80, it has
> the following settings which are not shown on the X80:
>
> Active PCR banks:   SHA-1 (read only)
> Available PCR banks:SHA-1,SHA256  (read only)
> TPM2.0 UEFI SPEC version:   TCG_2 (other possible setting: TCG_1_2
> Physical Presence SPEC ver: 1.2   (other possible setting: 1.3)
>
> I have the feeling that at least the first 2 indicate that
> the previous win10 installation has actually used the
> TPM, where as on the X80 the TPM is uninitialized.
> Note this is just a hunch I could be completely wrong.
>
> I would be happy to run any commands to try and debug this
> or to build a kernel with some patches to gather more info.
>
> Note any kernel patches to printk some debug stuff need
> to be based on 5.3 with 166a2809d65b reverted, without that
> reverted the device will not boot, and thus I cannot collect
> logs without it reverted.
>

Are you booting a 64-bit kernel on 32-bit firmware?


5.3 boot regression caused by 5.3 TPM changes

2019-08-04 Thread Hans de Goede

Hi All,

While testing 5.3-rc2 on an Irbis TW90 Intel Cherry Trail based
tablet I noticed that it does not boot on this device.

A git bisect points to commit 166a2809d65b ("tpm: Don't duplicate
events from the final event log in the TCG2 log")

And I can confirm that reverting just that single commit makes
the TW90 boot again.

This machine uses AptIO firmware with base component versions
of: UEFI 2.4 PI 1.3. I've tried to reproduce the problem on
a Teclast X80 Pro which is also CHT based and also uses AptIO
firmware with the same base components. But it does not reproduce
there. Neither does the problem reproduce on a CHT tablet using
InsideH20 based firmware.

Note that these devices have a software/firmware TPM-2.0
implementation, they do not have an actual TPM chip.

Comparing TPM firmware setting between the 2 AptIO based
tablets the settings are identical, but the troublesome
TW90 does have some more setting then the X80, it has
the following settings which are not shown on the X80:

Active PCR banks:   SHA-1 (read only)
Available PCR banks:SHA-1,SHA256  (read only)
TPM2.0 UEFI SPEC version:   TCG_2 (other possible setting: TCG_1_2
Physical Presence SPEC ver: 1.2   (other possible setting: 1.3)

I have the feeling that at least the first 2 indicate that
the previous win10 installation has actually used the
TPM, where as on the X80 the TPM is uninitialized.
Note this is just a hunch I could be completely wrong.

I would be happy to run any commands to try and debug this
or to build a kernel with some patches to gather more info.

Note any kernel patches to printk some debug stuff need
to be based on 5.3 with 166a2809d65b reverted, without that
reverted the device will not boot, and thus I cannot collect
logs without it reverted.

Regards,

Hans


Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base

2019-08-04 Thread Ard Biesheuvel
Hello Chester,

On Fri, 2 Aug 2019 at 08:40, Chester Lin  wrote:
>
> In some cases the arm32 efistub could fail to allocate memory for
> uncompressed kernel. For example, we got the following error message when
> verifying EFI stub on Raspberry Pi-2 [kernel-5.2.1 + grub-2.04] :
>
>   EFI stub: Booting Linux Kernel...
>   EFI stub: ERROR: Unable to allocate memory for uncompressed kernel.
>   EFI stub: ERROR: Failed to relocate kernel
>
> After checking the EFI memory map we found that the first page [0 - 0xfff]
> had been reserved by Raspberry Pi-2's firmware, and the efistub tried to
> set the dram base at 0, which was actually in a reserved region.
>

This by itself is a violation of the Linux boot protocol for 32-bit
ARM when using the decompressor. The decompressor rounds down its own
base address to a multiple of 128 MB, and assumes the whole area is
available for the decompressed kernel and related data structures.
(The first TEXT_OFFSET bytes are no longer used in practice, which is
why putting a reserved region of 4 KB bytes works at the moment, but
this is fragile). Note that the decompressor does not look at any DT
or EFI provided memory maps *at all*.

So unfortunately, this is not something we can fix in the kernel, but
we should fix it in the bootloader or in GRUB, so it does not put any
reserved regions in the first 128 MB of memory,


>   grub> lsefimmap
>   Type  Physical start  - end #PagesSize Attributes
>   reserved  -0fff 0001  4KiB WB
>   conv-mem  1000-07ef5fff 7ef5 130004KiB WB
>   RT-data   07ef6000-07f09fff 0014 80KiB RT WB
>   conv-mem  07f0a000-2d871fff 00025968 615840KiB WB
>   .
>
> To avoid a reserved address, we have to ignore the memory regions which are
> marked as EFI_RESERVED_TYPE, and only conventional memory regions can be
> chosen. If the region before the kernel base is unaligned, it will be
> marked as EFI_RESERVED_TYPE and let kernel ignore it so that memblock_limit
> will not be sticked with a very low address such as 0x1000.
>
> Signed-off-by: Chester Lin 
> ---
>  arch/arm/mm/mmu.c |  3 ++
>  drivers/firmware/efi/libstub/arm32-stub.c | 43 ++-
>  2 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index f3ce34113f89..909b11ba48d8 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> phys_addr_t block_start = reg->base;
> phys_addr_t block_end = reg->base + reg->size;
>
> +   if (memblock_is_nomap(reg))
> +   continue;
> +
> if (reg->base < vmalloc_limit) {
> if (block_end > lowmem_limit)
> /*
> diff --git a/drivers/firmware/efi/libstub/arm32-stub.c 
> b/drivers/firmware/efi/libstub/arm32-stub.c
> index e8f7aefb6813..10d33d36df00 100644
> --- a/drivers/firmware/efi/libstub/arm32-stub.c
> +++ b/drivers/firmware/efi/libstub/arm32-stub.c
> @@ -128,7 +128,7 @@ static efi_status_t 
> reserve_kernel_base(efi_system_table_t *sys_table_arg,
>
> for (l = 0; l < map_size; l += desc_size) {
> efi_memory_desc_t *desc;
> -   u64 start, end;
> +   u64 start, end, spare, kernel_base;
>
> desc = (void *)memory_map + l;
> start = desc->phys_addr;
> @@ -144,27 +144,52 @@ static efi_status_t 
> reserve_kernel_base(efi_system_table_t *sys_table_arg,
> case EFI_BOOT_SERVICES_DATA:
> /* Ignore types that are released to the OS anyway */
> continue;
> -
> +   case EFI_RESERVED_TYPE:
> +   /* Ignore reserved regions */
> +   continue;
> case EFI_CONVENTIONAL_MEMORY:
> /*
>  * Reserve the intersection between this entry and the
>  * region.
>  */
> start = max(start, (u64)dram_base);
> -   end = min(end, (u64)dram_base + 
> MAX_UNCOMP_KERNEL_SIZE);
> +   kernel_base = round_up(start, PMD_SIZE);
> +   spare = kernel_base - start;
> +   end = min(end, kernel_base + MAX_UNCOMP_KERNEL_SIZE);
> +
> +   status = efi_call_early(allocate_pages,
> +   EFI_ALLOCATE_ADDRESS,
> +   EFI_LOADER_DATA,
> +   MAX_UNCOMP_KERNEL_SIZE / 
> EFI_PAGE_SIZE,
> +   _base);
> +   if (status != EFI_SUCCESS) {
> +   pr_efi_err(sys_table_arg,
> +