Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755501AbdC1QTQ (ORCPT ); Tue, 28 Mar 2017 12:19:16 -0400 Received: from goliath.siemens.de ([192.35.17.28]:57034 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754410AbdC1QTO (ORCPT ); Tue, 28 Mar 2017 12:19:14 -0400 Subject: Re: [PATCH v2 5/7] efi/capsule: Prepare for loading images with security header To: Ard Biesheuvel References: <47e87c8eccc0ee5c4033ece037c0bffc32782876.1490376860.git.jan.kiszka@siemens.com> <8d26028c-0c1f-82be-a8b1-588ab7222ccd@siemens.com> Cc: Matt Fleming , "linux-efi@vger.kernel.org" , Linux Kernel Mailing List , Andy Shevchenko , "Bryan O'Donoghue" , Hock Leong Kweh , Borislav Petkov , Sascha Weisenberger From: Jan Kiszka Message-ID: <1fa542ed-6638-bac1-64cb-72ed7132f9d1@siemens.com> Date: Tue, 28 Mar 2017 18:18:35 +0200 User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4063 Lines: 108 On 2017-03-28 17:52, Ard Biesheuvel wrote: > 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? > Devices that mandate a signed capsule for updates expect the CSH in front of the regular capsule image. The interface to UEFI remains unchanged, i.e. you pass the virtually chopped off capsule, but you have to leave the CSH in RAM right in front of that very same image. It's a "nice" side-channel API. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux