Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1608707 > unrolled thread

[PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images

Started byJan Kiszka <jan.kiszka@siemens.com>
First post2017-03-24 18:40 +0100
Last post2017-03-27 13:10 +0200
Articles 18 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
    [PATCH v2 4/7] efi/capsule: Adjust return type of efi_capsule_setup_info Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
      Re: [PATCH v2 4/7] efi/capsule: Adjust return type of efi_capsule_setup_info Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-03-24 19:50 +0100
    [PATCH v2 6/7] efi/capsule: Factor out overloadable efi_capsule_identify_image Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
    [PATCH v2 5/7] efi/capsule: Prepare for loading images with security header Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
      Re: [PATCH v2 5/7] efi/capsule: Prepare for loading images with  security header Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-24 21:30 +0100
    [PATCH v2 3/7] efi/capsule: Clean up pr_err/info messages Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
      Re: [PATCH v2 3/7] efi/capsule: Clean up pr_err/info messages Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-03-24 19:20 +0100
    [PATCH v2 2/7] efi/capsule: Remove pr_debug on ENOMEM or EFAULT Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
      Re: [PATCH v2 2/7] efi/capsule: Remove pr_debug on ENOMEM or EFAULT Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-03-24 19:20 +0100
    [PATCH v2 1/7] efi/capsule: Fix return code on failing kmap/vmap Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
      Re: [PATCH v2 1/7] efi/capsule: Fix return code on failing kmap/vmap Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-03-24 19:20 +0100
    [PATCH v2 7/7] efi/capsule: Add support for Quark security header Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-24 18:40 +0100
      Re: [PATCH v2 7/7] efi/capsule: Add support for Quark security header Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-24 21:40 +0100
    Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed  Quark images Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-03-24 21:50 +0100
      Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed  Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-27 13:30 +0200
    Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed  Quark images Bryan O'Donoghue <pure.logic@nexus-software.ie> - 2017-03-27 12:50 +0200
      Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed  Quark images Jan Kiszka <jan.kiszka@siemens.com> - 2017-03-27 13:10 +0200

#1608707 — [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images
Message-ID<toBlT-4TS-1@gated-at.bofh.it>
This addresses the review feedback provided on round 1, specifically
- refactored queue to keep the Quark mess in 
- only check for CSH on Quark CPUs
- added some smaller cleanups of the capsule loader
- documented capsule header / linked to original code

See last patch for the background of the series.

The series has been tested on the Galileo Gen2, to exclude regressions,
with a firmware.cap with AND without security header and the SIMATIC
IOT2040 which requires the header because of its mandatory secure boot.

Jan

Jan Kiszka (7):
  efi/capsule: Fix return code on failing kmap/vmap
  efi/capsule: Remove pr_debug on ENOMEM or EFAULT
  efi/capsule: Clean up pr_err/info messages
  efi/capsule: Adjust return type of efi_capsule_setup_info
  efi/capsule: Prepare for loading images with security header
  efi/capsule: Factor out overloadable efi_capsule_identify_image
  efi/capsule: Add support for Quark security header

 arch/x86/platform/efi/quirks.c        |  95 ++++++++++++++++++++++++
 drivers/firmware/efi/capsule-loader.c | 136 +++++++++++++++++++---------------
 drivers/firmware/efi/capsule.c        |  21 +++++-
 include/linux/efi.h                   |  19 +++++
 4 files changed, 208 insertions(+), 63 deletions(-)

-- 
2.10.2

[toc] | [next] | [standalone]

#1608708 — [PATCH v2 4/7] efi/capsule: Adjust return type of efi_capsule_setup_info

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 4/7] efi/capsule: Adjust return type of efi_capsule_setup_info
Message-ID<toBlU-4TS-3@gated-at.bofh.it>
In reply to#1608707
We actually expect int at the caller and never return any size
information.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/firmware/efi/capsule-loader.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 3fb91e1..37d3f6e 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -53,8 +53,8 @@ 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,
-				      void *kbuff, size_t hdr_bytes)
+static int efi_capsule_setup_info(struct capsule_info *cap_info,
+				  void *kbuff, size_t hdr_bytes)
 {
 	efi_capsule_header_t *cap_hdr;
 	size_t pages_needed;
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1608873 — Re: [PATCH v2 4/7] efi/capsule: Adjust return type of efi_capsule_setup_info

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-03-24 19:50 +0100
SubjectRe: [PATCH v2 4/7] efi/capsule: Adjust return type of efi_capsule_setup_info
Message-ID<toCrF-5z0-33@gated-at.bofh.it>
In reply to#1608708
On 24 March 2017 at 17:34, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> We actually expect int at the caller and never return any size
> information.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  drivers/firmware/efi/capsule-loader.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
> index 3fb91e1..37d3f6e 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -53,8 +53,8 @@ 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,
> -                                     void *kbuff, size_t hdr_bytes)
> +static int efi_capsule_setup_info(struct capsule_info *cap_info,
> +                                 void *kbuff, size_t hdr_bytes)
>  {
>         efi_capsule_header_t *cap_hdr;
>         size_t pages_needed;
> --
> 2.10.2
>

[toc] | [prev] | [next] | [standalone]

#1608709 — [PATCH v2 6/7] efi/capsule: Factor out overloadable efi_capsule_identify_image

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 6/7] efi/capsule: Factor out overloadable efi_capsule_identify_image
Message-ID<toBlU-4TS-5@gated-at.bofh.it>
In reply to#1608707
Another step to prepare Quark's CSH capsule format: Factor out the weak
efi_capsule_identify_image function which is supposed to tell standard-
conforming images apart from the special ones. The conforming version of
it is __efi_capsule_identify_image, and that is called unless
efi_capsule_identify_image is overloaded by an architecture-specific
quirk implementation.

efi_capsule_setup_info calls the image identification callback and is
prepared for the case, efi_hdr_displacement becomes > 0 and the total
image size > than what the standard EFI header reports.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/firmware/efi/capsule-loader.c | 89 ++++++++++++++++++++++-------------
 include/linux/efi.h                   | 18 +++++++
 2 files changed, 73 insertions(+), 34 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 59e2694..50cacd4 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -20,26 +20,15 @@
 
 #define NO_FURTHER_WRITE_ACTION -1
 
-struct capsule_info {
-	bool		header_obtained;
-	int		reset_type;
-	long		index;
-	size_t		count;
-	size_t		total_size;
-	unsigned int	efi_hdr_displacement;
-	struct page	**pages;
-	size_t		page_bytes_remain;
-};
-
 /**
  * efi_free_all_buff_pages - free all previous allocated buffer pages
- * @cap_info: pointer to current instance of capsule_info structure
+ * @cap_info: pointer to current instance of efi_capsule_info structure
  *
  *	In addition to freeing buffer pages, it flags NO_FURTHER_WRITE_ACTION
  *	to cease processing data in subsequent write(2) calls until close(2)
  *	is called.
  **/
