Re: [PATCH v5 15/21] arm64: idreg-override: use early FDT mapping in ID map

Mark Rutland
2022-07-11T15:40:57Z

Hi Ard,

On Fri, Jun 24, 2022 at 05:06:45PM +0200, Ard Biesheuvel wrote:
> Instead of calling into the kernel to map the FDT into the kernel page
> tables before even calling start_kernel(), let's switch to the initial,
> temporary mapping of the device tree that has been added to the ID map.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Unfortunately, this patch breaks KASAN_INLINE=y, as the accesses to the idmap
alias of the FDT get a poison check, which faults, resulting in a panic()
before any of the command line is parsed.

It's a bit painful to fix this; I had hoped it would be simple to fix by
adding:

KASAN_SANITIZE_idreg-override.o := n

... to arch/arm64/kernel/Makefile, but there are a bunch of calls to
out-of-line functions that end up being instrumented (e.g. strncpy() and some
fdt_*() functions), and it doesn't look like it'll be simple to go fix those.

Can we revert this? IIUC this patch is an optimization rather being necessary
for later changes.

Thanks,
Mark.

> ---
> arch/arm64/kernel/head.S | 1 +
> arch/arm64/kernel/idreg-override.c | 17 ++++++-----------
> 2 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 8283ff848328..64ebff634b83 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -472,6 +472,7 @@ SYM_FUNC_START_LOCAL(__primary_switched)
> #endif
> mov x0, x21 // pass FDT address in x0
> bl early_fdt_map // Try mapping the FDT early
> + mov x0, x22 // pass FDT address in x0
> bl init_feature_override // Parse cpu feature overrides
> #ifdef CONFIG_RANDOMIZE_BASE
> tst x23, ~(MIN_KIMG_ALIGN - 1) // already running randomized?
> diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
> index 8a2ceb591686..f92836e196e5 100644
> --- a/arch/arm64/kernel/idreg-override.c
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -201,16 +201,11 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases)
> } while (1);
> }
>
> -static __init const u8 *get_bootargs_cmdline(void)
> +static __init const u8 *get_bootargs_cmdline(const void *fdt)
> {
> const u8 *prop;
> - void *fdt;
> int node;
>
> - fdt = get_early_fdt_ptr();
> - if (!fdt)
> - return NULL;
> -
> node = fdt_path_offset(fdt, "/chosen");
> if (node < 0)
> return NULL;
> @@ -222,9 +217,9 @@ static __init const u8 *get_bootargs_cmdline(void)
> return strlen(prop) ? prop : NULL;
> }
>
> -static __init void parse_cmdline(void)
> +static __init void parse_cmdline(const void *fdt)
> {
> - const u8 *prop = get_bootargs_cmdline();
> + const u8 *prop = get_bootargs_cmdline(fdt);
>
> if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !prop)
> __parse_cmdline(CONFIG_CMDLINE, true);
> @@ -234,9 +229,9 @@ static __init void parse_cmdline(void)
> }
>
> /* Keep checkers quiet */
> -void init_feature_override(void);
> +void init_feature_override(const void *fdt);
>
> -asmlinkage void __init init_feature_override(void)
> +asmlinkage void __init init_feature_override(const void *fdt)
> {
> int i;
>
> @@ -247,7 +242,7 @@ asmlinkage void __init init_feature_override(void)
> }
> }
>
> - parse_cmdline();
> + parse_cmdline(fdt);
>
> for (i = 0; i < ARRAY_SIZE(regs); i++) {
> if (regs[i]->override)
> --
> 2.35.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>