[tip:efi/core] efi/capsule-loader: Reinstate virtual capsule mapping

[tip-bot for Ard Biesheuvel]

Commit-ID:  f24c4d478013d82bd1b943df566fff3561d52864
Gitweb: https://git.kernel.org/tip/f24c4d478013d82bd1b943df566fff3561d52864
Author: Ard Biesheuvel 
AuthorDate: Tue, 2 Jan 2018 17:21:10 +
Committer:  Ingo Molnar 
CommitDate: Wed, 3 Jan 2018 13:54:31 +0100

efi/capsule-loader: Reinstate virtual capsule mapping

Commit:

  82c3768b8d68 ("efi/capsule-loader: Use a cached copy of the capsule header")

... refactored the capsule loading code that maps the capsule header,
to avoid having to map it several times.

However, as it turns out, the vmap() call we ended up removing did not
just map the header, but the entire capsule image, and dropping this
virtual mapping breaks capsules that are processed by the firmware
immediately (i.e., without a reboot).

Unfortunately, that change was part of a larger refactor that allowed
a quirk to be implemented for Quark, which has a non-standard memory
layout for capsules, and we have slightly painted ourselves into a
corner by allowing quirk code to mangle the capsule header and memory
layout.

So we need to fix this without breaking Quark. Fortunately, Quark does
not appear to care about the virtual mapping, and so we can simply
do a partial revert of commit:

  2a457fb31df6 ("efi/capsule-loader: Use page addresses rather than struct page 
pointers")

... and create a vmap() mapping of the entire capsule (including header)
based on the reinstated struct page array, unless running on Quark, in
which case we pass the capsule header copy as before.

Reported-by: Ge Song 
Tested-by: Bryan O'Donoghue 
Tested-by: Ge Song 
Signed-off-by: Ard Biesheuvel 
Cc: 
Cc: Dave Young 
Cc: Linus Torvalds 
Cc: Matt Fleming 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: linux-...@vger.kernel.org
Fixes: 82c3768b8d68 ("efi/capsule-loader: Use a cached copy of the capsule 
header")
Link: http://lkml.kernel.org/r/20180102172110.17018-3-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar 
---
 arch/x86/platform/efi/quirks.c| 13 +-
 drivers/firmware/efi/capsule-loader.c | 45 ---
 include/linux/efi.h   |  4 +++-
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 8a99a2e..5b513cc 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -592,7 +592,18 @@ static int qrk_capsule_setup_info(struct capsule_info 
*cap_info, void **pkbuff,
/*
 * Update the first page pointer to skip over the CSH header.
 */
-   cap_info->pages[0] += csh->headersize;
+   cap_info->phys[0] += csh->headersize;
+
+   /*
+* cap_info->capsule should point at a virtual mapping of the entire
+* capsule, starting at the capsule header. Our image has the Quark
+* security header prepended, so we cannot rely on the default vmap()
+* mapping created by the generic capsule code.
+* Given that the Quark firmware does not appear to care about the
+* virtual mapping, let's just point cap_info->capsule at our copy
+* of the capsule header.
+*/
+   cap_info->capsule = _info->header;
 
return 1;
 }
diff --git a/drivers/firmware/efi/capsule-loader.c 
b/drivers/firmware/efi/capsule-loader.c
index ec8ac5c..055e2e8 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -20,10 +20,6 @@
 
 #define NO_FURTHER_WRITE_ACTION -1
 
-#ifndef phys_to_page
-#define phys_to_page(x)pfn_to_page((x) >> PAGE_SHIFT)
-#endif
-
 /**
  * efi_free_all_buff_pages - free all previous allocated buffer pages
  * @cap_info: pointer to current instance of capsule_info structure
@@ -35,7 +31,7 @@
 static void efi_free_all_buff_pages(struct capsule_info *cap_info)
 {
while (cap_info->index > 0)
-   __free_page(phys_to_page(cap_info->pages[--cap_info->index]));
+   __free_page(cap_info->pages[--cap_info->index]);
 
cap_info->index = NO_FURTHER_WRITE_ACTION;
 }
@@ -71,6 +67,14 @@ int __efi_capsule_setup_info(struct capsule_info *cap_info)
 
cap_info->pages = temp_page;
 
+   temp_page = krealloc(cap_info->phys,
+pages_needed * sizeof(phys_addr_t *),
+GFP_KERNEL | __GFP_ZERO);
+   if (!temp_page)
+   return -ENOMEM;
+
+   cap_info->phys = temp_page;
+
return 0;
 }
 
@@ -105,9 +109,24 @@ int __weak efi_capsule_setup_info(struct capsule_info 
*cap_info, void *kbuff,
  **/
 static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 {
+   bool do_vunmap = false;

[tip:efi/core] efi/capsule-loader: Reinstate virtual capsule mapping

[tip-bot for Ard Biesheuvel]

Commit-ID:  f24c4d478013d82bd1b943df566fff3561d52864
Gitweb: https://git.kernel.org/tip/f24c4d478013d82bd1b943df566fff3561d52864
Author: Ard Biesheuvel 
AuthorDate: Tue, 2 Jan 2018 17:21:10 +
Committer:  Ingo Molnar 
CommitDate: Wed, 3 Jan 2018 13:54:31 +0100

efi/capsule-loader: Reinstate virtual capsule mapping

Commit:

  82c3768b8d68 ("efi/capsule-loader: Use a cached copy of the capsule header")

... refactored the capsule loading code that maps the capsule header,
to avoid having to map it several times.

However, as it turns out, the vmap() call we ended up removing did not
just map the header, but the entire capsule image, and dropping this
virtual mapping breaks capsules that are processed by the firmware
immediately (i.e., without a reboot).

Unfortunately, that change was part of a larger refactor that allowed
a quirk to be implemented for Quark, which has a non-standard memory
layout for capsules, and we have slightly painted ourselves into a
corner by allowing quirk code to mangle the capsule header and memory
layout.

So we need to fix this without breaking Quark. Fortunately, Quark does
not appear to care about the virtual mapping, and so we can simply
do a partial revert of commit:

  2a457fb31df6 ("efi/capsule-loader: Use page addresses rather than struct page 
pointers")

... and create a vmap() mapping of the entire capsule (including header)
based on the reinstated struct page array, unless running on Quark, in
which case we pass the capsule header copy as before.

Reported-by: Ge Song 
Tested-by: Bryan O'Donoghue 
Tested-by: Ge Song 
Signed-off-by: Ard Biesheuvel 
Cc: 
Cc: Dave Young 
Cc: Linus Torvalds 
Cc: Matt Fleming 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: linux-...@vger.kernel.org
Fixes: 82c3768b8d68 ("efi/capsule-loader: Use a cached copy of the capsule 
header")
Link: http://lkml.kernel.org/r/20180102172110.17018-3-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar 
---
 arch/x86/platform/efi/quirks.c| 13 +-
 drivers/firmware/efi/capsule-loader.c | 45 ---
 include/linux/efi.h   |  4 +++-
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 8a99a2e..5b513cc 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -592,7 +592,18 @@ static int qrk_capsule_setup_info(struct capsule_info 
*cap_info, void **pkbuff,
/*
 * Update the first page pointer to skip over the CSH header.
 */
-   cap_info->pages[0] += csh->headersize;
+   cap_info->phys[0] += csh->headersize;
+
+   /*
+* cap_info->capsule should point at a virtual mapping of the entire
+* capsule, starting at the capsule header. Our image has the Quark
+* security header prepended, so we cannot rely on the default vmap()
+* mapping created by the generic capsule code.
+* Given that the Quark firmware does not appear to care about the
+* virtual mapping, let's just point cap_info->capsule at our copy
+* of the capsule header.
+*/
+   cap_info->capsule = _info->header;
 
return 1;
 }
diff --git a/drivers/firmware/efi/capsule-loader.c 
b/drivers/firmware/efi/capsule-loader.c
index ec8ac5c..055e2e8 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -20,10 +20,6 @@
 
 #define NO_FURTHER_WRITE_ACTION -1
 
-#ifndef phys_to_page
-#define phys_to_page(x)pfn_to_page((x) >> PAGE_SHIFT)
-#endif
-
 /**
  * efi_free_all_buff_pages - free all previous allocated buffer pages
  * @cap_info: pointer to current instance of capsule_info structure
@@ -35,7 +31,7 @@
 static void efi_free_all_buff_pages(struct capsule_info *cap_info)
 {
while (cap_info->index > 0)
-   __free_page(phys_to_page(cap_info->pages[--cap_info->index]));
+   __free_page(cap_info->pages[--cap_info->index]);
 
cap_info->index = NO_FURTHER_WRITE_ACTION;
 }
@@ -71,6 +67,14 @@ int __efi_capsule_setup_info(struct capsule_info *cap_info)
 
cap_info->pages = temp_page;
 
+   temp_page = krealloc(cap_info->phys,
+pages_needed * sizeof(phys_addr_t *),
+GFP_KERNEL | __GFP_ZERO);
+   if (!temp_page)
+   return -ENOMEM;
+
+   cap_info->phys = temp_page;
+
return 0;
 }
 
@@ -105,9 +109,24 @@ int __weak efi_capsule_setup_info(struct capsule_info 
*cap_info, void *kbuff,
  **/
 static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 {
+   bool do_vunmap = false;
int ret;
 
-   ret = efi_capsule_update(_info->header, cap_info->pages);
+   /*
+* cap_info->capsule may have been assigned already by a quirk
+* handler, so only overwrite it if it is NULL
+*/
+   if (!cap_info->capsule) {
+   cap_info->capsule =