Regression from efi: call get_event_log before ExitBootServices

[Jeremy Cline]

Hi folks,

Commit 33b6d03469b2 ("efi: call get_event_log before ExitBootServices")
causes my GP-electronic T701 tablet to hang when booting. Reverting the
patch series or hiding the TPM in the BIOS fixes the problem.

I've never fiddled with TPMs before so I'm not sure what what debugging
information to provide. It's got an Atom Z3735G and the UEFI firmware is
InsydeH20 version BYT70A.YNCHENG.WIN.007.


Regards,
Jeremy
--
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 0/9] KEYS: Blacklisting & UEFI database load

[Jiri Slaby]

On 11/16/2016, 07:10 PM, David Howells wrote:
> Here are two sets of patches.  Firstly, the first three patches provide a
> blacklist, making the following changes:
...
> Secondly, the remaining patches allow the UEFI database to be used to load
> the system keyrings:
...
> Dave Howells (2):
>   efi: Add EFI signature data types
>   efi: Add an EFI signature blob parser
> 
> David Howells (5):
>   KEYS: Add a system blacklist keyring
>   X.509: Allow X.509 certs to be blacklisted
>   PKCS#7: Handle blacklisted certificates
>   KEYS: Allow unrestricted boot-time addition of keys to secondary keyring
>   efi: Add SHIM and image security database GUID definitions
> 
> Josh Boyer (2):
>   MODSIGN: Import certificates from UEFI Secure Boot
>   MODSIGN: Allow the "db" UEFI variable to be suppressed

Hi,

what's the status of this please? Distributors (I checked SUSE, RedHat
and Ubuntu) have to carry these patches and every of them have to
forward-port the patches to new kernels. So are you going to resend the
PR to have this merged?

thanks,
-- 
js
suse labs
--
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] efi: Use efi_rts_workqueue to invoke EFI Runtime Services

[Mark Rutland]

On Mon, Mar 05, 2018 at 03:23:10PM -0800, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth 
> 
> Presently, efi_runtime_services() are executed by firmware in process
> context. To execute efi_runtime_service(), kernel switches the page
> directory from swapper_pgd to efi_pgd. However, efi_pgd doesn't have any
> user space mappings. A potential issue could be, for instance, an NMI
> interrupt (like perf) trying to profile some user data while in efi_pgd.

It might be worth pointing out that this could result in disaster (e.g.
if the frame pointer happens to point at MMIO in the EFI runtime
services mappings).

[...]

> pstore writes could potentially be invoked in interrupt context and it
> uses set_variable<>() and query_variable_info<>() to store logs. If we
> invoke efi_runtime_services() through efi_rts_wq while in atomic()
> kernel issues a warning ("scheduling wile in atomic") and prints stack
> trace. One way to overcome this is to not make the caller process wait
> for the worker thread to finish. This approach breaks pstore i.e. the
> log messages aren't written to efi variables. Hence, pstore calls
> efi_runtime_services() without using efi_rts_wq or in other words
> efi_rts_wq will be used unconditionally for all the
> efi_runtime_services() except set_variable<>() and
> query_variable_info<>()

Can't NMIs still come in during this?

... or do we assume that since something has already gone wrong, this
doesn't matter?

Otherwise, this doesn't seem to break basic stuff on arm64 platforms. I
can boot up, read the EFI RTC, and reboot. I ahd lockdep and KASAN
enabled, I received no splats from either.

FWIW:

Tested-by: Mark Rutland 

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

[Mark Rutland]

On Mon, Mar 05, 2018 at 03:23:09PM -0800, Sai Praneeth Prakhya wrote:
> @@ -329,6 +331,19 @@ static int __init efisubsys_init(void)
>   return 0;
>  
>   /*
> +  * Since we process only one efi_runtime_service() at a time, an
> +  * ordered workqueue (which creates only one execution context)
> +  * should suffice all our needs.
> +  */
> + efi_rts_wq = alloc_ordered_workqueue("efi_rts_workqueue", 0);
> + if (!efi_rts_wq) {
> + pr_err("Failed to create efi_rts_workqueue, EFI runtime 
> services "
> +"disabled.\n");
> + clear_bit(EFI_RUNTIME_SERVICES, );
> + return 0;
> + }

