Linux-EFI Archive on lore.kernel.org
 help / color / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org
Cc: Ard Biesheuvel <ardb@kernel.org>,
	nivedita@alum.mit.edu, mingo@kernel.org, lukas@wunner.de,
	atish.patra@wdc.com
Subject: [PATCH 17/19] efi/libstub: Clean up command line parsing routine
Date: Mon, 10 Feb 2020 17:02:46 +0100
Message-ID: <20200210160248.4889-18-ardb@kernel.org> (raw)
In-Reply-To: <20200210160248.4889-1-ardb@kernel.org>

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>
---
 drivers/firmware/efi/libstub/Makefile          |  3 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c | 79 +++++++-------------
 drivers/firmware/efi/libstub/string.c          | 63 ++++++++++++++++
 3 files changed, 91 insertions(+), 54 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 74a097ef1780..82cb9f9c08ce 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -39,7 +39,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 \
+				   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 db23be5dc69b..b3621ceb3480 100644
--- 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);
+	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 + 1));
 
-		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/string.c b/drivers/firmware/efi/libstub/string.c
index ed10e3f602c5..33dfb83be239 100644
--- 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,65 @@ int strncmp(const char *cs, const char *ct, size_t count)
 	return 0;
 }
 #endif
+
+char *skip_spaces(const char *str)
+{
+	while (isspace(*str))
+		++str;
+	return (char *)str;
+}
+
+/* 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);
+}
-- 
2.17.1


  parent reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-10 16:02 [PATCH 00/19] EFI stub early spring cleaning part 2 Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 01/19] efi/libstub/x86: Remove pointless zeroing of apm_bios_info Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 02/19] efi/libstub/x86: Avoid overflowing code32_start on PE entry Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 03/19] efi/libstub: Use hidden visiblity for all source files Ard Biesheuvel
2020-02-24 23:15   ` Atish Patra
2020-02-24 23:36     ` Ard Biesheuvel
2020-02-25 19:18       ` Atish Patra
2020-02-10 16:02 ` [PATCH 04/19] efi/libstub/arm: Relax FDT alignment requirement Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 05/19] efi/libstub: Move memory map handling and allocation routines to mem.c Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 06/19] efi/libstub: Simplify efi_high_alloc() and rename to efi_allocate_pages() Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 07/19] efi/libstub/x86: Incorporate eboot.c into libstub Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 08/19] efi/libstub: Use consistent type names for file I/O protocols Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 09/19] efi/libstub: Move stub specific declarations into efistub.h Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 10/19] efi/libstub/x86: Permit bootparams struct to be allocated above 4 GB Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 11/19] efi/libstub/x86: Permit cmdline data " Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 12/19] efi/libstub: Move efi_random_alloc() into separate source file Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 13/19] efi/libstub: Move get_dram_base() into arm-stub.c Ard Biesheuvel
2020-02-17  1:17   ` Atish Patra
2020-02-17  8:37     ` Ard Biesheuvel
2020-02-26 23:34       ` Atish Patra
2020-02-27  7:38         ` Ard Biesheuvel
2020-02-27  7:48           ` Atish Patra
2020-02-27  7:50             ` Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 14/19] efi/libstub: Move file I/O support code into separate file Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 15/19] efi/libstub: Rewrite file I/O routine Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 16/19] efi/libstub: Take soft and hard memory limits into account for initrd loading Ard Biesheuvel
2020-02-10 16:02 ` Ard Biesheuvel [this message]
2020-02-10 16:02 ` [PATCH 18/19] efi/libstub: Expose LocateDevicePath boot service Ard Biesheuvel
2020-02-10 16:02 ` [PATCH 19/19] efi/libstub: Make the LoadFile EFI protocol accessible Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200210160248.4889-18-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=atish.patra@wdc.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mingo@kernel.org \
    --cc=nivedita@alum.mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-EFI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-efi/0 linux-efi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-efi linux-efi/ https://lore.kernel.org/linux-efi \
		linux-efi@vger.kernel.org
	public-inbox-index linux-efi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-efi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git