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

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

[Ard Biesheuvel]

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(-)

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,
-- 
2.20.1

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

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]

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.

Confirmed, this patch works for me on top of 4.9.180.
-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 313 bytes --]

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

[Sasha Levin]

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

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

[Ard Biesheuvel]

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.

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

[Greg KH]

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

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

[Ard Biesheuvel]

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