I'm a little worried that something might sample this flag between it
being set in an early_initcall (arm_enable_runtime_services), and
cleared in a subsys_initcall here.

However, nothing seems to do that so far, so maybe that's ok...

[...]

> +/* efi_runtime_service() function identifiers */
> +enum {
> + GET_TIME,
> + SET_TIME,
> + GET_WAKEUP_TIME,
> + SET_WAKEUP_TIME,
> + GET_VARIABLE,
> + GET_NEXT_VARIABLE,
> + SET_VARIABLE,
> + SET_VARIABLE_NONBLOCKING,
> + QUERY_VARIABLE_INFO,
> + QUERY_VARIABLE_INFO_NONBLOCKING,
> + GET_NEXT_HIGH_MONO_COUNT,
> + RESET_SYSTEM,
> + UPDATE_CAPSULE,
> + QUERY_CAPSULE_CAPS,
> +};

Can we please give this enum a name

[...]

> +/*
> + * efi_runtime_work: Details of EFI Runtime Service work
> + * @func:EFI Runtime Service function identifier
> + * @arg<1-5>:EFI Runtime Service function arguments
> + * @status:  Status of executing EFI Runtime Service
> + */
> +struct efi_runtime_work {
> + u8 func;

... and use it here rather than an opaque u8? I realise that means
placing the enum in .

> + void *arg1;
> + void *arg2;
> + void *arg3;
> + void *arg4;
> + void *arg5;
> + efi_status_t status;
> + struct work_struct work;
> +};

Thanks,
Mark.
--
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

[Thiebaud Weksteen]

On Mon, Mar 5, 2018 at 4:40 PM Marc-André Lureau

wrote:

> Hi Thiebaud

> On Wed, Sep 20, 2017 at 10:13 AM, 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.
> >

> Do you have plans to add support for the crypto-agile format? I am
> working on uefi/ovmf support, and I am wondering if it is at all
> necessary to add support for the 1.2 format. What do you think? I can
> eventually try to work on 2.0 format support.

Yes, this is definitely my intent. I am running low on free time for this
piece of work to happen just now though.

Thanks


> Thanks

> > Signed-off-by: Thiebaud Weksteen 
> > ---
> >  arch/x86/boot/compressed/eboot.c  |  1 +
> >  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 
> >  7 files changed, 174 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/firmware/efi/tpm.c
> >
> > 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..0308acfaaf76 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -52,6 +52,7 @@ struct efi __read_mostly efi = {
> > .properties_table   = EFI_INVALID_TABLE_ADDR,
> > .mem_attr_table = EFI_INVALID_TABLE_ADDR,
> > .rng_seed   = EFI_INVALID_TABLE_ADDR,
> > +   .tpm_log= EFI_INVALID_TABLE_ADDR
> >  };
> >  EXPORT_SYMBOL(efi);
> >
> > @@ -444,6 +445,7 @@ static __initdata efi_config_table_type_t
common_tables[] = {
> > {EFI_PROPERTIES_TABLE_GUID, "PROP", _table},
> > {EFI_MEMORY_ATTRIBUTES_TABLE_GUID, "MEMATTR",
_attr_table},
> > {LINUX_EFI_RANDOM_SEED_TABLE_GUID, "RNG", _seed},
> > +   {LINUX_EFI_TPM_EVENT_LOG_GUID, "TPMEventLog", _log},
> > {NULL_GUID, NULL, NULL},
> >  };
> >
> > @@ -532,6 +534,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

Re: [PATCH 0/2] ESRT fixes for relocatable kexec'd kernel

[AKASHI Takahiro]

Tyler, Jeffrey,

