efi/libstub: Clean up command line parsing routine

We currently parse the command non-destructively, to avoid having to
allocate memory for a copy before passing it to the standard parsing
routines that are used by the core kernel, and which modify the input
to delineate the parsed tokens with NUL characters.

Instead, we call strstr() and strncmp() to go over the input multiple
times, and match prefixes rather than tokens, which implies that we
would match, e.g., 'nokaslrfoo' in the stub and disable KASLR, while
the kernel would disregard the option and run with KASLR enabled.

In order to avoid having to reason about whether and how this behavior
may be abused, let's clean up the parsing routines, and rebuild them
on top of the existing helpers.

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

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 87cb3d45b4bd8a58d9c17478ec7eebf74078ac5e..9a7aef0d6f703d83509012e87bb50ee68e8be1cb 100644 (file)
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -47,6 +47,7 @@ __efistub_stext                       = stext;
 __efistub__end                 = _end;
 __efistub__edata               = _edata;
 __efistub_screen_info          = screen_info;
+__efistub__ctype               = _ctype;
 
 #endif
 

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 1202c9ee0ea95f64a8f1e716634c3022116b6e21..4d6246c6f651a7605a56b6375850fc7f379abd14 100644 (file)
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -40,7 +40,8 @@ OBJECT_FILES_NON_STANDARD     := y
 KCOV_INSTRUMENT                        := n
 
 lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o \
-                                  file.o mem.o random.o randomalloc.o pci.o
+                                  file.o mem.o random.o randomalloc.o pci.o \
+                                  skip_spaces.o lib-cmdline.o lib-ctype.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index db23be5dc69b7f202df3b1ccd5a1338f76268a8e..49008ac88b6315fb3524df2a06bb22e3fe261663 100644 (file)
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -68,66 +68,39 @@ void efi_printk(char *str)
  */
 efi_status_t efi_parse_options(char const *cmdline)
 {
-       char *str;
-
-       str = strstr(cmdline, "nokaslr");
-       if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
-               efi_nokaslr = true;
-
-       str = strstr(cmdline, "quiet");
-       if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
-               efi_quiet = true;
-
-       /*
-        * If no EFI parameters were specified on the cmdline we've got
-        * nothing to do.
-        */
-       str = strstr(cmdline, "efi=");
-       if (!str)
-               return EFI_SUCCESS;
-
-       /* Skip ahead to first argument */
-       str += strlen("efi=");
-
-       /*
-        * Remember, because efi= is also used by the kernel we need to
-        * skip over arguments we don't understand.
-        */
-       while (*str && *str != ' ') {
-               if (!strncmp(str, "nochunk", 7)) {
-                       str += strlen("nochunk");
-                       efi_nochunk = true;
-               }
+       size_t len = strlen(cmdline) + 1;
+       efi_status_t status;
+       char *str, *buf;
 
-               if (!strncmp(str, "novamap", 7)) {
-                       str += strlen("novamap");
-                       efi_novamap = true;
-               }
+       status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, len, (void **)&buf);
+       if (status != EFI_SUCCESS)
+               return status;
 
-               if (IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) &&
-                   !strncmp(str, "nosoftreserve", 7)) {
-                       str += strlen("nosoftreserve");
-                       efi_nosoftreserve = true;
-               }
+       str = skip_spaces(memcpy(buf, cmdline, len));
 
-               if (!strncmp(str, "disable_early_pci_dma", 21)) {
-                       str += strlen("disable_early_pci_dma");
-                       efi_disable_pci_dma = true;
-               }
+       while (*str) {
+               char *param, *val;
 
-               if (!strncmp(str, "no_disable_early_pci_dma", 24)) {
-                       str += strlen("no_disable_early_pci_dma");
-                       efi_disable_pci_dma = false;
-               }
+               str = next_arg(str, &param, &val);
 
-               /* Group words together, delimited by "," */
-               while (*str && *str != ' ' && *str != ',')
-                       str++;
+               if (!strcmp(param, "nokaslr")) {
+                       efi_nokaslr = true;
+               } else if (!strcmp(param, "quiet")) {
+                       efi_quiet = true;
+               } else if (!strcmp(param, "efi") && val) {
+                       efi_nochunk = parse_option_str(val, "nochunk");
+                       efi_novamap = parse_option_str(val, "novamap");
 
-               if (*str == ',')
-                       str++;
-       }
+                       efi_nosoftreserve = IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) &&
+                                           parse_option_str(val, "nosoftreserve");
 
+                       if (parse_option_str(val, "disable_early_pci_dma"))
+                               efi_disable_pci_dma = true;
+                       if (parse_option_str(val, "no_disable_early_pci_dma"))
+                               efi_disable_pci_dma = false;
+               }
+       }
+       efi_bs_call(free_pool, buf);
        return EFI_SUCCESS;
 }
 

diff --git a/drivers/firmware/efi/libstub/skip_spaces.c b/drivers/firmware/efi/libstub/skip_spaces.c
new file mode 100644 (file)
index 0000000..a700b3c
--- /dev/null
+++ b/drivers/firmware/efi/libstub/skip_spaces.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/ctype.h>
+#include <linux/types.h>
+
+char *skip_spaces(const char *str)
+{
+       while (isspace(*str))
+               ++str;
+       return (char *)str;
+}

diff --git a/drivers/firmware/efi/libstub/string.c b/drivers/firmware/efi/libstub/string.c
index ed10e3f602c5edde5b57a502dd7d0b74c0dd6fb0..1ac2f87647152ba75614a85f0c4650d0d21d18ae 100644 (file)
--- a/drivers/firmware/efi/libstub/string.c
+++ b/drivers/firmware/efi/libstub/string.c
@@ -6,6 +6,7 @@
  *  Copyright (C) 1991, 1992  Linus Torvalds
  */
 
+#include <linux/ctype.h>
 #include <linux/types.h>
 #include <linux/string.h>
 
@@ -56,3 +57,58 @@ int strncmp(const char *cs, const char *ct, size_t count)
        return 0;
 }
 #endif
+
+/* Works only for digits and letters, but small and fast */
+#define TOLOWER(x) ((x) | 0x20)
+
+static unsigned int simple_guess_base(const char *cp)
+{
+       if (cp[0] == '0') {
+               if (TOLOWER(cp[1]) == 'x' && isxdigit(cp[2]))
+                       return 16;
+               else
+                       return 8;
+       } else {
+               return 10;
+       }
+}
+
+/**
+ * simple_strtoull - convert a string to an unsigned long long
+ * @cp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ */
+
+unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
+{
+       unsigned long long result = 0;
+
+       if (!base)
+               base = simple_guess_base(cp);
+
+       if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
+               cp += 2;
+
+       while (isxdigit(*cp)) {
+               unsigned int value;
+
+               value = isdigit(*cp) ? *cp - '0' : TOLOWER(*cp) - 'a' + 10;
+               if (value >= base)
+                       break;
+               result = result * base + value;
+               cp++;
+       }
+       if (endp)
+               *endp = (char *)cp;
+
+       return result;
+}
+
+long simple_strtol(const char *cp, char **endp, unsigned int base)
+{
+       if (*cp == '-')
+               return -simple_strtoull(cp + 1, endp, base);
+
+       return simple_strtoull(cp, endp, base);
+}

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 52a1a2df2071b80ff41cb31dc39ee6eba9805f48..681b620d8d40e532454af578fe0acaa9451ad93d 100644 (file)
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -726,7 +726,7 @@ struct boot_params *efi_main(efi_handle_t handle,
        hdr->code32_start = (u32)bzimage_addr;
 
        /*
-        * make_boot_params() may have been called before efi_main(), in which
+        * efi_pe_entry() may have been called before efi_main(), in which
         * case this is the second time we parse the cmdline. This is ok,
         * parsing the cmdline multiple times does not have side-effects.
         */