On 12 September 2018 at 17:28, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > Hi, > > > On 12-09-18 17:06, Ard Biesheuvel wrote: >> >> On 12 September 2018 at 15:18, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>> >>> Before this commit we were only calling efi_parse_options() from >>> make_boot_params(), but make_boot_params() only gets called if the >>> kernel gets booted directly as an EFI executable. So when booted through >>> e.g. grub we ended up not parsing the commandline in the boot code. >>> >>> This makes the drivers/firmware/efi/libstub code ignore the "quiet" >>> commandline argument resulting in the following message being printed: >>> "EFI stub: UEFI Secure Boot is enabled." >>> >>> Despite the quiet request. This commits adds an extra call to >>> efi_parse_options() to efi_main() to make sure that the options are >>> always processed. This fixes quiet not working. >>> >>> This also fixes the libstub code ignoring nokaslr and efi=nochunk. >>> >>> Reported-by: Peter Robinson <pbrobinson@xxxxxxxxxx> >>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >>> --- >>> arch/x86/boot/compressed/eboot.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/arch/x86/boot/compressed/eboot.c >>> b/arch/x86/boot/compressed/eboot.c >>> index aaabf979cbd9..a4c85d37adc9 100644 >>> --- a/arch/x86/boot/compressed/eboot.c >>> +++ b/arch/x86/boot/compressed/eboot.c >>> @@ -738,6 +738,7 @@ efi_main(struct efi_config *c, struct boot_params >>> *boot_params) >>> struct desc_struct *desc; >>> void *handle; >>> efi_system_table_t *_table; >>> + const char *cmdline_ptr; >>> >>> efi_early = c; >>> >>> @@ -755,6 +756,15 @@ efi_main(struct efi_config *c, struct boot_params >>> *boot_params) >>> else >>> setup_boot_services32(efi_early); >>> >>> + /* >>> + * make_boot_params() 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. >>> + */ >>> + cmdline_ptr = (const char *)((u64)hdr->cmd_line_ptr | >>> + ((u64)boot_params->ext_cmd_line_ptr >>> << 32)); >>> + efi_parse_options(cmdline_ptr); >>> + >> >> >> This triggers a warning on 32-bit because sizeof(void*) != sizeof(u64) > > > Grumble, ok so looking at what other code accessing hdr->cmd_line_ptr + > boot_params->ext_cmd_line_ptr is most code seems to store it in an > unsigned long. So here is what I suggest for v2: > > --- a/arch/x86/boot/compressed/eboot.c > +++ b/arch/x86/boot/compressed/eboot.c > @@ -738,6 +738,7 @@ efi_main(struct efi_config *c, struct boot_params > *boot_params) > struct desc_struct *desc; > void *handle; > efi_system_table_t *_table; > + unsigned long cmdline_paddr; > > efi_early = c; > > @@ -755,6 +756,15 @@ efi_main(struct efi_config *c, struct boot_params > *boot_params) > else > setup_boot_services32(efi_early); > > + /* > + * make_boot_params() 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. > + */ > + cmdline_paddr = ((u64)hdr->cmd_line_ptr | > + ((u64)boot_params->ext_cmd_line_ptr << 32)); > + efi_parse_options((char *)cmdline_paddr); > + > /* > * If the boot loader gave us a value for secure_boot then we use > that, > * otherwise we ask the BIOS. > > If that looks ok to you I will send out a proper v2 with this. > Using a temp variable is fine. Using (const char *)(unsigned long)((u64)xxx ...) is also fine, whichever works best for you.
