diff mbox series

[for-4.9-stable] efi/libstub: Unify command line param parsing

Message ID 20190606102513.16321-1-ard.biesheuvel@linaro.org
State New
Headers show
Series [for-4.9-stable] efi/libstub: Unify command line param parsing | expand

Commit Message

Ard Biesheuvel June 6, 2019, 10:25 a.m. UTC 
Commit 60f38de7a8d4e816100ceafd1b382df52527bd50 upstream.

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.

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

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bhe@redhat.com
Cc: bhsharma@redhat.com
Cc: bp@alien8.de
Cc: eugene@hp.com
Cc: evgeny.kalugin@intel.com
Cc: jhugo@codeaurora.org
Cc: leif.lindholm@linaro.org
Cc: linux-efi@vger.kernel.org
Cc: mark.rutland@arm.com
Cc: roy.franz@cavium.com
Cc: rruigrok@codeaurora.org
Link: http://lkml.kernel.org/r/20170404160910.28115-1-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>

[ardb: fix up merge conflicts with 4.9.180]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
This fixes the GCC build issue reported by Eike.

Note that testing of arm64 stable kernels should cover CONFIG_RANDOMIZE_BASE,
since it has a profound impact on how the kernel binary gets put together.

 drivers/firmware/efi/libstub/arm-stub.c        | 23 ++++++--------------
 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(+), 28 deletions(-)

-- 
2.20.1

Comments

Sasha Levin June 6, 2019, 1:22 p.m. UTC | #1 
On Thu, Jun 06, 2019 at 12:25:13PM +0200, Ard Biesheuvel wrote:
>Commit 60f38de7a8d4e816100ceafd1b382df52527bd50 upstream.

>

>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.

>

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

>Cc: Linus Torvalds <torvalds@linux-foundation.org>

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

>Cc: Peter Zijlstra <peterz@infradead.org>

>Cc: Thomas Gleixner <tglx@linutronix.de>

>Cc: bhe@redhat.com

>Cc: bhsharma@redhat.com

>Cc: bp@alien8.de

>Cc: eugene@hp.com

>Cc: evgeny.kalugin@intel.com

>Cc: jhugo@codeaurora.org

>Cc: leif.lindholm@linaro.org

>Cc: linux-efi@vger.kernel.org

>Cc: mark.rutland@arm.com

>Cc: roy.franz@cavium.com

>Cc: rruigrok@codeaurora.org

>Link: http://lkml.kernel.org/r/20170404160910.28115-1-ard.biesheuvel@linaro.org

>Signed-off-by: Ingo Molnar <mingo@kernel.org>

>[ardb: fix up merge conflicts with 4.9.180]

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

>---

>This fixes the GCC build issue reported by Eike.

>

>Note that testing of arm64 stable kernels should cover CONFIG_RANDOMIZE_BASE,

>since it has a profound impact on how the kernel binary gets put together.


Should this fix be applied to 4.9 as well?

I see it in 4.14+

--
Thanks,
Sasha
Ard Biesheuvel June 6, 2019, 1:33 p.m. UTC | #2 
On Thu, 6 Jun 2019 at 15:22, Sasha Levin <sashal@kernel.org> wrote:
>

> On Thu, Jun 06, 2019 at 12:25:13PM +0200, Ard Biesheuvel wrote:

> >Commit 60f38de7a8d4e816100ceafd1b382df52527bd50 upstream.

> >

> >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.

> >

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

> >Cc: Linus Torvalds <torvalds@linux-foundation.org>

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

> >Cc: Peter Zijlstra <peterz@infradead.org>

> >Cc: Thomas Gleixner <tglx@linutronix.de>

> >Cc: bhe@redhat.com

> >Cc: bhsharma@redhat.com

> >Cc: bp@alien8.de

> >Cc: eugene@hp.com

> >Cc: evgeny.kalugin@intel.com

> >Cc: jhugo@codeaurora.org

> >Cc: leif.lindholm@linaro.org

> >Cc: linux-efi@vger.kernel.org

> >Cc: mark.rutland@arm.com

> >Cc: roy.franz@cavium.com

> >Cc: rruigrok@codeaurora.org

> >Link: http://lkml.kernel.org/r/20170404160910.28115-1-ard.biesheuvel@linaro.org

> >Signed-off-by: Ingo Molnar <mingo@kernel.org>

> >[ardb: fix up merge conflicts with 4.9.180]

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

> >---

> >This fixes the GCC build issue reported by Eike.

> >

> >Note that testing of arm64 stable kernels should cover CONFIG_RANDOMIZE_BASE,

> >since it has a profound impact on how the kernel binary gets put together.

>

> Should this fix be applied to 4.9 as well?

>

> I see it in 4.14+

>


I don't understand this question. The fix is proposed for v4.9 because
it fixes a build error with GCC that was caused by a backport of one
of the clang enablement patches.
gregkh@linuxfoundation.org June 6, 2019, 3:26 p.m. UTC | #3 
On Thu, Jun 06, 2019 at 12:25:13PM +0200, Ard Biesheuvel wrote:
> Commit 60f38de7a8d4e816100ceafd1b382df52527bd50 upstream.

