Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1583018
| From | Bryan O'Donoghue <pure.logic@nexus-software.ie> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 2/2] efi/capsule: Add support for Quark security header |
| Date | 2017-02-17 02:40 +0100 |
| Message-ID | <tbFGF-607-3@gated-at.bofh.it> (permalink) |
| References | <tbclj-3oS-9@gated-at.bofh.it> <tbclk-3oS-29@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 15/02/17 18:14, Jan Kiszka wrote:
> The firmware for Quark X102x prepends a security header to the capsule
> which is needed to support the mandatory secure boot on this processor.
> The header can be detected by checking for the "_CSH" signature and -
> to avoid any GUID conflict - validating its size field to contain the
> expected value. Then we need to look for the EFI header right after the
> security header and pass the image offset to efi_capsule_update while
> keeping the whole image in RAM - the firmware will look for the header
> on its own.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> drivers/firmware/efi/capsule-loader.c | 73 ++++++++++++++++++++++++++++++-----
> 1 file changed, 63 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
> index 63ceca9..571d931 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -26,10 +26,32 @@ struct capsule_info {
> long index;
> size_t count;
> size_t total_size;
> + unsigned int efi_hdr_offset;
> struct page **pages;
> size_t page_bytes_remain;
> };
>
> +#define QUARK_CSH_SIGNATURE 0x5f435348 /* _CSH */
> +#define QUARK_SECURITY_HEADER_SIZE 0x400
> +
> +struct efi_quark_security_header {
> + u32 csh_signature;
> + u32 version;
> + u32 modulesize;
> + u32 security_version_number_index;
> + u32 security_version_number;
> + u32 rsvd_module_id;
> + u32 rsvd_module_vendor;
> + u32 rsvd_date;
> + u32 headersize;
> + u32 hash_algo;
> + u32 cryp_algo;
> + u32 keysize;
> + u32 signaturesize;
> + u32 rsvd_next_header;
> + u32 rsvd[2];
> +};
This is a real nitpick (sorry) - but it'd be nice to have a document
reference or a link to describe this header i.e. it is officially
documented - outside of the UEFI specification. Make life easy for
someone reading this header and make an document reference.
Also it'd be appreciated if you could describe the format of the
structure with
@member member-attribute description
> +
> /**
> * efi_free_all_buff_pages - free all previous allocated buffer pages
> * @cap_info: pointer to current instance of capsule_info structure
> @@ -56,18 +78,46 @@ static void efi_free_all_buff_pages(struct capsule_info *cap_info)
> static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
> void *kbuff, size_t hdr_bytes)
> {
> + struct efi_quark_security_header *quark_hdr;
> efi_capsule_header_t *cap_hdr;
> size_t pages_needed;
> int ret;
> void *temp_page;
>
> - /* Only process data block that is larger than efi header size */
> - if (hdr_bytes < sizeof(efi_capsule_header_t))
> + /* Only process data block that is larger than the security header
> + * (which is larger than the EFI header) */
> + if (hdr_bytes < sizeof(struct efi_quark_security_header))
> return 0;
>
> /* Reset back to the correct offset of header */
> cap_hdr = kbuff - cap_info->count;
> - pages_needed = ALIGN(cap_hdr->imagesize, PAGE_SIZE) >> PAGE_SHIFT;
> +
> + quark_hdr = (struct efi_quark_security_header *)cap_hdr;
> +
> + if (quark_hdr->csh_signature == QUARK_CSH_SIGNATURE &&
> + quark_hdr->headersize == QUARK_SECURITY_HEADER_SIZE) {
> + /* Only process data block if EFI header is included */
> + if (hdr_bytes < QUARK_SECURITY_HEADER_SIZE +
> + sizeof(efi_capsule_header_t))
> + return 0;
At this point if cap_info->header_obtained == false then this is an
error - you should be barfing on this - not literally barfing - at least
not on your keyboard :)
return -ETHISHEADERSUCKS or some other sensible value -EINVAL like you
have below.
Point being you've validated the signature, the header size and
cap_info->header_obtained is false then you definitely have a bogus
capsule..
> +
> + pr_debug("%s: Quark security header detected\n", __func__);
... and %s __func__ is verboten don't do it. actually there's a bunch of
those pairs all over this code - if you have the time in a supplementary
patch please kill them - there must be a a dev pointer we can get at
somewhere that makes sense to use dev_dbg- if not those __func__
parameters still need to go away - please kill them.
> +
> + if (quark_hdr->rsvd_next_header != 0) {
> + pr_err("%s: multiple security headers not supported\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + cap_hdr = (void *)cap_hdr + quark_hdr->headersize;
You could have a separate void * variable and not have the cast.
---
bod
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-15 19:20 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-15 19:20 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-15 19:50 +0100
[PATCH 1/2] efi/capsule: Prepare for loading images with security header Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-15 19:20 +0100
[PATCH 2/2] efi/capsule: Add support for Quark security header Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-15 19:20 +0100
Re: [PATCH 2/2] efi/capsule: Add support for Quark security header Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-17 02:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-15 19:50 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-15 19:50 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-15 20:00 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-15 20:10 +0100
RE: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images "Kweh, Hock Leong" <hock.leong.kweh@intel.com> - 2017-02-16 04:10 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-16 08:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-18 23:00 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-19 14:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-20 02:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-20 03:00 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-17 02:00 +0100
RE: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images "Kweh, Hock Leong" <hock.leong.kweh@intel.com> - 2017-02-17 09:30 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-17 10:30 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Matt Fleming <matt@codeblueprint.co.uk> - 2017-02-28 13:20 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-28 13:30 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Matt Fleming <matt@codeblueprint.co.uk> - 2017-02-28 13:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-28 14:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-28 16:10 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-28 16:20 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-28 17:30 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-28 18:20 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-28 19:50 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-28 18:30 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-28 14:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-28 14:40 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-17 11:00 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-02-17 11:20 +0100
Re: [PATCH 0/2] efi: Enhance capsule loader to support signed Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-02-17 12:50 +0100
csiph-web