-static void efi_free_all_buff_pages(struct capsule_info *cap_info)
+static void efi_free_all_buff_pages(struct efi_capsule_info *cap_info)
 {
 	while (cap_info->index > 0)
 		__free_page(cap_info->pages[--cap_info->index]);
@@ -47,28 +36,63 @@ static void efi_free_all_buff_pages(struct capsule_info *cap_info)
 	cap_info->index = NO_FURTHER_WRITE_ACTION;
 }
 
+int __efi_capsule_identify_image(struct efi_capsule_info *cap_info,
+				 void *header, size_t hdr_bytes)
+{
+	efi_capsule_header_t *cap_hdr = header;
+
+	/* Only process data block that is larger than efi header size */
+	if (hdr_bytes < sizeof(efi_capsule_header_t))
+		return 0;
+
+	cap_info->total_size = cap_hdr->imagesize;
+	cap_info->efi_hdr_displacement = 0;
+
+	return 1;
+}
+
+/**
+ * efi_capsule_identify_image - identify the capsule image layout and initialize
+ * 				efi_capsule_info fields accordingly
+ * @cap_info: pointer to current instance of efi_capsule_info structure
+ * @header: mapped image header
+ * @hdr_bytes: the total received number of bytes for header
+ *
+ * Return 1 on success, 0 if insufficient data was read so far, otherwise
+ * negative error code.
+ */
+int __weak efi_capsule_identify_image(struct efi_capsule_info *cap_info,
+				      void *header, size_t hdr_bytes)
+{
+	return __efi_capsule_identify_image(cap_info, header, hdr_bytes);
+}
+
 /**
  * efi_capsule_setup_info - obtain the efi capsule header in the binary and
- *			    setup capsule_info structure
- * @cap_info: pointer to current instance of capsule_info structure
+ *			    setup efi_capsule_info structure
+ * @cap_info: pointer to current instance of efi_capsule_info structure
  * @kbuff: a mapped first page buffer pointer
  * @hdr_bytes: the total received number of bytes for efi header
  **/
-static int efi_capsule_setup_info(struct capsule_info *cap_info,
+static int efi_capsule_setup_info(struct efi_capsule_info *cap_info,
 				  void *kbuff, size_t hdr_bytes)
 {
 	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))
-		return 0;
+	void *header;
+	int ret;
 
 	/* Reset back to the correct offset of header */