On Fri, Mar 02, 2018 at 08:27:11AM -0500, Tyler Baicar wrote:
> On 3/2/2018 12:53 AM, AKASHI Takahiro wrote:
> >Tyler, Jeffrey,
> >
> >[Note: This issue takes place in kexec, not kdump. So to be precise,
> >it is not the same phenomenon as what I addressed in [1],[2]:
> >   [1] 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-February/557254.html
> >   [2] 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553098.html
> >]
> >
> >On Thu, Mar 01, 2018 at 12:56:38PM -0500, Tyler Baicar wrote:
> >>Hello,
> >>
> >>On 2/28/2018 9:50 PM, AKASHI Takahiro wrote:
> >>>Hi,
> >>>
> >>>On Wed, Feb 28, 2018 at 08:39:42AM -0700, Jeffrey Hugo wrote:
> On 2/27/2018 11:19 PM, AKASHI Takahiro wrote:
> >Tyler,
> >
> ># I missed catching your patch as its subject doesn't contain arm64.
> >
> >On Fri, Feb 23, 2018 at 12:42:31PM -0700, Tyler Baicar wrote:
> >>Currently on arm64 ESRT memory does not appear to be properly blocked 
> >>off.
> >>Upon successful initialization, ESRT prints out the memory region that 
> >>it
> >>exists in like:
> >>
> >>esrt: Reserving ESRT space from 0x0a4c1c18 to 
> >>0x0a4c1cf0.
> >>
> >>But then by dumping /proc/iomem this region appears as part of System 
> >>RAM
> >>rather than being reserved:
> >>
> >>08f1-0dee : System RAM
> >>
> >>This causes issues when trying to kexec if the kernel is relocatable. 
> >>When
> >>kexec tries to execute, this memory can be selected to relocate the 
> >>kernel to
> >>which then overwrites all the ESRT information. Then when the kexec'd 
> >>kernel
> >>tries to initialize ESRT, it doesn't recognize the ESRT version number 
> >>and
> >>just returns from efi_esrt_init().
> >I'm not sure what is the root cause of your problem.
> >Do you have good confidence that the kernel (2nd kernel image in this 
> >case?)
> >really overwrite ESRT region?
> According to my debug, yes.
> Using JTAG, I was able to determine that the ESRT memory region was 
> getting
> overwritten by the secondary kernel in
> kernel/arch/arm64/kernel/relocate_kernel.S - specifically the "copy_page"
> line of arm64_relocate_new_kernel()
> 
> >To my best knowledge, kexec is carefully designed not to do such a thing
> >as it allocates a temporary buffer for kernel image and copies it to the
> >final destination at the very end of the 1st kernel.
> >
> >My guess is that kexec, or rather kexec-tools, tries to load the kernel 
> >image
> >at 0x8f8 (or 0x908?, not sure) in your case. It may or may not be
> >overlapped with ESRT.
> >(Try "-d" option when executing kexec command for confirmation.)
> With -d, I see
> 
> get_memory_ranges_iomem_cb: 09611000 - 0e5f : System 
> RAM
> 
> That overlaps the ESRT reservation -
> [ 0.00] esrt: Reserving ESRT space from 0x0b708718 to
> 0x0b7087f0
> 
> >Are you using initrd with kexec?
> Yes
> >>>To make the things clear, can you show me, if possible, the followings:
> >>I have attached all of these:
> >Many thanks.
> >According to the data, ESRT was overwritten by initrd, not the kernel image.
> >It doesn't matter to you though :)
> >
> >The solution would be, as Ard suggested, that more information be
> >added to /proc/iomem.
> >I'm going to fix the issue as quickly as possible.
> Great, thank you!! Please add us to the fix and we will gladly test it out.

I have created a workaround patch, attached below, as a kind of PoC.
Can you give it a go, please?
You need another patch for kexec-tools, too. See
https:/git.linaro.org/people/takahiro.akashi/kexecl-tools.git arm64/resv_mem

With this patch, extra entries for firmware-reserved memory resources,
which means any regions that are already reserved before arm64_memblock_init(),
or specifically efi/acpi tables in this case, are added to /proc/iomem.

 $ cat /proc/iomem (on my qemu+edk2 execution)
...
4000-5871 : System RAM
  4008-40f1 : Kernel code
  4104-411e9fff : Kernel data
  5440-583f : Crash kernel
  5859-585e : reserved
  5870-5871 : reserved
5872-58b5 : reserved
58b6-5be3 : System RAM
  58b61000-58b61fff : reserved
  59a7b118-59a7b667 : reserved
5be4-5bec : reserved
5bed-5bed : System RAM
  5bee-5bff : reserved
5c00-5fff : System RAM
  5ec0-5edf : reserved
80-ff : PCI Bus :00
  80-803fff : :00:01.0
80-803fff : virtio-pci-modern

While all the entries are currently marked as just "reserved," we'd better
give them more specific names for