Message ID | 20220713140949.45440-1-mark.rutland@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: fix KASAN_INLINE | expand |
On Wed, Jul 13, 2022 at 03:09:49PM +0100, Mark Rutland wrote: > Since commit: > > a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map") > > Kernels built with KASAN_INLINE=y die early in boot before producing any > console output. This is because the accesses made to the FDT (e.g. in > generic string processing functions) are instrumented with KASAN, and > with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus > shadow VA, resulting in a data abort. > > This patch fixes this by reverting commits: > > 7559d9f97581654f ("arm64: setup: drop early FDT pointer helpers") > bd0c3fa21878b6d0 ("arm64: idreg-override: use early FDT mapping in ID map") > > ... and using the TTBR1 fixmap mapping of the FDT. > > Note that due to a later commit: > > b65e411d6cc2f12a ("arm64: Save state of HCR_EL2.E2H before switch to EL1") > > ... which altered the prototype of init_feature_override() (and > invocation from head.S), commit bd0c3fa21878b6d0 does not revert > cleanly, and I've fixed that up manually. > > Fixes: a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map") > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Will Deacon <will@kernel.org> I'll leave this to Will to pick since the fixed commit is only in -next. Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Wed, 13 Jul 2022 15:09:49 +0100, Mark Rutland wrote: > Since commit: > > a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map") > > Kernels built with KASAN_INLINE=y die early in boot before producing any > console output. This is because the accesses made to the FDT (e.g. in > generic string processing functions) are instrumented with KASAN, and > with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus > shadow VA, resulting in a data abort. > > [...] Applied to arm64 (for-next/boot), thanks! [1/1] arm64: fix KASAN_INLINE https://git.kernel.org/arm64/c/c2cbc16df707 Cheers,
On Wed, Jul 13, 2022 at 03:09:49PM +0100, Mark Rutland wrote: > Since commit: > > a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map") > > Kernels built with KASAN_INLINE=y die early in boot before producing any > console output. This is because the accesses made to the FDT (e.g. in > generic string processing functions) are instrumented with KASAN, and > with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus > shadow VA, resulting in a data abort. > > This patch fixes this by reverting commits: > > 7559d9f97581654f ("arm64: setup: drop early FDT pointer helpers") > bd0c3fa21878b6d0 ("arm64: idreg-override: use early FDT mapping in ID map") > > ... and using the TTBR1 fixmap mapping of the FDT. > > Note that due to a later commit: > > b65e411d6cc2f12a ("arm64: Save state of HCR_EL2.E2H before switch to EL1") > > ... which altered the prototype of init_feature_override() (and > invocation from head.S), commit bd0c3fa21878b6d0 does not revert > cleanly, and I've fixed that up manually. > Whoops; this was meant to have: Signed-off-by: Mark Rutland <mark.rutland@arm.com> ... but I somehow messed that up. Will, are you happy to fold that in? Mark. > Fixes: a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map") > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Will Deacon <will@kernel.org> > --- > arch/arm64/include/asm/setup.h | 3 +++ > arch/arm64/kernel/head.S | 5 +++-- > arch/arm64/kernel/idreg-override.c | 17 +++++++++++------ > arch/arm64/kernel/setup.c | 15 +++++++++++++++ > 4 files changed, 32 insertions(+), 8 deletions(-) > > diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h > index 5f147a418281..6437df661700 100644 > --- a/arch/arm64/include/asm/setup.h > +++ b/arch/arm64/include/asm/setup.h > @@ -5,6 +5,9 @@ > > #include <uapi/asm/setup.h> > > +void *get_early_fdt_ptr(void); > +void early_fdt_map(u64 dt_phys); > + > /* > * These two variables are used in the head.S file. > */ > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S > index 29d641290293..cefe6a73ee54 100644 > --- a/arch/arm64/kernel/head.S > +++ b/arch/arm64/kernel/head.S > @@ -456,8 +456,9 @@ SYM_FUNC_START_LOCAL(__primary_switched) > #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) > bl kasan_early_init > #endif > - mov x0, x22 // pass FDT address in x0 > - mov x1, x20 // pass the full boot status > + mov x0, x21 // pass FDT address in x0 > + bl early_fdt_map // Try mapping the FDT early > + mov x0, x20 // pass the full boot status > bl init_feature_override // Parse cpu feature overrides > mov x0, x20 > bl finalise_el2 // Prefer VHE if possible > diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c > index 42883657f711..1b0542c69738 100644 > --- a/arch/arm64/kernel/idreg-override.c > +++ b/arch/arm64/kernel/idreg-override.c > @@ -262,11 +262,16 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases) > } while (1); > } > > -static __init const u8 *get_bootargs_cmdline(const void *fdt) > +static __init const u8 *get_bootargs_cmdline(void) > { > 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; > @@ -278,9 +283,9 @@ static __init const u8 *get_bootargs_cmdline(const void *fdt) > return strlen(prop) ? prop : NULL; > } > > -static __init void parse_cmdline(const void *fdt) > +static __init void parse_cmdline(void) > { > - const u8 *prop = get_bootargs_cmdline(fdt); > + const u8 *prop = get_bootargs_cmdline(); > > if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !prop) > __parse_cmdline(CONFIG_CMDLINE, true); > @@ -290,9 +295,9 @@ static __init void parse_cmdline(const void *fdt) > } > > /* Keep checkers quiet */ > -void init_feature_override(const void *fdt, u64 boot_status); > +void init_feature_override(u64 boot_status); > > -asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status) > +asmlinkage void __init init_feature_override(u64 boot_status) > { > int i; > > @@ -305,7 +310,7 @@ asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status) > > __boot_status = boot_status; > > - parse_cmdline(fdt); > + parse_cmdline(); > > for (i = 0; i < ARRAY_SIZE(regs); i++) { > if (regs[i]->override) > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index d0e6c7a291da..fea3223704b6 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -163,6 +163,21 @@ static void __init smp_build_mpidr_hash(void) > pr_warn("Large number of MPIDR hash buckets detected\n"); > } > > +static void *early_fdt_ptr __initdata; > + > +void __init *get_early_fdt_ptr(void) > +{ > + return early_fdt_ptr; > +} > + > +asmlinkage void __init early_fdt_map(u64 dt_phys) > +{ > + int fdt_size; > + > + early_fixmap_init(); > + early_fdt_ptr = fixmap_remap_fdt(dt_phys, &fdt_size, PAGE_KERNEL); > +} > + > static void __init setup_machine_fdt(phys_addr_t dt_phys) > { > int size; > -- > 2.30.2 >
On Wed, Jul 20, 2022 at 03:53:41PM +0100, Mark Rutland wrote: > On Wed, Jul 13, 2022 at 03:09:49PM +0100, Mark Rutland wrote: > > Since commit: > > > > a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map") > > > > Kernels built with KASAN_INLINE=y die early in boot before producing any > > console output. This is because the accesses made to the FDT (e.g. in > > generic string processing functions) are instrumented with KASAN, and > > with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus > > shadow VA, resulting in a data abort. > > > > This patch fixes this by reverting commits: > > > > 7559d9f97581654f ("arm64: setup: drop early FDT pointer helpers") > > bd0c3fa21878b6d0 ("arm64: idreg-override: use early FDT mapping in ID map") > > > > ... and using the TTBR1 fixmap mapping of the FDT. > > > > Note that due to a later commit: > > > > b65e411d6cc2f12a ("arm64: Save state of HCR_EL2.E2H before switch to EL1") > > > > ... which altered the prototype of init_feature_override() (and > > invocation from head.S), commit bd0c3fa21878b6d0 does not revert > > cleanly, and I've fixed that up manually. > > > > Whoops; this was meant to have: > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > ... but I somehow messed that up. > > Will, are you happy to fold that in? Thanks, yes, I'll add this now. Will
diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h index 5f147a418281..6437df661700 100644 --- a/arch/arm64/include/asm/setup.h +++ b/arch/arm64/include/asm/setup.h @@ -5,6 +5,9 @@ #include <uapi/asm/setup.h> +void *get_early_fdt_ptr(void); +void early_fdt_map(u64 dt_phys); + /* * These two variables are used in the head.S file. */ diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 29d641290293..cefe6a73ee54 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -456,8 +456,9 @@ SYM_FUNC_START_LOCAL(__primary_switched) #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) bl kasan_early_init #endif - mov x0, x22 // pass FDT address in x0 - mov x1, x20 // pass the full boot status + mov x0, x21 // pass FDT address in x0 + bl early_fdt_map // Try mapping the FDT early + mov x0, x20 // pass the full boot status bl init_feature_override // Parse cpu feature overrides mov x0, x20 bl finalise_el2 // Prefer VHE if possible diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c index 42883657f711..1b0542c69738 100644 --- a/arch/arm64/kernel/idreg-override.c +++ b/arch/arm64/kernel/idreg-override.c @@ -262,11 +262,16 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases) } while (1); } -static __init const u8 *get_bootargs_cmdline(const void *fdt) +static __init const u8 *get_bootargs_cmdline(void) { 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; @@ -278,9 +283,9 @@ static __init const u8 *get_bootargs_cmdline(const void *fdt) return strlen(prop) ? prop : NULL; } -static __init void parse_cmdline(const void *fdt) +static __init void parse_cmdline(void) { - const u8 *prop = get_bootargs_cmdline(fdt); + const u8 *prop = get_bootargs_cmdline(); if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !prop) __parse_cmdline(CONFIG_CMDLINE, true); @@ -290,9 +295,9 @@ static __init void parse_cmdline(const void *fdt) } /* Keep checkers quiet */ -void init_feature_override(const void *fdt, u64 boot_status); +void init_feature_override(u64 boot_status); -asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status) +asmlinkage void __init init_feature_override(u64 boot_status) { int i; @@ -305,7 +310,7 @@ asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status) __boot_status = boot_status; - parse_cmdline(fdt); + parse_cmdline(); for (i = 0; i < ARRAY_SIZE(regs); i++) { if (regs[i]->override) diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index d0e6c7a291da..fea3223704b6 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -163,6 +163,21 @@ static void __init smp_build_mpidr_hash(void) pr_warn("Large number of MPIDR hash buckets detected\n"); } +static void *early_fdt_ptr __initdata; + +void __init *get_early_fdt_ptr(void) +{ + return early_fdt_ptr; +} + +asmlinkage void __init early_fdt_map(u64 dt_phys) +{ + int fdt_size; + + early_fixmap_init(); + early_fdt_ptr = fixmap_remap_fdt(dt_phys, &fdt_size, PAGE_KERNEL); +} + static void __init setup_machine_fdt(phys_addr_t dt_phys) { int size;