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: <b4271434-3868-6014-1dfd-dde5c1381c9d@siemens.com>
Date:   Sun, 19 Feb 2017 17:52:37 -0800
From:   Jan Kiszka <jan.kiszka@...mens.com>
To:     "Bryan O'Donoghue" <pure.logic@...us-software.ie>,
        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 2017-02-19 17:33, Bryan O'Donoghue wrote:
> 
> 
> 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);
> +}
>

Good idea.

> 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.

Yes, I agree. I will look into this when I'm back from ELC next week (no
related hardware with me).

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux