Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752604AbdC1Pwq (ORCPT ); Tue, 28 Mar 2017 11:52:46 -0400 Received: from mail-it0-f46.google.com ([209.85.214.46]:38175 "EHLO mail-it0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752431AbdC1Pwo (ORCPT ); Tue, 28 Mar 2017 11:52:44 -0400 MIME-Version: 1.0 In-Reply-To: <8d26028c-0c1f-82be-a8b1-588ab7222ccd@siemens.com> References: <47e87c8eccc0ee5c4033ece037c0bffc32782876.1490376860.git.jan.kiszka@siemens.com> <8d26028c-0c1f-82be-a8b1-588ab7222ccd@siemens.com> From: Ard Biesheuvel Date: Tue, 28 Mar 2017 16:52:18 +0100 Message-ID: Subject: Re: [PATCH v2 5/7] efi/capsule: Prepare for loading images with security header To: Jan Kiszka Cc: Matt Fleming , "linux-efi@vger.kernel.org" , Linux Kernel Mailing List , Andy Shevchenko , "Bryan O'Donoghue" , Hock Leong Kweh , Borislav Petkov , Sascha Weisenberger Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3503 Lines: 94 On 28 March 2017 at 16:43, Jan Kiszka wrote: > On 2017-03-28 17:13, Jan Kiszka wrote: >> On 2017-03-28 15:49, Ard Biesheuvel wrote: [..] >>> Could you please have a look at >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule >>> >>> and tell me if that would work for you? I will send them out for >>> proper review in any case, but to avoid confusion (if I missed >>> something obvious), I don't want to send them out just yet. >> >> There is more needed to make things work again, maybe around passing the >> right image size. I'm looking into this. >> > > This makes CSH images being accepted again: > > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c > index 4b6f93f..a4e2311 100644 > --- a/arch/x86/platform/efi/quirks.c > +++ b/arch/x86/platform/efi/quirks.c > @@ -562,6 +562,8 @@ int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff, > { > struct quark_security_header *csh = kbuff; > > + cap_info->total_size = 0; > + > if (!x86_match_cpu(quark_ids)) > goto fallback; > > @@ -587,12 +589,16 @@ int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff, > > kbuff += csh->headersize; > > + cap_info->total_size = csh->headersize; > + > fallback: > if (hdr_bytes < sizeof(efi_capsule_header_t)) > return 0; > > memcpy(&cap_info->header, kbuff, sizeof(cap_info->header)); > > + cap_info->total_size += cap_info->header.imagesize; > + > return __efi_capsule_setup_info(cap_info); > } > > diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c > index e851951..40dc354 100644 > --- a/drivers/firmware/efi/capsule-loader.c > +++ b/drivers/firmware/efi/capsule-loader.c > @@ -42,7 +42,7 @@ int __efi_capsule_setup_info(struct capsule_info *cap_info) > int ret; > void *temp_page; > > - pages_needed = ALIGN(cap_info->header.imagesize, PAGE_SIZE) / PAGE_SIZE; > + pages_needed = ALIGN(cap_info->total_size, PAGE_SIZE) / PAGE_SIZE; > > if (pages_needed == 0) { > pr_err("invalid capsule size"); > @@ -59,7 +59,6 @@ int __efi_capsule_setup_info(struct capsule_info *cap_info) > return ret; > } > > - cap_info->total_size = cap_info->header.imagesize; > temp_page = krealloc(cap_info->pages, > pages_needed * sizeof(void *), > GFP_KERNEL | __GFP_ZERO); > @@ -87,6 +86,8 @@ int __weak efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff, > > memcpy(&cap_info->header, kbuff, sizeof(cap_info->header)); > > + cap_info->total_size = cap_info->header.imagesize; > + > return __efi_capsule_setup_info(cap_info); > } > OK, thanks for debugging that. > But then my changes to efi_capsule_update are missing the present the > right format to the loader. As efi_capsule_update needs to lay out the > sg-list in as special way, excluding the CSH on the first page, it needs > to know about the displacement. > > Any suggestions how to address that without rolling back to my aproach? > OK, I'm a bit lost now: for my understanding, could you please reiterate how the CSH image deviates from the ordinary one? Or more specifically, what exactly is preventing us from simply chopping off the CSH header and pushing the capsule header + payload into /dev/capsule_loader?