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

Ard Biesheuvel
2022-07-12T17:05:32Z

On Mon, 11 Jul 2022 at 17:39, Mark Rutland <mark.rutland@arm.com> wrote:
>
> 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.
>

Ugh. I suppose the KASAN projection produces invalid shadow addresses
for the TTBR0 range, so there is no way we can map some zero shadow
for the FDT?

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

Indeed, let's revert this for now, along with the follow-up patch that
removes the early FDT fixmap remap hack.

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