On 28 March 2017 at 18:17, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > On 28 March 2017 at 17:18, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote: >> On 2017-03-28 17:52, Ard Biesheuvel wrote: >>> On 28 March 2017 at 16:43, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> 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. >> > > Wow, that is worse than I thought. > > So my suggestion (which I coded up, please pull again), Hmm, it does not build on x86 atm. Let me fix that up first (~15 min) > is to replace > the array of struct page pointers with an array of physical addresses > in the capsule_info struct. This way, your special version of > efi_capsule_setup_info() can advance the first one by the size of the > header. > > I hope this works for you. I am not sure whether imagesize needs to be > modified, so I left it alone for now (but I suspect it should be). -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