-	cap_hdr = kbuff - cap_info->count;
-	pages_needed = ALIGN(cap_hdr->imagesize, PAGE_SIZE) >> PAGE_SHIFT;
+	header = kbuff - cap_info->count;
+
+	ret = efi_capsule_identify_image(cap_info, header, hdr_bytes);
+	if (ret <= 0)
+		return ret;
+
+	cap_hdr = header + cap_info->efi_hdr_displacement;
+
+	pages_needed = ALIGN(cap_info->total_size, PAGE_SIZE) >> PAGE_SHIFT;
 
 	if (pages_needed == 0) {
 		pr_err("invalid capsule size");
@@ -77,16 +101,13 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
 
 	/* Check if the capsule binary supported */
 	ret = efi_capsule_supported(cap_hdr->guid, cap_hdr->flags,
-				    cap_hdr->imagesize,
+				    cap_info->total_size,
 				    &cap_info->reset_type);
 	if (ret) {
 		pr_err("capsule not supported\n");
 		return ret;
 	}
 
-	cap_info->efi_hdr_displacement = 0;
-
-	cap_info->total_size = cap_hdr->imagesize;
 	temp_page = krealloc(cap_info->pages,
 			     pages_needed * sizeof(void *),
 			     GFP_KERNEL | __GFP_ZERO);
@@ -102,9 +123,9 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
 /**
  * efi_capsule_submit_update - invoke the efi_capsule_update API once binary
  *			       upload done
- * @cap_info: pointer to current instance of capsule_info structure
+ * @cap_info: pointer to current instance of efi_capsule_info structure
  **/
-static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
+static ssize_t efi_capsule_submit_update(struct efi_capsule_info *cap_info)
 {
 	efi_capsule_header_t *cap_hdr;
 	void *mapped_pages;
@@ -157,7 +178,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 				 size_t count, loff_t *offp)
 {
 	int ret = 0;
-	struct capsule_info *cap_info = file->private_data;
+	struct efi_capsule_info *cap_info = file->private_data;
 	struct page *page;
 	void *kbuff = NULL;
 	size_t write_byte;
@@ -244,7 +265,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 static int efi_capsule_flush(struct file *file, fl_owner_t id)
 {
 	int ret = 0;
-	struct capsule_info *cap_info = file->private_data;
+	struct efi_capsule_info *cap_info = file->private_data;
 
 	if (cap_info->index > 0) {
 		pr_err("capsule upload not complete\n");
@@ -265,7 +286,7 @@ static int efi_capsule_flush(struct file *file, fl_owner_t id)
  **/
 static int efi_capsule_release(struct inode *inode, struct file *file)
 {
-	struct capsule_info *cap_info = file->private_data;
+	struct efi_capsule_info *cap_info = file->private_data;
 
 	kfree(cap_info->pages);
 	kfree(file->private_data);
@@ -278,14 +299,14 @@ static int efi_capsule_release(struct inode *inode, struct file *file)
  * @inode: not used
  * @file: file pointer
  *
- *	Will allocate each capsule_info memory for each file open call.
+ *	Will allocate each efi_capsule_info memory for each file open call.
  *	This provided the capability to support multiple file open feature
  *	where user is not needed to wait for others to finish in order to
  *	upload their capsule binary.
  **/
 static int efi_capsule_open(struct inode *inode, struct file *file)
 {
-	struct capsule_info *cap_info;
+	struct efi_capsule_info *cap_info;
 
 	cap_info = kzalloc(sizeof(*cap_info), GFP_KERNEL);
 	if (!cap_info)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index d83095c6..5561817 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1397,6 +1397,18 @@ int efivars_sysfs_init(void);
 #define EFIVARS_DATA_SIZE_MAX 1024
 
 #endif /* CONFIG_EFI_VARS */
+
+struct efi_capsule_info {
+	bool		header_obtained;
+	int		reset_type;
+	long		index;
+	size_t		count;
+	size_t		total_size;
+	unsigned int	efi_hdr_displacement;
+	struct page	**pages;
+	size_t		page_bytes_remain;
+};
+
 extern bool efi_capsule_pending(int *reset_type);
 
 extern int efi_capsule_supported(efi_guid_t guid, u32 flags,
@@ -1406,6 +1418,12 @@ extern int efi_capsule_update(efi_capsule_header_t *capsule,
 			      unsigned int efi_hdr_displacement,
 			      struct page **pages);
 
+int __efi_capsule_identify_image(struct efi_capsule_info *cap_info,
+				 void *header, size_t hdr_bytes);
+
+int efi_capsule_identify_image(struct efi_capsule_info *cap_info, void *header,
+			       size_t hdr_bytes);
+
 #ifdef CONFIG_EFI_RUNTIME_MAP
 int efi_runtime_map_init(struct kobject *);
 int efi_get_runtime_map_size(void);
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1608710 — [PATCH v2 5/7] efi/capsule: Prepare for loading images with security header

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 5/7] efi/capsule: Prepare for loading images with security header
Message-ID<toBlU-4TS-7@gated-at.bofh.it>
In reply to#1608707
The Quark security header is nicely located in front of the capsule
image, but we still need to pass the image to the update service as if
there was none. Prepare efi_capsule_update and its user for this by
defining and evaluating a EFI header displacement in the image located
in memory. For standard-conforming capsules, this displacement is 0.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/firmware/efi/capsule-loader.c | 19 +++++++++++++------
 drivers/firmware/efi/capsule.c        | 21 +++++++++++++++++----
 include/linux/efi.h                   |  1 +
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 37d3f6e..59e2694 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -26,6 +26,7 @@ struct capsule_info {
 	long		index;
 	size_t		count;
 	size_t		total_size;
+	unsigned int	efi_hdr_displacement;
 	struct page	**pages;
 	size_t		page_bytes_remain;
 };
@@ -83,6 +84,8 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
 		return ret;
 	}
 
+	cap_info->efi_hdr_displacement = 0;
+
 	cap_info->total_size = cap_hdr->imagesize;
 	temp_page = krealloc(cap_info->pages,
 			     pages_needed * sizeof(void *),
@@ -103,16 +106,20 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
  **/
 static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 {
+	efi_capsule_header_t *cap_hdr;
+	void *mapped_pages;
 	int ret;
-	void *cap_hdr_temp;
 
-	cap_hdr_temp = vmap(cap_info->pages, cap_info->index,
-			VM_MAP, PAGE_KERNEL);
-	if (!cap_hdr_temp)
+	mapped_pages = vmap(cap_info->pages, cap_info->index,
+			    VM_MAP, PAGE_KERNEL);
+	if (!mapped_pages)
 		return -ENOMEM;
 
-	ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
-	vunmap(cap_hdr_temp);
+	cap_hdr = mapped_pages + cap_info->efi_hdr_displacement;
+
+	ret = efi_capsule_update(cap_hdr, cap_info->efi_hdr_displacement,
+				 cap_info->pages);
+	vunmap(mapped_pages);
 	if (ret) {
 		pr_err("capsule update failed\n");
 		return ret;
diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
index 6eedff4..a60c4c4 100644
--- a/drivers/firmware/efi/capsule.c
+++ b/drivers/firmware/efi/capsule.c
@@ -184,6 +184,8 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
 /**
  * efi_capsule_update - send a capsule to the firmware
  * @capsule: capsule to send to firmware
+ * @efi_hdr_displacement: EFI header offset on first data page (only needed for
+ * non-conforming CSH capsules)
  * @pages: an array of capsule data pages
  *
  * Build a scatter gather list with EFI capsule block descriptors to
@@ -214,9 +216,12 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
  *
  * Return 0 on success, a converted EFI status code on failure.
  */
-int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
+int efi_capsule_update(efi_capsule_header_t *capsule,
+		       unsigned int efi_hdr_displacement,
+		       struct page **pages)
 {
 	u32 imagesize = capsule->imagesize;
+	u32 total_size = imagesize + efi_hdr_displacement;
 	efi_guid_t guid = capsule->guid;
 	unsigned int count, sg_count;
 	u32 flags = capsule->flags;
@@ -224,11 +229,14 @@ int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
 	int rv, reset_type;
 	int i, j;
 
-	rv = efi_capsule_supported(guid, flags, imagesize, &reset_type);
+	if (efi_hdr_displacement > PAGE_SIZE - sizeof(efi_capsule_header_t))
+		return -EINVAL;
+
+	rv = efi_capsule_supported(guid, flags, total_size, &reset_type);
 	if (rv)
 		return rv;
 
-	count = DIV_ROUND_UP(imagesize, PAGE_SIZE);
+	count = DIV_ROUND_UP(total_size, PAGE_SIZE);
 	sg_count = sg_pages_num(count);
 
 	sg_pages = kzalloc(sg_count * sizeof(*sg_pages), GFP_KERNEL);
@@ -255,8 +263,13 @@ int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
 		for (j = 0; j < SGLIST_PER_PAGE && count > 0; j++) {
 			u64 sz = min_t(u64, imagesize, PAGE_SIZE);
 
-			sglist[j].length = sz;
 			sglist[j].data = page_to_phys(*pages++);
+			if (efi_hdr_displacement > 0) {
+				sglist[j].data += efi_hdr_displacement;
+				sz -= efi_hdr_displacement;
+				efi_hdr_displacement = 0;
+			}
+			sglist[j].length = sz;
 
 			imagesize -= sz;
 			count--;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 94d34e0..d83095c6 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1403,6 +1403,7 @@ extern int efi_capsule_supported(efi_guid_t guid, u32 flags,
 				 size_t size, int *reset);
 
 extern int efi_capsule_update(efi_capsule_header_t *capsule,
+			      unsigned int efi_hdr_displacement,
 			      struct page **pages);
 
 #ifdef CONFIG_EFI_RUNTIME_MAP
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1608957 — Re: [PATCH v2 5/7] efi/capsule: Prepare for loading images with security header

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-03-24 21:30 +0100
SubjectRe: [PATCH v2 5/7] efi/capsule: Prepare for loading images with security header
Message-ID<toE0q-6Pe-11@gated-at.bofh.it>
In reply to#1608710
On Fri, Mar 24, 2017 at 7:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> The Quark security header is nicely located in front of the capsule
> image, but we still need to pass the image to the update service as if
> there was none. Prepare efi_capsule_update and its user for this by
> defining and evaluating a EFI header displacement in the image located
> in memory. For standard-conforming capsules, this displacement is 0.

To me sounds like new field should be of type size_t.

>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/firmware/efi/capsule-loader.c | 19 +++++++++++++------
>  drivers/firmware/efi/capsule.c        | 21 +++++++++++++++++----
>  include/linux/efi.h                   |  1 +
>  3 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
> index 37d3f6e..59e2694 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -26,6 +26,7 @@ struct capsule_info {
>         long            index;
>         size_t          count;
>         size_t          total_size;
> +       unsigned int    efi_hdr_displacement;
>         struct page     **pages;
>         size_t          page_bytes_remain;
>  };
> @@ -83,6 +84,8 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
>                 return ret;
>         }
>
> +       cap_info->efi_hdr_displacement = 0;
> +
>         cap_info->total_size = cap_hdr->imagesize;
>         temp_page = krealloc(cap_info->pages,
>                              pages_needed * sizeof(void *),
> @@ -103,16 +106,20 @@ static int efi_capsule_setup_info(struct capsule_info *cap_info,
>   **/
>  static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
>  {
> +       efi_capsule_header_t *cap_hdr;
> +       void *mapped_pages;
>         int ret;
> -       void *cap_hdr_temp;
>
> -       cap_hdr_temp = vmap(cap_info->pages, cap_info->index,
> -                       VM_MAP, PAGE_KERNEL);
> -       if (!cap_hdr_temp)
> +       mapped_pages = vmap(cap_info->pages, cap_info->index,
> +                           VM_MAP, PAGE_KERNEL);
> +       if (!mapped_pages)
>                 return -ENOMEM;
>
> -       ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
> -       vunmap(cap_hdr_temp);
> +       cap_hdr = mapped_pages + cap_info->efi_hdr_displacement;
> +
> +       ret = efi_capsule_update(cap_hdr, cap_info->efi_hdr_displacement,
> +                                cap_info->pages);
> +       vunmap(mapped_pages);
>         if (ret) {
>                 pr_err("capsule update failed\n");
>                 return ret;
> diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> index 6eedff4..a60c4c4 100644
> --- a/drivers/firmware/efi/capsule.c
> +++ b/drivers/firmware/efi/capsule.c
> @@ -184,6 +184,8 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
>  /**
>   * efi_capsule_update - send a capsule to the firmware
>   * @capsule: capsule to send to firmware
> + * @efi_hdr_displacement: EFI header offset on first data page (only needed for
> + * non-conforming CSH capsules)
>   * @pages: an array of capsule data pages
>   *
>   * Build a scatter gather list with EFI capsule block descriptors to
> @@ -214,9 +216,12 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
>   *
>   * Return 0 on success, a converted EFI status code on failure.
>   */
> -int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
> +int efi_capsule_update(efi_capsule_header_t *capsule,
> +                      unsigned int efi_hdr_displacement,
> +                      struct page **pages)
>  {
>         u32 imagesize = capsule->imagesize;
> +       u32 total_size = imagesize + efi_hdr_displacement;
>         efi_guid_t guid = capsule->guid;
>         unsigned int count, sg_count;
>         u32 flags = capsule->flags;
> @@ -224,11 +229,14 @@ int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
>         int rv, reset_type;
>         int i, j;
>
> -       rv = efi_capsule_supported(guid, flags, imagesize, &reset_type);
> +       if (efi_hdr_displacement > PAGE_SIZE - sizeof(efi_capsule_header_t))
> +               return -EINVAL;
> +
> +       rv = efi_capsule_supported(guid, flags, total_size, &reset_type);
>         if (rv)
>                 return rv;
>
> -       count = DIV_ROUND_UP(imagesize, PAGE_SIZE);
> +       count = DIV_ROUND_UP(total_size, PAGE_SIZE);
>         sg_count = sg_pages_num(count);
>
>         sg_pages = kzalloc(sg_count * sizeof(*sg_pages), GFP_KERNEL);
> @@ -255,8 +263,13 @@ int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
>                 for (j = 0; j < SGLIST_PER_PAGE && count > 0; j++) {
>                         u64 sz = min_t(u64, imagesize, PAGE_SIZE);
>
> -                       sglist[j].length = sz;
>                         sglist[j].data = page_to_phys(*pages++);
> +                       if (efi_hdr_displacement > 0) {
> +                               sglist[j].data += efi_hdr_displacement;
> +                               sz -= efi_hdr_displacement;
> +                               efi_hdr_displacement = 0;
> +                       }
> +                       sglist[j].length = sz;
>
>                         imagesize -= sz;
>                         count--;
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 94d34e0..d83095c6 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1403,6 +1403,7 @@ extern int efi_capsule_supported(efi_guid_t guid, u32 flags,
>                                  size_t size, int *reset);
>
>  extern int efi_capsule_update(efi_capsule_header_t *capsule,
> +                             unsigned int efi_hdr_displacement,
>                               struct page **pages);
>
>  #ifdef CONFIG_EFI_RUNTIME_MAP
> --
> 2.10.2
>



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1608711 — [PATCH v2 3/7] efi/capsule: Clean up pr_err/info messages

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 3/7] efi/capsule: Clean up pr_err/info messages
Message-ID<toBlU-4TS-11@gated-at.bofh.it>
In reply to#1608707
Avoid __func__, improve the information provided by some of the
messages.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/firmware/efi/capsule-loader.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 7b57dda..3fb91e1 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -70,7 +70,7 @@ static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
 	pages_needed = ALIGN(cap_hdr->imagesize, PAGE_SIZE) >> PAGE_SHIFT;
 
 	if (pages_needed == 0) {
-		pr_err("%s: pages count invalid\n", __func__);
+		pr_err("invalid capsule size");
 		return -EINVAL;
 	}
 
@@ -79,8 +79,7 @@ static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
 				    cap_hdr->imagesize,
 				    &cap_info->reset_type);
 	if (ret) {
-		pr_err("%s: efi_capsule_supported() failed\n",
-		       __func__);
+		pr_err("capsule not supported\n");
 		return ret;
 	}
 
@@ -115,14 +114,14 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 	ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
 	vunmap(cap_hdr_temp);
 	if (ret) {
-		pr_err("%s: efi_capsule_update() failed\n", __func__);
+		pr_err("capsule update failed\n");
 		return ret;
 	}
 
 	/* Indicate capsule binary uploading is done */
 	cap_info->index = NO_FURTHER_WRITE_ACTION;
-	pr_info("%s: Successfully upload capsule file with reboot type '%s'\n",
-		__func__, !cap_info->reset_type ? "RESET_COLD" :
+	pr_info("Successfully upload capsule file with reboot type '%s'\n",
+		!cap_info->reset_type ? "RESET_COLD" :
 		cap_info->reset_type == 1 ? "RESET_WARM" :
 		"RESET_SHUTDOWN");
 	return 0;
@@ -207,8 +206,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 	if (cap_info->header_obtained &&
 	    cap_info->count >= cap_info->total_size) {
 		if (cap_info->count > cap_info->total_size) {
-			pr_err("%s: upload size exceeded header defined size\n",
-			       __func__);
+			pr_err("capsule upload size exceeded header defined size\n");
 			ret = -EINVAL;
 			goto failed;
 		}
@@ -242,7 +240,7 @@ static int efi_capsule_flush(struct file *file, fl_owner_t id)
 	struct capsule_info *cap_info = file->private_data;
 
 	if (cap_info->index > 0) {
-		pr_err("%s: capsule upload not complete\n", __func__);
+		pr_err("capsule upload not complete\n");
 		efi_free_all_buff_pages(cap_info);
 		ret = -ECANCELED;
 	}
@@ -321,8 +319,7 @@ static int __init efi_capsule_loader_init(void)
 
 	ret = misc_register(&efi_capsule_misc);
 	if (ret)
-		pr_err("%s: Failed to register misc char file note\n",
-		       __func__);
+		pr_err("Unable to register capsule loader device\n");
 
 	return ret;
 }
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1608822 — Re: [PATCH v2 3/7] efi/capsule: Clean up pr_err/info messages

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-03-24 19:20 +0100
SubjectRe: [PATCH v2 3/7] efi/capsule: Clean up pr_err/info messages
Message-ID<toBYF-5oX-83@gated-at.bofh.it>
In reply to#1608711
On 24 March 2017 at 17:34, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Avoid __func__, improve the information provided by some of the
> messages.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  drivers/firmware/efi/capsule-loader.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
> index 7b57dda..3fb91e1 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -70,7 +70,7 @@ static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
>         pages_needed = ALIGN(cap_hdr->imagesize, PAGE_SIZE) >> PAGE_SHIFT;
>
>         if (pages_needed == 0) {
> -               pr_err("%s: pages count invalid\n", __func__);
> +               pr_err("invalid capsule size");
>                 return -EINVAL;
>         }
>
> @@ -79,8 +79,7 @@ static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
>                                     cap_hdr->imagesize,
>                                     &cap_info->reset_type);
>         if (ret) {
> -               pr_err("%s: efi_capsule_supported() failed\n",
> -                      __func__);
> +               pr_err("capsule not supported\n");
>                 return ret;
>         }
>
> @@ -115,14 +114,14 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
>         ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
>         vunmap(cap_hdr_temp);
>         if (ret) {
> -               pr_err("%s: efi_capsule_update() failed\n", __func__);
> +               pr_err("capsule update failed\n");
>                 return ret;
>         }
>
>         /* Indicate capsule binary uploading is done */
>         cap_info->index = NO_FURTHER_WRITE_ACTION;
> -       pr_info("%s: Successfully upload capsule file with reboot type '%s'\n",
> -               __func__, !cap_info->reset_type ? "RESET_COLD" :
> +       pr_info("Successfully upload capsule file with reboot type '%s'\n",
> +               !cap_info->reset_type ? "RESET_COLD" :
>                 cap_info->reset_type == 1 ? "RESET_WARM" :
>                 "RESET_SHUTDOWN");
>         return 0;
> @@ -207,8 +206,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
>         if (cap_info->header_obtained &&
>             cap_info->count >= cap_info->total_size) {
>                 if (cap_info->count > cap_info->total_size) {
> -                       pr_err("%s: upload size exceeded header defined size\n",
> -                              __func__);
> +                       pr_err("capsule upload size exceeded header defined size\n");
>                         ret = -EINVAL;
>                         goto failed;
>                 }
> @@ -242,7 +240,7 @@ static int efi_capsule_flush(struct file *file, fl_owner_t id)
>         struct capsule_info *cap_info = file->private_data;
>
>         if (cap_info->index > 0) {
> -               pr_err("%s: capsule upload not complete\n", __func__);
> +               pr_err("capsule upload not complete\n");
>                 efi_free_all_buff_pages(cap_info);
>                 ret = -ECANCELED;
>         }
> @@ -321,8 +319,7 @@ static int __init efi_capsule_loader_init(void)
>
>         ret = misc_register(&efi_capsule_misc);
>         if (ret)
> -               pr_err("%s: Failed to register misc char file note\n",
> -                      __func__);
> +               pr_err("Unable to register capsule loader device\n");
>
>         return ret;
>  }
> --
> 2.10.2
>

[toc] | [prev] | [next] | [standalone]

#1608713 — [PATCH v2 2/7] efi/capsule: Remove pr_debug on ENOMEM or EFAULT

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 2/7] efi/capsule: Remove pr_debug on ENOMEM or EFAULT
Message-ID<toBlU-4TS-17@gated-at.bofh.it>
In reply to#1608707
Both cases are not worth a debug log message - the error code is telling
enough.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/firmware/efi/capsule-loader.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 91e91f7..7b57dda 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -88,10 +88,8 @@ static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
 	temp_page = krealloc(cap_info->pages,
 			     pages_needed * sizeof(void *),
 			     GFP_KERNEL | __GFP_ZERO);
-	if (!temp_page) {
-		pr_debug("%s: krealloc() failed\n", __func__);
+	if (!temp_page)
 		return -ENOMEM;
-	}
 
 	cap_info->pages = temp_page;
 	cap_info->header_obtained = true;
@@ -111,10 +109,8 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 
 	cap_hdr_temp = vmap(cap_info->pages, cap_info->index,
 			VM_MAP, PAGE_KERNEL);
-	if (!cap_hdr_temp) {
-		pr_debug("%s: vmap() failed\n", __func__);
+	if (!cap_hdr_temp)
 		return -ENOMEM;
-	}
 
 	ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
 	vunmap(cap_hdr_temp);
@@ -171,7 +167,6 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 	if (!cap_info->page_bytes_remain) {
 		page = alloc_page(GFP_KERNEL);
 		if (!page) {
-			pr_debug("%s: alloc_page() failed\n", __func__);
 			ret = -ENOMEM;
 			goto failed;
 		}
@@ -184,7 +179,6 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 
 	kbuff = kmap(page);
 	if (!kbuff) {
-		pr_debug("%s: kmap() failed\n", __func__);
 		ret = -ENOMEM;
 		goto failed;
 	}
@@ -193,7 +187,6 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 	/* Copy capsule binary data from user space to kernel space buffer */
 	write_byte = min_t(size_t, count, cap_info->page_bytes_remain);
 	if (copy_from_user(kbuff, buff, write_byte)) {
-		pr_debug("%s: copy_from_user() failed\n", __func__);
 		ret = -EFAULT;
 		goto fail_unmap;
 	}
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1608797 — Re: [PATCH v2 2/7] efi/capsule: Remove pr_debug on ENOMEM or EFAULT

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-03-24 19:20 +0100
SubjectRe: [PATCH v2 2/7] efi/capsule: Remove pr_debug on ENOMEM or EFAULT
Message-ID<toBYD-5oX-31@gated-at.bofh.it>
In reply to#1608713
On 24 March 2017 at 17:34, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Both cases are not worth a debug log message - the error code is telling
> enough.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  drivers/firmware/efi/capsule-loader.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
> index 91e91f7..7b57dda 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -88,10 +88,8 @@ static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
>         temp_page = krealloc(cap_info->pages,
>                              pages_needed * sizeof(void *),
>                              GFP_KERNEL | __GFP_ZERO);
> -       if (!temp_page) {
> -               pr_debug("%s: krealloc() failed\n", __func__);
> +       if (!temp_page)
>                 return -ENOMEM;
> -       }
>
>         cap_info->pages = temp_page;
>         cap_info->header_obtained = true;
> @@ -111,10 +109,8 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
>
>         cap_hdr_temp = vmap(cap_info->pages, cap_info->index,
>                         VM_MAP, PAGE_KERNEL);
> -       if (!cap_hdr_temp) {
> -               pr_debug("%s: vmap() failed\n", __func__);
> +       if (!cap_hdr_temp)
>                 return -ENOMEM;
> -       }
>
>         ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
>         vunmap(cap_hdr_temp);
> @@ -171,7 +167,6 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
>         if (!cap_info->page_bytes_remain) {
>                 page = alloc_page(GFP_KERNEL);
>                 if (!page) {
> -                       pr_debug("%s: alloc_page() failed\n", __func__);
>                         ret = -ENOMEM;
>                         goto failed;
>                 }
> @@ -184,7 +179,6 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
>
>         kbuff = kmap(page);
>         if (!kbuff) {
> -               pr_debug("%s: kmap() failed\n", __func__);
>                 ret = -ENOMEM;
>                 goto failed;
>         }
> @@ -193,7 +187,6 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
>         /* Copy capsule binary data from user space to kernel space buffer */
>         write_byte = min_t(size_t, count, cap_info->page_bytes_remain);
>         if (copy_from_user(kbuff, buff, write_byte)) {
> -               pr_debug("%s: copy_from_user() failed\n", __func__);
>                 ret = -EFAULT;
>                 goto fail_unmap;
>         }
> --
> 2.10.2
>

[toc] | [prev] | [next] | [standalone]

#1608714 — [PATCH v2 1/7] efi/capsule: Fix return code on failing kmap/vmap

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 1/7] efi/capsule: Fix return code on failing kmap/vmap
Message-ID<toBlU-4TS-23@gated-at.bofh.it>
In reply to#1608707
If kmap or vmap fail, it means we ran out of memory. There are no
user-provided addressed involved that would justify EFAULT.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/firmware/efi/capsule-loader.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 9ae6c11..91e91f7 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -113,7 +113,7 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 			VM_MAP, PAGE_KERNEL);
 	if (!cap_hdr_temp) {
 		pr_debug("%s: vmap() failed\n", __func__);
-		return -EFAULT;
+		return -ENOMEM;
 	}
 
 	ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
@@ -185,7 +185,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 	kbuff = kmap(page);
 	if (!kbuff) {
 		pr_debug("%s: kmap() failed\n", __func__);
-		ret = -EFAULT;
+		ret = -ENOMEM;
 		goto failed;
 	}
 	kbuff += PAGE_SIZE - cap_info->page_bytes_remain;
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1608820 — Re: [PATCH v2 1/7] efi/capsule: Fix return code on failing kmap/vmap

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-03-24 19:20 +0100
SubjectRe: [PATCH v2 1/7] efi/capsule: Fix return code on failing kmap/vmap
Message-ID<toBYF-5oX-77@gated-at.bofh.it>
In reply to#1608714
On 24 March 2017 at 17:34, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> If kmap or vmap fail, it means we ran out of memory. There are no
> user-provided addressed involved that would justify EFAULT.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  drivers/firmware/efi/capsule-loader.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
> index 9ae6c11..91e91f7 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -113,7 +113,7 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
>                         VM_MAP, PAGE_KERNEL);
>         if (!cap_hdr_temp) {
>                 pr_debug("%s: vmap() failed\n", __func__);
> -               return -EFAULT;
> +               return -ENOMEM;
>         }
>
>         ret = efi_capsule_update(cap_hdr_temp, cap_info->pages);
> @@ -185,7 +185,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
>         kbuff = kmap(page);
>         if (!kbuff) {
>                 pr_debug("%s: kmap() failed\n", __func__);
> -               ret = -EFAULT;
> +               ret = -ENOMEM;
>                 goto failed;
>         }
>         kbuff += PAGE_SIZE - cap_info->page_bytes_remain;
> --
> 2.10.2
>

[toc] | [prev] | [next] | [standalone]

#1608715 — [PATCH v2 7/7] efi/capsule: Add support for Quark security header

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-24 18:40 +0100
Subject[PATCH v2 7/7] efi/capsule: Add support for Quark security header
Message-ID<toBlU-4TS-19@gated-at.bofh.it>
In reply to#1608707
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 displacement in cap_info.

To be minimal invasive and maximal safe, the quirk version of
efi_capsule_identify_image is only effective on Quark processors.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/platform/efi/quirks.c | 95 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 30031d5..7f16295 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -13,12 +13,66 @@
 #include <linux/dmi.h>
 #include <asm/efi.h>
 #include <asm/uv/uv.h>
+#include <asm/cpu_device_id.h>
 
 #define EFI_MIN_RESERVE 5120
 
 #define EFI_DUMMY_GUID \
 	EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92, 0xa9)
 
+#define QUARK_CSH_SIGNATURE		0x5f435348	/* _CSH */
+#define QUARK_SECURITY_HEADER_SIZE	0x400
+
+/*
+ * Header prepended to the standard EFI capsule on Quark systems the are based
+ * on Intel firmware BSP.
+ * @csh_signature:	Unique identifier to sanity check signed module
+ * 			presence ("_CSH").
+ * @version:		Current version of CSH used. Should be one for Quark A0.
+ * @modulesize:		Size of the entire module including the module header
+ * 			and payload.
+ * @security_version_number_index: Index of SVN to use for validation of signed
+ * 			module.
+ * @security_version_number: Used to prevent against roll back of modules.
+ * @rsvd_module_id:	Currently unused for Clanton (Quark).
+ * @rsvd_module_vendor:	Vendor Identifier. For Intel products value is
+ * 			0x00008086.
+ * @rsvd_date:		BCD representation of build date as yyyymmdd, where
+ * 			yyyy=4 digit year, mm=1-12, dd=1-31.
+ * @headersize:		Total length of the header including including any
+ * 			padding optionally added by the signing tool.
+ * @hash_algo:		What Hash is used in the module signing.
+ * @cryp_algo:		What Crypto is used in the module signing.
+ * @keysize:		Total length of the key data including including any
+ * 			padding optionally added by the signing tool.
+ * @signaturesize:	Total length of the signature including including any
+ * 			padding optionally added by the signing tool.
+ * @rsvd_next_header:	32-bit pointer to the next Secure Boot Module in the
+ * 			chain, if there is a next header.
+ * @rsvd:		Reserved, padding structure to required size.
+ *
+ * See also QuartSecurityHeader_t in
+ * Quark_EDKII_v1.2.1.1/QuarkPlatformPkg/Include/QuarkBootRom.h
+ * from https://downloadcenter.intel.com/download/23197/Intel-Quark-SoC-X1000-Board-Support-Package-BSP
+ */
+struct 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];
+};
+
 static efi_char16_t efi_dummy_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 };
 
 static bool efi_no_storage_paranoia;
@@ -495,3 +549,44 @@ bool efi_poweroff_required(void)
 {
 	return acpi_gbl_reduced_hardware || acpi_no_s5;
 }
+
+static const struct x86_cpu_id quark_ids[] = {
+	{ X86_VENDOR_INTEL, 5, 9 },	/* Intel Quark X1000 */
+	{ }
+};
+
+int efi_capsule_identify_image(struct efi_capsule_info *cap_info, void *header,
+			       size_t hdr_bytes)
+{
+	struct quark_security_header *csh = header;
+
+	if (!x86_match_cpu(quark_ids))
+		return __efi_capsule_identify_image(cap_info, header,
+						    hdr_bytes);
+
+	/* Only process data block that is larger than the security header */
+	if (hdr_bytes < sizeof(struct quark_security_header))
+		return 0;
+
+	if (csh->csh_signature != QUARK_CSH_SIGNATURE ||
+	    csh->headersize != QUARK_SECURITY_HEADER_SIZE)
+		return __efi_capsule_identify_image(cap_info, header,
+						    hdr_bytes);
+
+	/* Only process data block if EFI header is included */
+	if (hdr_bytes < QUARK_SECURITY_HEADER_SIZE +
+			sizeof(efi_capsule_header_t))
+		return 0;
+
+	pr_debug("Quark security header detected\n");
+
+	if (csh->rsvd_next_header != 0) {
+		pr_err("multiple Quark security headers not supported\n");
+		return -EINVAL;
+	}
+
+	cap_info->total_size = csh->modulesize;
+	cap_info->efi_hdr_displacement = csh->headersize;
+
+	return 1;
+}
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1608960 — Re: [PATCH v2 7/7] efi/capsule: Add support for Quark security header

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-03-24 21:40 +0100
SubjectRe: [PATCH v2 7/7] efi/capsule: Add support for Quark security header
Message-ID<toEa5-6Td-17@gated-at.bofh.it>
In reply to#1608715
On Fri, Mar 24, 2017 at 7:34 PM, Jan Kiszka <jan.kiszka@siemens.com> 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 displacement in cap_info.
>
> To be minimal invasive and maximal safe, the quirk version of
> efi_capsule_identify_image is only effective on Quark processors.


> +static const struct x86_cpu_id quark_ids[] = {
> +       { X86_VENDOR_INTEL, 5, 9 },     /* Intel Quark X1000 */
> +       { }
> +};
> +
> +int efi_capsule_identify_image(struct efi_capsule_info *cap_info, void *header,
> +                              size_t hdr_bytes)
> +{
> +       struct quark_security_header *csh = header;
> +

> +       if (!x86_match_cpu(quark_ids))
> +               return __efi_capsule_identify_image(cap_info, header,
> +                                                   hdr_bytes);

I would slightly differently, i.e. introduce a helper
capsule_identify_image_qrk() and do here something like


#define ICPU(family, model, data) ...

static const struct x86_cpu_id efi_capsule_quirk_ids[] = {
      ICPU(5, 9, qrk_capsule_identify_image),
      {}
};

...
       id = x86_match_cpu(efi_capsule_quirk_ids);
       if (id)
          return ((...)id->data)(...);

       return __efi_capsule_identify_image(cap_info, header,  hdr_bytes);

> +
> +       /* Only process data block that is larger than the security header */
> +       if (hdr_bytes < sizeof(struct quark_security_header))
> +               return 0;
> +
> +       if (csh->csh_signature != QUARK_CSH_SIGNATURE ||
> +           csh->headersize != QUARK_SECURITY_HEADER_SIZE)
> +               return __efi_capsule_identify_image(cap_info, header,
> +                                                   hdr_bytes);
> +
> +       /* Only process data block if EFI header is included */
> +       if (hdr_bytes < QUARK_SECURITY_HEADER_SIZE +
> +                       sizeof(efi_capsule_header_t))
> +               return 0;
> +
> +       pr_debug("Quark security header detected\n");
> +
> +       if (csh->rsvd_next_header != 0) {
> +               pr_err("multiple Quark security headers not supported\n");
> +               return -EINVAL;
> +       }
> +
> +       cap_info->total_size = csh->modulesize;
> +       cap_info->efi_hdr_displacement = csh->headersize;
> +
> +       return 1;
> +}


-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1608963 — Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-03-24 21:50 +0100
SubjectRe: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images
Message-ID<toEjL-6Xy-1@gated-at.bofh.it>
In reply to#1608707
On Fri, Mar 24, 2017 at 7:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This addresses the review feedback provided on round 1, specifically
> - refactored queue to keep the Quark mess in
> - only check for CSH on Quark CPUs
> - added some smaller cleanups of the capsule loader
> - documented capsule header / linked to original code
>
> See last patch for the background of the series.
>
> The series has been tested on the Galileo Gen2, to exclude regressions,
> with a firmware.cap with AND without security header and the SIMATIC
> IOT2040 which requires the header because of its mandatory secure boot.

The series looks good to me from code prospective. It's clean and
understandable, though I have couple of comments (see related
patches).
Please, address them.

I wasn't able to test it, thus FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Jan
>
> Jan Kiszka (7):
>   efi/capsule: Fix return code on failing kmap/vmap
>   efi/capsule: Remove pr_debug on ENOMEM or EFAULT
>   efi/capsule: Clean up pr_err/info messages
>   efi/capsule: Adjust return type of efi_capsule_setup_info
>   efi/capsule: Prepare for loading images with security header
>   efi/capsule: Factor out overloadable efi_capsule_identify_image
>   efi/capsule: Add support for Quark security header
>
>  arch/x86/platform/efi/quirks.c        |  95 ++++++++++++++++++++++++
>  drivers/firmware/efi/capsule-loader.c | 136 +++++++++++++++++++---------------
>  drivers/firmware/efi/capsule.c        |  21 +++++-
>  include/linux/efi.h                   |  19 +++++
>  4 files changed, 208 insertions(+), 63 deletions(-)
>
> --
> 2.10.2
>



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1609743 — Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-27 13:30 +0200
SubjectRe: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images
Message-ID<tpB0u-7gY-5@gated-at.bofh.it>
In reply to#1608963
On 2017-03-24 21:39, Andy Shevchenko wrote:
> On Fri, Mar 24, 2017 at 7:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> This addresses the review feedback provided on round 1, specifically
>> - refactored queue to keep the Quark mess in
>> - only check for CSH on Quark CPUs
>> - added some smaller cleanups of the capsule loader
>> - documented capsule header / linked to original code
>>
>> See last patch for the background of the series.
>>
>> The series has been tested on the Galileo Gen2, to exclude regressions,
>> with a firmware.cap with AND without security header and the SIMATIC
>> IOT2040 which requires the header because of its mandatory secure boot.
> 
> The series looks good to me from code prospective. It's clean and
> understandable, though I have couple of comments (see related
> patches).
> Please, address them.

Done, also the kbuild reboot finding and another build issue along that.
Will send v3 after testing again (unfortunately not on the Galileo - my
board died).

> 
> I wasn't able to test it, thus FWIW:
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thanks,
Jan

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

[toc] | [prev] | [next] | [standalone]

#1609733 — Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images

FromBryan O'Donoghue <pure.logic@nexus-software.ie>
Date2017-03-27 12:50 +0200
SubjectRe: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images
Message-ID<tpAnM-6J9-33@gated-at.bofh.it>
In reply to#1608707

On 24/03/17 17:34, Jan Kiszka wrote:
> This addresses the review feedback provided on round 1, specifically
> - refactored queue to keep the Quark mess in
> - only check for CSH on Quark CPUs
> - added some smaller cleanups of the capsule loader
> - documented capsule header / linked to original code
>
> See last patch for the background of the series.
>
> The series has been tested on the Galileo Gen2, to exclude regressions,
> with a firmware.cap with AND without security header and the SIMATIC
> IOT2040 which requires the header because of its mandatory secure boot.
>
> Jan
>
> Jan Kiszka (7):
>   efi/capsule: Fix return code on failing kmap/vmap
>   efi/capsule: Remove pr_debug on ENOMEM or EFAULT
>   efi/capsule: Clean up pr_err/info messages
>   efi/capsule: Adjust return type of efi_capsule_setup_info
>   efi/capsule: Prepare for loading images with security header
>   efi/capsule: Factor out overloadable efi_capsule_identify_image
>   efi/capsule: Add support for Quark security header
>
>  arch/x86/platform/efi/quirks.c        |  95 ++++++++++++++++++++++++
>  drivers/firmware/efi/capsule-loader.c | 136 +++++++++++++++++++---------------
>  drivers/firmware/efi/capsule.c        |  21 +++++-
>  include/linux/efi.h                   |  19 +++++
>  4 files changed, 208 insertions(+), 63 deletions(-)
>

BTW,

Thanks for taking the time to remove the __func__ stuff all over the place.

I'll try to test this out for you. I found that the current BSP Intel is 
releasing has some sort of GUI that downloads an image to a board (which 
completely fails for me on the Galileo I have)... not sure if you have 
different results with the stuff from the Intel website but it's 
non-functional for me :(

I'd like to suggest to you adding something to Documentation describing 
how to load and trigger a capsule update. For example on Quark you need 
to use the EFI reset method to cause capsule update to work.

Could you add a patch to your series for Documentation detailing:

1. Entry criteria (needing to boot in EFI reset mode)
1. Description of loading a capsule
3. Description of triggering the update (reboot)
4. Verifying the update succeeded (actually is this possible right now?)

---
bod

[toc] | [prev] | [next] | [standalone]

#1609740 — Re: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images

FromJan Kiszka <jan.kiszka@siemens.com>
Date2017-03-27 13:10 +0200
SubjectRe: [PATCH v2 0/7] efi: Enhance capsule loader to support signed Quark images
Message-ID<tpAH8-75T-11@gated-at.bofh.it>
In reply to#1609733
On 2017-03-27 12:29, Bryan O'Donoghue wrote:
> 
> 
> On 24/03/17 17:34, Jan Kiszka wrote:
>> This addresses the review feedback provided on round 1, specifically
>> - refactored queue to keep the Quark mess in
>> - only check for CSH on Quark CPUs
>> - added some smaller cleanups of the capsule loader
>> - documented capsule header / linked to original code
>>
>> See last patch for the background of the series.
>>
>> The series has been tested on the Galileo Gen2, to exclude regressions,
>> with a firmware.cap with AND without security header and the SIMATIC
>> IOT2040 which requires the header because of its mandatory secure boot.
>>
>> Jan
>>
>> Jan Kiszka (7):
>>   efi/capsule: Fix return code on failing kmap/vmap
>>   efi/capsule: Remove pr_debug on ENOMEM or EFAULT
>>   efi/capsule: Clean up pr_err/info messages
>>   efi/capsule: Adjust return type of efi_capsule_setup_info
>>   efi/capsule: Prepare for loading images with security header
>>   efi/capsule: Factor out overloadable efi_capsule_identify_image
>>   efi/capsule: Add support for Quark security header
>>
>>  arch/x86/platform/efi/quirks.c        |  95 ++++++++++++++++++++++++
>>  drivers/firmware/efi/capsule-loader.c | 136
>> +++++++++++++++++++---------------
>>  drivers/firmware/efi/capsule.c        |  21 +++++-
>>  include/linux/efi.h                   |  19 +++++
>>  4 files changed, 208 insertions(+), 63 deletions(-)
>>
> 
> BTW,
> 
> Thanks for taking the time to remove the __func__ stuff all over the place.
> 
> I'll try to test this out for you. I found that the current BSP Intel is
> releasing has some sort of GUI that downloads an image to a board (which
> completely fails for me on the Galileo I have)... not sure if you have
> different results with the stuff from the Intel website but it's
> non-functional for me :(

I found the Galileo capsules *.cap in the jar archives of the Galileo
firmware update packages, and they work.
> 
> I'd like to suggest to you adding something to Documentation describing
> how to load and trigger a capsule update. For example on Quark you need
> to use the EFI reset method to cause capsule update to work.

cat /path/to/capsule.cap > /dev/efi_capsule_loader

The reset method is auto-adjusted by the kernel when an update is
pending. So firmware update is now seriously simple. But I can write a 2
or 3-lines readme if it's preferred.

> 
> Could you add a patch to your series for Documentation detailing:
> 
> 1. Entry criteria (needing to boot in EFI reset mode)
> 1. Description of loading a capsule
> 3. Description of triggering the update (reboot)
> 4. Verifying the update succeeded (actually is this possible right now?)

dmidecode -> check BIOS version information (vendor-specific)

Jan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web