linux-kernel - Re: [PATCH 3/3] efi/libstub: consider CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXFe1ZYuR=45VhwMyHcZhSTQVwLrbZDWpgG7Zqw+Awws_A@mail.gmail.com>
Date: Sat, 12 Oct 2024 15:36:07 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Jonathan Marek <jonathan@...ek.ca>
Cc: linux-efi@...r.kernel.org, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] efi/libstub: consider CONFIG_CMDLINE for initrd= and
 dtb= options

On Sat, 12 Oct 2024 at 14:04, Jonathan Marek <jonathan@...ek.ca> wrote:
>
>
>
> On 10/12/24 3:54 AM, Ard Biesheuvel wrote:
> > On Sat, 12 Oct 2024 at 00:52, Jonathan Marek <jonathan@...ek.ca> wrote:
> >>
> >> Replace cmdline with CONFIG_CMDLINE when it should be used instead of
> >> load_options.
> >>
> >> In the EXTEND case, it may be necessary to combine both CONFIG_CMDLINE and
> >> load_options. In that case, keep the old behavior and print a warning about
> >> the incorrect behavior.
> >>
> >
> > The core kernel has its own handling for EXTEND/FORCE, so while we
> > should parse it in the EFI stub to look for options that affect the
> > stub's own behavior, we should not copy it into the command line that
> > the stub provides to the core kernel.
> >
> > E.g., drivers/of/fdt.c takes the bootargs from the DT and combines
> > them with CONFIG_CMDLINE.
> >
> >
>
> I'm aware of that - the replacement the commit message is referring to
> is specifically for handle_cmdline_files() which this commit is modifying.
>

Ah ok - I missed that.

This is the kind of context that I'd expect in a cover letter, i.e.,
that the command line handling is inconsistent, and that we obtain the
command line from the loaded image twice.

Also, the fact the initrd= handling and dtb= are special, because
a) multiple initrd=  arguments are processed in order, and the files
concatenated,
b) the filenames are consumed as UTF-16 as they are plugged into the
file I/O protocols

> Currently efistub completely ignores initrd= and dtb= options provided
> through CONFIG_CMDLINE (handle_cmdline_files() only parses the EFI options)
>

Indeed. You haven't explained why this is a problem. initrd= and dtb=
contain references to files in the file system, and this does not seem
like something CONFIG_EXTEND was intended for.

> For the EXTEND case, I didn't implement the full solution because its
> more complex and EXTEND is not available on arm64 anyway, so I went with
> just printing a warning instead.

This code is shared between all architectures, so what arm64 does or
does not support is irrelevant.

Can you explain your use case please?