| 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
| ||
|
Message-ID: <87y2fcz1dg.wl-maz@kernel.org>
Date: Thu, 25 Feb 2021 14:08:11 +0000
From: Marc Zyngier <maz@...nel.org>
To: Will Deacon <will@...nel.org>
Cc: linux-kernel@...r.kernel.org, Max Uvarov <muvarov@...il.com>,
Rob Herring <robh@...nel.org>,
Ard Biesheuvel <ardb@...nel.org>,
Doug Anderson <dianders@...omium.org>,
Tyler Hicks <tyhicks@...ux.microsoft.com>,
Frank Rowand <frowand.list@...il.com>,
Arnd Bergmann <arnd@...db.de>,
Palmer Dabbelt <palmer@...belt.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Catalin Marinas <catalin.marinas@....com>,
kernel-team@...roid.com, linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] of/fdt: Append bootloader arguments when CMDLINE_EXTEND=y
On Thu, 25 Feb 2021 12:59:21 +0000,
Will Deacon <will@...nel.org> wrote:
>
> The Kconfig help text for CMDLINE_EXTEND is sadly duplicated across all
> architectures that implement it (arm, arm64, powerpc, riscv and sh), but
> they all seem to agree that the bootloader arguments will be appended to
> the CONFIG_CMDLINE. For example, on arm64:
>
> | The command-line arguments provided by the boot loader will be
> | appended to the default kernel command string.
>
> This also matches the behaviour of the EFI stub, which parses the
> bootloader arguments first if CMDLINE_EXTEND is set, as well as the
> out-of-tree CMDLINE_EXTEND implementation in Android.
>
> However, the behaviour in the upstream fdt code appears to be the other
> way around: CONFIG_CMDLINE is appended to the bootloader arguments.
>
> Fix the code to follow the documentation by moving the cmdline
> processing out into a new function, early_init_dt_retrieve_cmdline(),
> and copying CONFIG_CMDLINE to the beginning of the cmdline buffer rather
> than concatenating it onto the end.
>
> Cc: Max Uvarov <muvarov@...il.com>
> Cc: Rob Herring <robh@...nel.org>
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Cc: Marc Zyngier <maz@...nel.org>
> Cc: Doug Anderson <dianders@...omium.org>
> Cc: Tyler Hicks <tyhicks@...ux.microsoft.com>
> Cc: Frank Rowand <frowand.list@...il.com>
> Fixes: 34b82026a507 ("fdt: fix extend of cmd line")
> Signed-off-by: Will Deacon <will@...nel.org>
> ---
> drivers/of/fdt.c | 64 +++++++++++++++++++++++++++++-------------------
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index dcc1dd96911a..83b9d065e58d 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1033,11 +1033,48 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
> return 0;
> }
>
> +static int __init cmdline_from_bootargs(unsigned long node, void *dst, int sz)
> +{
> + int l;
> + const char *p = of_get_flat_dt_prop(node, "bootargs", &l);
> +
> + if (!p || l <= 0)
> + return -EINVAL;
> +
> + return strlcpy(dst, p, min(l, sz));
> +}
> +
> +/* dst is a zero-initialised buffer of COMMAND_LINE_SIZE bytes */
> +static void __init early_init_dt_retrieve_cmdline(unsigned long node, char *dst)
> +{
> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) {
> + /* Copy CONFIG_CMDLINE to the start of destination buffer */
> + size_t idx = strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +
> + /* Check that we have enough space to concatenate */
> + if (idx + 1 >= COMMAND_LINE_SIZE)
> + return;
> +
> + /* Append the bootloader arguments */
> + dst[idx++] = ' ';
> + cmdline_from_bootargs(node, &dst[idx], COMMAND_LINE_SIZE - idx);
> + } else if (IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
> + /* Just use CONFIG_CMDLINE */
> + strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> + } else if (IS_ENABLED(CONFIG_CMDLINE_FROM_BOOTLOADER)) {
> + /* Use CONFIG_CMDLINE if no arguments from bootloader. */
> + if (cmdline_from_bootargs(node, dst, COMMAND_LINE_SIZE) <= 0)
> + strlcpy(dst, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> + } else {
Do we have any arch that can end-up not defining any of the 3 above
cases? We should be able to just have the above case as the catch-all,
and drop the one below.
> + /* Just use bootloader arguments */
> + cmdline_from_bootargs(node, dst, COMMAND_LINE_SIZE);
> + }
> +}
> +
> int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> int depth, void *data)
> {
> int l;
> - const char *p;
> const void *rng_seed;
>
> pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
> @@ -1047,30 +1084,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> return 0;
>
> early_init_dt_check_for_initrd(node);
> -
> - /* Retrieve command line */
> - p = of_get_flat_dt_prop(node, "bootargs", &l);
> - if (p != NULL && l > 0)
> - strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
> -
> - /*
> - * CONFIG_CMDLINE is meant to be a default in case nothing else
> - * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> - * is set in which case we override whatever was found earlier.
> - */
> -#ifdef CONFIG_CMDLINE
> -#if defined(CONFIG_CMDLINE_EXTEND)
> - strlcat(data, " ", COMMAND_LINE_SIZE);
> - strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#elif defined(CONFIG_CMDLINE_FORCE)
> - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#else
> - /* No arguments from boot loader, use kernel's cmdl*/
> - if (!((char *)data)[0])
> - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#endif
> -#endif /* CONFIG_CMDLINE */
> -
> + early_init_dt_retrieve_cmdline(node, data);
> pr_debug("Command line is: %s\n", (char *)data);
>
> rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l);
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
>
Other than the above nit:
Reviewed-by: Marc Zyngier <maz@...nel.org>
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists