diff mbox series

[10/12] efi/libstub: Unify command line param parsing

Message ID 20170404160910.28115-1-ard.biesheuvel@linaro.org
State Accepted
Commit 60f38de7a8d4e816100ceafd1b382df52527bd50
Headers show
Series [1/2] efi/libstub: Skip GOP with PIXEL_BLT_ONLY format | expand

Commit Message

Ard Biesheuvel April 4, 2017, 4:09 p.m. UTC 
Merge the parsing of the command line carried out in arm-stub.c with
the handling in efi_parse_options. Note that this also fixes the
missing handling of CONFIG_CMDLINE_FORCE=y, in which case the builtin
command line should supersede the one passed by the firmware.

Cc: Matt Fleming <matt@codeblueprint.co.uk>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 drivers/firmware/efi/libstub/arm-stub.c        | 24 +++++++-----------------
 drivers/firmware/efi/libstub/arm64-stub.c      |  4 +---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 19 +++++++++++--------
 drivers/firmware/efi/libstub/efistub.h         |  2 ++
 include/linux/efi.h                            |  2 +-
 5 files changed, 22 insertions(+), 29 deletions(-)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Catalin Marinas April 7, 2017, 3:58 p.m. UTC | #1 
Hi Ard,

On 4 April 2017 at 17:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Update the allocation logic for the virtual mapping of the UEFI runtime

> services to start from a randomized base address if KASLR is in effect,

> and if the UEFI firmware exposes an implementation of EFI_RNG_PROTOCOL.

>

> This makes it more difficult to predict the location of exploitable

> data structures in the runtime UEFI firmware, which increases robustness

> against attacks. Note that these regions are only mapped during the

> time a runtime service call is in progress, and only on a single CPU

> at a time, bit give the lack of a downside, let's enable it nonetheless.

>

> Cc: Ingo Molnar <mingo@kernel.org>

> Cc: Borislav Petkov <bp@alien8.de>

> Cc: Matt Fleming <matt@codeblueprint.co.uk>

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

> ---

>  drivers/firmware/efi/libstub/arm-stub.c | 49 ++++++++++++++++++++++++---------

>  1 file changed, 36 insertions(+), 13 deletions(-)


This patch causes a boot regression on Juno and Seattle (reported by
James Morse) with defconfig. On Juno I get a Synchronous Exception
(translation fault, second level). If I build the kernel with 64K
pages, it boots ok.

See below, though likely wrapped by gmail:

EFI stub: Booting Linux Kernel...
EFI stub: Using DTB from configuration table

Synchronous Exception at 0x00000000F82A045C

 X0 0x0000000000000000 X1 0x00000000FDF01614 X2 0x7BF580F1AD8AE581 X3
0x0000000000000000
 X4 0x0000000000000000 X5 0x00000000FDFBFC60 X6 0x42F27CEB1CE1E5BC X7
0x7BF580F1AD8AE581
 X8 0x0000000000000000 X9 0x0000000700000000 X10 0x00000000FB5C0000
X11 0x00000000FB5C5FFF
 X12 0x0000000000000000 X13 0x000000000000000E X14 0x8000000000000001
X15 0x0000000000000000
 X16 0x00000000FEFFF5E0 X17 0x0000000000000000 X18 0x0000000000000000
X19 0x00000000FDFB0018
 X20 0x00000000F8390000 X21 0x00000000FB5BDA18 X22 0x00000000FEFFF5D0
X23 0x00000009FFFF0000
 X24 0x00000000FEFFF598 X25 0x0000000000000000 X26 0x0000000000000000
X27 0x0000000000000000
 X28 0x0000000000000000 FP 0x00000000FEFFF510 LR 0x00000000F82A0454

 V0 0xA4775AF0716632D7 DB7509796C96DACB V1 0xE67211106C932C4D 4F7141B24C78074F
 V2 0x550C7DC3243185BE 12835B01D807AA98 V3 0xC19BF1749BDC06A7 80DEB1FE72BE5D74
 V4 0x240CA1CC0FC19DC6 EFBE4786E49B69C1 V5 0x76F988DA5CB0A9DC 4A7484AA2DE92C6F
 V6 0xBF597FC7B00327C8 A831C66D983E5152 V7 0x1429296706CA6351 D5A79147C6E00BF3
 V8 0x0000000000000000 2E1B213827B70A85 V9 0x0000000000000000 766A0ABB650A7354
 V10 0x0000000000000000 A81A664BA2BFE8A1 V11 0x0000000000000000 D6990624D192E819
 V12 0x0000000000000000 1E376C0819A4C116 V13 0x0000000000000000 4ED8AA4A391C0CB3
 V14 0x0000000000000000 78A5636F748F82EE V15 0x0000000000000000 A4506CEB90BEFFFA
 V16 0x3C83709547545153 BC990E9F897D4FA8 V17 0xBEBEF297C5EC0578 E1D99AE8C2B92607
 V18 0x13242833C5F05BAB 101C24C521387481 V19 0x0A50ABCAD0BDDA12 996865483E880969
 V20 0x6C4ABAA53A9BE1CB 4416D9F479B08221 V21 0x60952147C3A68574 55B8AE51435C1A1A
 V22 0x9FEB2A3B4AB8D3BF 88C1883495C7F76F V23 0xD0C224BC8FB77E09 3DB8D233CF470963
 V24 0x0E72963E02D21C93 C95B757DA62B3A12 V25 0x2A77DBA1E4EE5D5C E08479A1B557DFA8
 V26 0xB593F86668CA8129 927A0019CDEC1A76 V27 0xFF03D7BC13D5FFF6 BBA1EFEF4B817FFA
 V28 0xE3FEE1F77F5FF733 5FBC3CE8E54BBB53 V29 0x4E8582114F72CE7E EED3F6BF4B0F5BDC
 V30 0xFFFA7FFBF72FF9F7 F8F06FDB9FF5EDEA V31 0xDE3785D1AEB3DAB7 12E08FF9AD0F87FF

 SP 0x00000000FEFFF500 ELR 0x00000000F82A045C SPSR 0x60000209 FPSR 0x00000000
 ESR 0x96000006 FAR 0x0000000000000000

 ESR : EC 0x25 IL 0x1 ISS 0x00000006

Data abort: Translation fault, second level

Stack dump:
 00000FEFFF400: 996865483E880969 0A50ABCAD0BDDA12 4416D9F479B08221
6C4ABAA53A9BE1CB
 00000FEFFF420: 55B8AE51435C1A1A 60952147C3A68574 88C1883495C7F76F
9FEB2A3B4AB8D3BF
 00000FEFFF440: 3DB8D233CF470963 D0C224BC8FB77E09 C95B757DA62B3A12
0E72963E02D21C93
 00000FEFFF460: E08479A1B557DFA8 2A77DBA1E4EE5D5C 927A0019CDEC1A76
B593F86668CA8129
 00000FEFFF480: BBA1EFEF4B817FFA FF03D7BC13D5FFF6 5FBC3CE8E54BBB53
E3FEE1F77F5FF733
 00000FEFFF4A0: EED3F6BF4B0F5BDC 4E8582114F72CE7E F8F06FDB9FF5EDEA
FFFA7FFBF72FF9F7
 00000FEFFF4C0: 12E08FF9AD0F87FF DE3785D1AEB3DAB7 00000000F82A045C
0000000060000209
 00000FEFFF4E0: 0000000000000000 0000000096000006 0000000000000000
7BF580F1AD8AE581
> 00000FEFFF500: 00000000FEFFF520 00000000FDFE1658 00000000FEFFF5C0 00000000F829D4CC

 00000FEFFF520: 00000000FB5C6040 00000000FCA74698 0000000000000000
0000000000000000
 00000FEFFF540: 0000000000000000 0000000000000000 00000000000000B0
000000F2F865A8B0
 00000FEFFF560: 00000000FB5C6040 0000000000000000 0000000000000000
00000000F86C0000
 00000FEFFF580: 000000000000503F 0000000080080000 0000000000F20000
0000000000000000
 00000FEFFF5A0: 11D295625B1B31A1 3B7269C9A0003F8E 4A3823DC9042A9DE
6A5180D0DE7AFB96
 00000FEFFF5C0: 00000000FEFFF5E0 00000000FDFD3AA8 0000000080080000
00000000FB5D6C18
 00000FEFFF5E0: 00000000FEFFF660 00000000F859C830 00000000F865A8B0
0000000000000000


Thanks,

Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel April 7, 2017, 4:02 p.m. UTC | #2 
On 7 April 2017 at 16:58, Catalin Marinas <catalin.marinas@arm.com> wrote:
> Hi Ard,

>

> On 4 April 2017 at 17:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>> Update the allocation logic for the virtual mapping of the UEFI runtime

>> services to start from a randomized base address if KASLR is in effect,

>> and if the UEFI firmware exposes an implementation of EFI_RNG_PROTOCOL.

>>

>> This makes it more difficult to predict the location of exploitable

>> data structures in the runtime UEFI firmware, which increases robustness

>> against attacks. Note that these regions are only mapped during the

>> time a runtime service call is in progress, and only on a single CPU

>> at a time, bit give the lack of a downside, let's enable it nonetheless.

>>

>> Cc: Ingo Molnar <mingo@kernel.org>

>> Cc: Borislav Petkov <bp@alien8.de>

>> Cc: Matt Fleming <matt@codeblueprint.co.uk>

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

>> ---

>>  drivers/firmware/efi/libstub/arm-stub.c | 49 ++++++++++++++++++++++++---------

>>  1 file changed, 36 insertions(+), 13 deletions(-)

>

> This patch causes a boot regression on Juno and Seattle (reported by

> James Morse) with defconfig. On Juno I get a Synchronous Exception

> (translation fault, second level). If I build the kernel with 64K

> pages, it boots ok.

>

> See below, though likely wrapped by gmail:

>

> EFI stub: Booting Linux Kernel...

> EFI stub: Using DTB from configuration table

>

> Synchronous Exception at 0x00000000F82A045C

>

>  X0 0x0000000000000000 X1 0x00000000FDF01614 X2 0x7BF580F1AD8AE581 X3

> 0x0000000000000000

>  X4 0x0000000000000000 X5 0x00000000FDFBFC60 X6 0x42F27CEB1CE1E5BC X7

> 0x7BF580F1AD8AE581

>  X8 0x0000000000000000 X9 0x0000000700000000 X10 0x00000000FB5C0000

> X11 0x00000000FB5C5FFF

>  X12 0x0000000000000000 X13 0x000000000000000E X14 0x8000000000000001

> X15 0x0000000000000000

>  X16 0x00000000FEFFF5E0 X17 0x0000000000000000 X18 0x0000000000000000

> X19 0x00000000FDFB0018

>  X20 0x00000000F8390000 X21 0x00000000FB5BDA18 X22 0x00000000FEFFF5D0

> X23 0x00000009FFFF0000

>  X24 0x00000000FEFFF598 X25 0x0000000000000000 X26 0x0000000000000000

> X27 0x0000000000000000

>  X28 0x0000000000000000 FP 0x00000000FEFFF510 LR 0x00000000F82A0454

>

>  V0 0xA4775AF0716632D7 DB7509796C96DACB V1 0xE67211106C932C4D 4F7141B24C78074F

>  V2 0x550C7DC3243185BE 12835B01D807AA98 V3 0xC19BF1749BDC06A7 80DEB1FE72BE5D74

>  V4 0x240CA1CC0FC19DC6 EFBE4786E49B69C1 V5 0x76F988DA5CB0A9DC 4A7484AA2DE92C6F

>  V6 0xBF597FC7B00327C8 A831C66D983E5152 V7 0x1429296706CA6351 D5A79147C6E00BF3

>  V8 0x0000000000000000 2E1B213827B70A85 V9 0x0000000000000000 766A0ABB650A7354

>  V10 0x0000000000000000 A81A664BA2BFE8A1 V11 0x0000000000000000 D6990624D192E819

>  V12 0x0000000000000000 1E376C0819A4C116 V13 0x0000000000000000 4ED8AA4A391C0CB3

>  V14 0x0000000000000000 78A5636F748F82EE V15 0x0000000000000000 A4506CEB90BEFFFA

>  V16 0x3C83709547545153 BC990E9F897D4FA8 V17 0xBEBEF297C5EC0578 E1D99AE8C2B92607

>  V18 0x13242833C5F05BAB 101C24C521387481 V19 0x0A50ABCAD0BDDA12 996865483E880969

>  V20 0x6C4ABAA53A9BE1CB 4416D9F479B08221 V21 0x60952147C3A68574 55B8AE51435C1A1A

>  V22 0x9FEB2A3B4AB8D3BF 88C1883495C7F76F V23 0xD0C224BC8FB77E09 3DB8D233CF470963

>  V24 0x0E72963E02D21C93 C95B757DA62B3A12 V25 0x2A77DBA1E4EE5D5C E08479A1B557DFA8

>  V26 0xB593F86668CA8129 927A0019CDEC1A76 V27 0xFF03D7BC13D5FFF6 BBA1EFEF4B817FFA

>  V28 0xE3FEE1F77F5FF733 5FBC3CE8E54BBB53 V29 0x4E8582114F72CE7E EED3F6BF4B0F5BDC

>  V30 0xFFFA7FFBF72FF9F7 F8F06FDB9FF5EDEA V31 0xDE3785D1AEB3DAB7 12E08FF9AD0F87FF

>

>  SP 0x00000000FEFFF500 ELR 0x00000000F82A045C SPSR 0x60000209 FPSR 0x00000000

>  ESR 0x96000006 FAR 0x0000000000000000

>

>  ESR : EC 0x25 IL 0x1 ISS 0x00000006

>

> Data abort: Translation fault, second level

>

> Stack dump:

>  00000FEFFF400: 996865483E880969 0A50ABCAD0BDDA12 4416D9F479B08221

> 6C4ABAA53A9BE1CB

>  00000FEFFF420: 55B8AE51435C1A1A 60952147C3A68574 88C1883495C7F76F

> 9FEB2A3B4AB8D3BF

>  00000FEFFF440: 3DB8D233CF470963 D0C224BC8FB77E09 C95B757DA62B3A12

> 0E72963E02D21C93

>  00000FEFFF460: E08479A1B557DFA8 2A77DBA1E4EE5D5C 927A0019CDEC1A76

> B593F86668CA8129

>  00000FEFFF480: BBA1EFEF4B817FFA FF03D7BC13D5FFF6 5FBC3CE8E54BBB53

> E3FEE1F77F5FF733

>  00000FEFFF4A0: EED3F6BF4B0F5BDC 4E8582114F72CE7E F8F06FDB9FF5EDEA

> FFFA7FFBF72FF9F7

>  00000FEFFF4C0: 12E08FF9AD0F87FF DE3785D1AEB3DAB7 00000000F82A045C

> 0000000060000209

>  00000FEFFF4E0: 0000000000000000 0000000096000006 0000000000000000

> 7BF580F1AD8AE581

>> 00000FEFFF500: 00000000FEFFF520 00000000FDFE1658 00000000FEFFF5C0 00000000F829D4CC

>  00000FEFFF520: 00000000FB5C6040 00000000FCA74698 0000000000000000

> 0000000000000000

>  00000FEFFF540: 0000000000000000 0000000000000000 00000000000000B0

> 000000F2F865A8B0

>  00000FEFFF560: 00000000FB5C6040 0000000000000000 0000000000000000

> 00000000F86C0000

>  00000FEFFF580: 000000000000503F 0000000080080000 0000000000F20000

> 0000000000000000

>  00000FEFFF5A0: 11D295625B1B31A1 3B7269C9A0003F8E 4A3823DC9042A9DE

> 6A5180D0DE7AFB96

>  00000FEFFF5C0: 00000000FEFFF5E0 00000000FDFD3AA8 0000000080080000

> 00000000FB5D6C18

>  00000FEFFF5E0: 00000000FEFFF660 00000000F859C830 00000000F865A8B0

> 0000000000000000

>


As I replied to James already, this is a bit surprising given the lack
of EFI_RNG_PROTOCOL in the AMI UEFI implementation. I will look into
it.