> 

> 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.

> 

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

> Cc: Linus Torvalds <torvalds@linux-foundation.org>

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

> Cc: Peter Zijlstra <peterz@infradead.org>

> Cc: Thomas Gleixner <tglx@linutronix.de>

> Cc: bhe@redhat.com

> Cc: bhsharma@redhat.com

> Cc: bp@alien8.de

> Cc: eugene@hp.com

> Cc: evgeny.kalugin@intel.com

> Cc: jhugo@codeaurora.org

> Cc: leif.lindholm@linaro.org

> Cc: linux-efi@vger.kernel.org

> Cc: mark.rutland@arm.com

> Cc: roy.franz@cavium.com

> Cc: rruigrok@codeaurora.org

> Link: http://lkml.kernel.org/r/20170404160910.28115-1-ard.biesheuvel@linaro.org

> Signed-off-by: Ingo Molnar <mingo@kernel.org>

> [ardb: fix up merge conflicts with 4.9.180]

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

> ---

> This fixes the GCC build issue reported by Eike.

> 

> Note that testing of arm64 stable kernels should cover CONFIG_RANDOMIZE_BASE,

> since it has a profound impact on how the kernel binary gets put together.


Good idea.  Is that in any arm64 stable kernel configuration?  If not, I
can ask the Linaro build/test people to add it there.

And isn't it part of kernel.ci already?  We get the results of that for
stable releases.

Anyway, thanks for the patch, now queued up.

thanks,

greg k-h
Ard Biesheuvel June 7, 2019, 9:51 a.m. UTC | #4 
On Thu, 6 Jun 2019 at 17:26, Greg KH <gregkh@linuxfoundation.org> wrote:
>

> On Thu, Jun 06, 2019 at 12:25:13PM +0200, Ard Biesheuvel wrote:

> > Commit 60f38de7a8d4e816100ceafd1b382df52527bd50 upstream.

> >

> > 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.

> >

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

> > Cc: Linus Torvalds <torvalds@linux-foundation.org>

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

> > Cc: Peter Zijlstra <peterz@infradead.org>

> > Cc: Thomas Gleixner <tglx@linutronix.de>

> > Cc: bhe@redhat.com

> > Cc: bhsharma@redhat.com

> > Cc: bp@alien8.de

> > Cc: eugene@hp.com

> > Cc: evgeny.kalugin@intel.com

> > Cc: jhugo@codeaurora.org

> > Cc: leif.lindholm@linaro.org

> > Cc: linux-efi@vger.kernel.org

> > Cc: mark.rutland@arm.com

> > Cc: roy.franz@cavium.com

> > Cc: rruigrok@codeaurora.org

> > Link: http://lkml.kernel.org/r/20170404160910.28115-1-ard.biesheuvel@linaro.org

> > Signed-off-by: Ingo Molnar <mingo@kernel.org>

> > [ardb: fix up merge conflicts with 4.9.180]

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

> > ---

> > This fixes the GCC build issue reported by Eike.

> >

> > Note that testing of arm64 stable kernels should cover CONFIG_RANDOMIZE_BASE,

> > since it has a profound impact on how the kernel binary gets put together.

>

> Good idea.  Is that in any arm64 stable kernel configuration?  If not, I

> can ask the Linaro build/test people to add it there.

>


It is not in the defconfig, so it probably doesn't get a lot of coverage.

> And isn't it part of kernel.ci already?  We get the results of that for

> stable releases.

>


kernelci has lost its usefulness for me, since i have to wade through
pages and pages of mips and riscv results before i get to the meat.

> Anyway, thanks for the patch, now queued up.

>

> thanks,

>

> greg k-h
diff mbox series

Patch

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 993aa56755f6..726d1467b778 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -18,7 +18,6 @@ 
 
 #include "efistub.h"
 
-bool __nokaslr;
 
 static int efi_get_secureboot(efi_system_table_t *sys_table_arg)
 {
@@ -268,18 +267,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,
@@ -291,9 +278,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);
 	if (secure_boot > 0)
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 959d9b8d4845..f7a6970e9abc 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -24,8 +24,6 @@ 
 
 #include "efistub.h"
 
-extern bool __nokaslr;
-
 efi_status_t check_platform_features(efi_system_table_t *sys_table_arg)
 {
 	u64 tg;
@@ -60,7 +58,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 09d10dcf1fc6..1c963c4d1bde 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;
+}
+
 /*
  * Allow the platform to override the allocation granularity: this allows
  * systems that have the capability to run with a larger page size to deal
@@ -351,17 +358,13 @@  void efi_free(efi_system_table_t *sys_table_arg, unsigned long size,
  * 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 fac67992bede..d0e5acab547f 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -15,6 +15,8 @@ 
  */
 #undef __init
 
+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 e6711bf9f0d1..02c4f16685b6 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1427,7 +1427,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,