diff mbox series

[v5] test_firmware: Test platform fw loading on non-EFI systems

Message ID 20200909225354.3118328-1-keescook@chromium.org
State New
Headers show
Series [v5] test_firmware: Test platform fw loading on non-EFI systems | expand

Commit Message

Kees Cook Sept. 9, 2020, 10:53 p.m. UTC
On non-EFI systems, it wasn't possible to test the platform firmware
loader because it will have never set "checked_fw" during __init.
Instead, allow the test code to override this check. Additionally split
the declarations into a private symbol namespace so there is greater
enforcement of the symbol visibility.

Fixes: 548193cba2a7 ("test_firmware: add support for firmware_request_platform")
Cc: stable@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
This is split out from the larger kernel_read_file series:
    https://lore.kernel.org/lkml/20200729175845.1745471-1-keescook@chromium.org/
specifically this was:
    https://lore.kernel.org/lkml/20200729175845.1745471-2-keescook@chromium.org/

I've dropped the review tags, since this is changing the "how" of the patch...
---
 drivers/firmware/efi/embedded-firmware.c | 10 +++++-----
 include/linux/efi_embedded_fw.h          |  6 ++----
 lib/test_firmware.c                      |  9 +++++++++
 3 files changed, 16 insertions(+), 9 deletions(-)

Comments