In the mean time, you can circumvent the issue by passing 'nokaslr' on
the kernel command line.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 02049ff25c6b..ac3222f6f805 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -18,8 +18,6 @@ 
 
 #include "efistub.h"
 
-bool __nokaslr;
-
 efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
 			     void *__image, void **__fh)
 {
@@ -153,18 +151,6 @@  unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
 		goto fail;
 	}
 
-	/* check whether 'nokaslr' was passed on the command line */
-	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
-		static const u8 default_cmdline[] = CONFIG_CMDLINE;
-		const u8 *str, *cmdline = cmdline_ptr;
-
-		if (IS_ENABLED(CONFIG_CMDLINE_FORCE))
-			cmdline = default_cmdline;
-		str = strstr(cmdline, "nokaslr");
-		if (str == cmdline || (str > cmdline && *(str - 1) == ' '))
-			__nokaslr = true;
-	}
-
 	si = setup_graphics(sys_table);
 
 	status = handle_kernel_image(sys_table, image_addr, &image_size,
@@ -176,9 +162,13 @@  unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
 		goto fail_free_cmdline;
 	}
 
-	status = efi_parse_options(cmdline_ptr);
-	if (status != EFI_SUCCESS)
-		pr_efi_err(sys_table, "Failed to parse EFI cmdline options\n");
+	if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
+	    IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
+	    cmdline_size == 0)
+		efi_parse_options(CONFIG_CMDLINE);
+
+	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0)
+		efi_parse_options(cmdline_ptr);
 
 	secure_boot = efi_get_secureboot(sys_table);
 
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index eae693eb3e91..b4c2589d7c91 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -16,8 +16,6 @@ 
 
 #include "efistub.h"
 
-extern bool __nokaslr;
-
 efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
 {
 	u64 tg;
@@ -52,7 +50,7 @@  efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
 	u64 phys_seed = 0;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
-		if (!__nokaslr) {
+		if (!nokaslr()) {
 			status = efi_get_random_bytes(sys_table_arg,
 						      sizeof(phys_seed),
 						      (u8 *)&phys_seed);
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 3290fae0b38f..2e17d2b8787c 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -32,6 +32,13 @@ 
 
 static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
 
+static int __section(.data) __nokaslr;
+
+int __pure nokaslr(void)
+{
+	return __nokaslr;
+}
+
 #define EFI_MMAP_NR_SLACK_SLOTS	8
 
 struct file_info {
@@ -409,17 +416,13 @@  static efi_status_t efi_file_close(void *handle)
  * environments, first in the early boot environment of the EFI boot
  * stub, and subsequently during the kernel boot.
  */
-efi_status_t efi_parse_options(char *cmdline)
+efi_status_t efi_parse_options(char const *cmdline)
 {
 	char *str;
 
-	/*
-	 * Currently, the only efi= option we look for is 'nochunk', which
-	 * is intended to work around known issues on certain x86 UEFI
-	 * versions. So ignore for now on other architectures.
-	 */
-	if (!IS_ENABLED(CONFIG_X86))
-		return EFI_SUCCESS;
+	str = strstr(cmdline, "nokaslr");
+	if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
+		__nokaslr = 1;
 
 	/*
 	 * If no EFI parameters were specified on the cmdline we've got
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 71c4d0e3c4ed..a7a2a2c3f199 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -24,6 +24,8 @@ 
 #define EFI_ALLOC_ALIGN		EFI_PAGE_SIZE
 #endif
 
+extern int __pure nokaslr(void);
+
 void efi_char16_printk(efi_system_table_t *, efi_char16_t *);
 
 efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg, void *__image,
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 94d34e0be24f..e485e87615d1 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1471,7 +1471,7 @@  efi_status_t handle_cmdline_files(efi_system_table_t *sys_table_arg,
 				  unsigned long *load_addr,
 				  unsigned long *load_size);
 
-efi_status_t efi_parse_options(char *cmdline);
+efi_status_t efi_parse_options(char const *cmdline);
 
 efi_status_t efi_setup_gop(efi_system_table_t *sys_table_arg,
 			   struct screen_info *si, efi_guid_t *proto,