linux-kernel - Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c1beeeb5-2359-d9c1-4759-b112c7d1a613@nexus-software.ie>
Date:   Mon, 20 Feb 2017 01:33:44 +0000
From:   Bryan O'Donoghue <pure.logic@...us-software.ie>
To:     Jan Kiszka <jan.kiszka@...mens.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:     "Kweh, Hock Leong" <hock.leong.kweh@...el.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bp@...en8.de>,
        Sascha Weisenberger <sascha.weisenberger@...mens.com>,
        Daniel Wagner <daniel.wagner@...mens.com>
Subject: Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark
 images



On 19/02/17 13:33, Jan Kiszka wrote:
>> I would not object strongly to having conditionally compiled code in
>> mainline that adds support for this, but bodging the default code path
>> like this for a Quark quirk is out of the question imo.
> I'm open for any consensus that avoids bending mainline too much and
> still helps us (and maybe also other Quark X1020 integrators) getting
> rid of additional patches.

We could make efi_capsule_setup_info() a weak symbol just like

drivers/firmware/efi/reboot.c:
bool __weak efi_poweroff_required(void)

that way Arm is none the wiser and we can bury the Quark Quirk in 
x86/platform/efi/quirks.c - where you're right Ard it arguably belongs - 
not in the core code.

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 30031d5..950663da 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -495,3 +495,19 @@ bool efi_poweroff_required(void)
  {
         return acpi_gbl_reduced_hardware || acpi_no_s5;
  }
+
+ssize_t csh_efi_capsule_setup_info(struct capsule_info *cap_info,
+                                  void *kbuff, size_t hdr_bytes)
+{
+       /* Code to deal with the CSH goes here */
+       return 0;
+}
+
+ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
+                              void *kbuff, size_t hdr_bytes)
+{
+       if (quark)
+               return csh_efi_capsule_setup_info(cap_info, kbuff, 
hdr_bytes);
+       else
+               return __efi_capsule_setup_info(cap_info, kbuff, hdr_bytes);
+}

diff --git a/drivers/firmware/efi/capsule-loader.c 
b/drivers/firmware/efi/capsule-loader.c
index 9ae6c11..d8bdc6f 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -53,7 +53,7 @@ static void efi_free_all_buff_pages(struct 
capsule_info *cap_info)
   * @kbuff: a mapped first page buffer pointer
   * @hdr_bytes: the total received number of bytes for efi header
   **/
-static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
+ssize_t __efi_capsule_setup_info(struct capsule_info *cap_info,
                                       void *kbuff, size_t hdr_bytes)
  {
         efi_capsule_header_t *cap_hdr;
@@ -98,6 +98,13 @@ static ssize_t efi_capsule_setup_info(struct 
capsule_info *cap_info,

         return 0;
  }
+EXPORT_SYMBOL_GPL(__efi_capsule_setup_info);
+
+ssize_t __weak efi_capsule_setup_info(struct capsule_info *cap_info,
+                                            void *kbuff, size_t hdr_bytes)
+{
+       return __efi_capsule_setup_info(cap_info, kbuff, hdr_bytes);
+}

One thing we want is to continue to have Quark work on ia32 builds 
without having to compile a Quark specific kernel just to get this 
feature working.

Jan I haven't had time to look at what you said about the BSP code not 
working with capsules on Gen2 (I will during the week though). If you 
currently have to strip the CSH to make this work then we're missing a 
trick on tip-of-tree and need to sort that out for the final version of 
this.

---
bod