Ard Biesheuvel Sept. 10, 2020, 10:06 a.m. UTC | #1
On Thu, 10 Sep 2020 at 01:54, Kees Cook <keescook@chromium.org> wrote:
>
> On non-EFI systems, it wasn't possible to test the platform firmware
> loader because it will have never set "checked_fw" during __init.
> Instead, allow the test code to override this check. Additionally split
> the declarations into a private symbol namespace so there is greater
> enforcement of the symbol visibility.
>
> Fixes: 548193cba2a7 ("test_firmware: add support for firmware_request_platform")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> This is split out from the larger kernel_read_file series:
>     https://lore.kernel.org/lkml/20200729175845.1745471-1-keescook@chromium.org/
> specifically this was:
>     https://lore.kernel.org/lkml/20200729175845.1745471-2-keescook@chromium.org/
>
> I've dropped the review tags, since this is changing the "how" of the patch...
> ---
>  drivers/firmware/efi/embedded-firmware.c | 10 +++++-----
>  include/linux/efi_embedded_fw.h          |  6 ++----
>  lib/test_firmware.c                      |  9 +++++++++
>  3 files changed, 16 insertions(+), 9 deletions(-)
>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> diff --git a/drivers/firmware/efi/embedded-firmware.c b/drivers/firmware/efi/embedded-firmware.c
> index a1b199de9006..84e32634ed6c 100644
> --- a/drivers/firmware/efi/embedded-firmware.c
> +++ b/drivers/firmware/efi/embedded-firmware.c
> @@ -16,9 +16,9 @@
>
>  /* Exported for use by lib/test_firmware.c only */
>  LIST_HEAD(efi_embedded_fw_list);
> -EXPORT_SYMBOL_GPL(efi_embedded_fw_list);
> -
> -static bool checked_for_fw;
> +EXPORT_SYMBOL_NS_GPL(efi_embedded_fw_list, TEST_FIRMWARE);
> +bool efi_embedded_fw_checked;
> +EXPORT_SYMBOL_NS_GPL(efi_embedded_fw_checked, TEST_FIRMWARE);
>
>  static const struct dmi_system_id * const embedded_fw_table[] = {
>  #ifdef CONFIG_TOUCHSCREEN_DMI
> @@ -119,14 +119,14 @@ void __init efi_check_for_embedded_firmwares(void)
>                 }
>         }
>
> -       checked_for_fw = true;
> +       efi_embedded_fw_checked = true;
>  }
>
>  int efi_get_embedded_fw(const char *name, const u8 **data, size_t *size)
>  {
>         struct efi_embedded_fw *iter, *fw = NULL;
>
> -       if (!checked_for_fw) {
> +       if (!efi_embedded_fw_checked) {
>                 pr_warn("Warning %s called while we did not check for embedded fw\n",
>                         __func__);
>                 return -ENOENT;
> diff --git a/include/linux/efi_embedded_fw.h b/include/linux/efi_embedded_fw.h
> index 57eac5241303..a97a12bb2c9e 100644
> --- a/include/linux/efi_embedded_fw.h
> +++ b/include/linux/efi_embedded_fw.h
> @@ -8,8 +8,8 @@
>  #define EFI_EMBEDDED_FW_PREFIX_LEN             8
>
>  /*
> - * This struct and efi_embedded_fw_list are private to the efi-embedded fw
> - * implementation they are in this header for use by lib/test_firmware.c only!
> + * This struct is private to the efi-embedded fw implementation.
> + * They are in this header for use by lib/test_firmware.c only!
>   */
>  struct efi_embedded_fw {
>         struct list_head list;
> @@ -18,8 +18,6 @@ struct efi_embedded_fw {
>         size_t length;
>  };
>
> -extern struct list_head efi_embedded_fw_list;
> -
>  /**
>   * struct efi_embedded_fw_desc - This struct is used by the EFI embedded-fw
>   *                               code to search for embedded firmwares.
> diff --git a/lib/test_firmware.c b/lib/test_firmware.c
> index 9fee2b93a8d1..06c955057756 100644
> --- a/lib/test_firmware.c
> +++ b/lib/test_firmware.c
> @@ -26,6 +26,8 @@
>  #include <linux/vmalloc.h>
>  #include <linux/efi_embedded_fw.h>
>
> +MODULE_IMPORT_NS(TEST_FIRMWARE);
> +
>  #define TEST_FIRMWARE_NAME     "test-firmware.bin"
>  #define TEST_FIRMWARE_NUM_REQS 4
>  #define TEST_FIRMWARE_BUF_SIZE SZ_1K
> @@ -489,6 +491,9 @@ static ssize_t trigger_request_store(struct device *dev,
>  static DEVICE_ATTR_WO(trigger_request);
>
>  #ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
> +extern struct list_head efi_embedded_fw_list;
> +extern bool efi_embedded_fw_checked;
> +
>  static ssize_t trigger_request_platform_store(struct device *dev,
>                                               struct device_attribute *attr,
>                                               const char *buf, size_t count)
> @@ -501,6 +506,7 @@ static ssize_t trigger_request_platform_store(struct device *dev,
>         };
>         struct efi_embedded_fw efi_embedded_fw;
>         const struct firmware *firmware = NULL;
> +       bool saved_efi_embedded_fw_checked;
>         char *name;
>         int rc;
>
> @@ -513,6 +519,8 @@ static ssize_t trigger_request_platform_store(struct device *dev,
>         efi_embedded_fw.data = (void *)test_data;
>         efi_embedded_fw.length = sizeof(test_data);
>         list_add(&efi_embedded_fw.list, &efi_embedded_fw_list);
> +       saved_efi_embedded_fw_checked = efi_embedded_fw_checked;
> +       efi_embedded_fw_checked = true;
>
>         pr_info("loading '%s'\n", name);
>         rc = firmware_request_platform(&firmware, name, dev);
> @@ -530,6 +538,7 @@ static ssize_t trigger_request_platform_store(struct device *dev,
>         rc = count;
>
>  out:
> +       efi_embedded_fw_checked = saved_efi_embedded_fw_checked;
>         release_firmware(firmware);
>         list_del(&efi_embedded_fw.list);
>         kfree(name);
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/firmware/efi/embedded-firmware.c b/drivers/firmware/efi/embedded-firmware.c
index a1b199de9006..84e32634ed6c 100644
--- a/drivers/firmware/efi/embedded-firmware.c
+++ b/drivers/firmware/efi/embedded-firmware.c
@@ -16,9 +16,9 @@ 
 
 /* Exported for use by lib/test_firmware.c only */
 LIST_HEAD(efi_embedded_fw_list);
-EXPORT_SYMBOL_GPL(efi_embedded_fw_list);
-
-static bool checked_for_fw;
+EXPORT_SYMBOL_NS_GPL(efi_embedded_fw_list, TEST_FIRMWARE);
+bool efi_embedded_fw_checked;
+EXPORT_SYMBOL_NS_GPL(efi_embedded_fw_checked, TEST_FIRMWARE);
 
 static const struct dmi_system_id * const embedded_fw_table[] = {
 #ifdef CONFIG_TOUCHSCREEN_DMI
@@ -119,14 +119,14 @@  void __init efi_check_for_embedded_firmwares(void)
 		}
 	}
 
-	checked_for_fw = true;
+	efi_embedded_fw_checked = true;
 }
 
 int efi_get_embedded_fw(const char *name, const u8 **data, size_t *size)
 {
 	struct efi_embedded_fw *iter, *fw = NULL;
 
-	if (!checked_for_fw) {
+	if (!efi_embedded_fw_checked) {
 		pr_warn("Warning %s called while we did not check for embedded fw\n",
 			__func__);
 		return -ENOENT;
diff --git a/include/linux/efi_embedded_fw.h b/include/linux/efi_embedded_fw.h
index 57eac5241303..a97a12bb2c9e 100644
--- a/include/linux/efi_embedded_fw.h
+++ b/include/linux/efi_embedded_fw.h
@@ -8,8 +8,8 @@ 
 #define EFI_EMBEDDED_FW_PREFIX_LEN		8
 
 /*
- * This struct and efi_embedded_fw_list are private to the efi-embedded fw
- * implementation they are in this header for use by lib/test_firmware.c only!
+ * This struct is private to the efi-embedded fw implementation.
+ * They are in this header for use by lib/test_firmware.c only!
  */
 struct efi_embedded_fw {
 	struct list_head list;
@@ -18,8 +18,6 @@  struct efi_embedded_fw {
 	size_t length;
 };
 
-extern struct list_head efi_embedded_fw_list;
-
 /**
  * struct efi_embedded_fw_desc - This struct is used by the EFI embedded-fw
  *                               code to search for embedded firmwares.
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 9fee2b93a8d1..06c955057756 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -26,6 +26,8 @@ 
 #include <linux/vmalloc.h>
 #include <linux/efi_embedded_fw.h>
 
+MODULE_IMPORT_NS(TEST_FIRMWARE);
+
 #define TEST_FIRMWARE_NAME	"test-firmware.bin"
 #define TEST_FIRMWARE_NUM_REQS	4
 #define TEST_FIRMWARE_BUF_SIZE	SZ_1K
@@ -489,6 +491,9 @@  static ssize_t trigger_request_store(struct device *dev,
 static DEVICE_ATTR_WO(trigger_request);
 
 #ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
+extern struct list_head efi_embedded_fw_list;
+extern bool efi_embedded_fw_checked;
+
 static ssize_t trigger_request_platform_store(struct device *dev,
 					      struct device_attribute *attr,
 					      const char *buf, size_t count)
@@ -501,6 +506,7 @@  static ssize_t trigger_request_platform_store(struct device *dev,
 	};
 	struct efi_embedded_fw efi_embedded_fw;
 	const struct firmware *firmware = NULL;
+	bool saved_efi_embedded_fw_checked;
 	char *name;
 	int rc;
 
@@ -513,6 +519,8 @@  static ssize_t trigger_request_platform_store(struct device *dev,
 	efi_embedded_fw.data = (void *)test_data;
 	efi_embedded_fw.length = sizeof(test_data);
 	list_add(&efi_embedded_fw.list, &efi_embedded_fw_list);
+	saved_efi_embedded_fw_checked = efi_embedded_fw_checked;
+	efi_embedded_fw_checked = true;
 
 	pr_info("loading '%s'\n", name);
 	rc = firmware_request_platform(&firmware, name, dev);
@@ -530,6 +538,7 @@  static ssize_t trigger_request_platform_store(struct device *dev,
 	rc = count;
 
 out:
+	efi_embedded_fw_checked = saved_efi_embedded_fw_checked;
 	release_firmware(firmware);
 	list_del(&efi_embedded_fw.list);
 	kfree(name);