Messages in this thread | | | From | Ard Biesheuvel <> | Date | Thu, 18 Aug 2022 12:00:39 +0200 | Subject | Re: [PATCH] arm64: fix rodata=full |
| |
On Wed, 17 Aug 2022 at 17:40, Mark Rutland <mark.rutland@arm.com> wrote: > > On arm64, "rodata=full" has been suppored (but not documented) since > commit: > > c55191e96caa9d78 ("arm64: mm: apply r/o permissions of VM areas to its linear alias as well") > > As it's necessary to determine the rodata configuration early during > boot, arm64 has an early_param() handler for this, whereas init/main.c > has a __setup() handler which is run later. > > Unfortunately, this split meant that since commit: > > f9a40b0890658330 ("init/main.c: return 1 from handled __setup() functions") > > ... passing "rodata=full" would result in a spurious warning from the > __setup() handler (though RO permissions would be configured > appropriately). > > Further, "rodata=full" has been broken since commit: > > 0d6ea3ac94ca77c5 ("lib/kstrtox.c: add "false"/"true" support to kstrtobool()") > > ... which caused strtobool() to parse "full" as false (in addition to > many other values not documented for the "rodata=" kernel parameter. > > This patch fixes this breakage by: > > * Moving the core parameter parser to an __early_param(), such that it > is available early. > > * Adding an (optional) arch hook which arm64 can use to parse "full". > > * Updating the documentation to mention that "full" is valid for arm64. > > * Having the core parameter parser handle "on" and "off" explicitly, > such that any undocumented values (e.g. typos such as "ful") are > reported as errors rather than being silently accepted. > > Note that __setup() and early_param() have opposite conventions for > their return values, where __setup() uses 1 to indicate a parameter was > handled and early_param() uses 0 to indicate a parameter was handled. > > Fixes: f9a40b0890658330 ("init/main.c: return 1 from handled __setup() functions") > Fixes: 0d6ea3ac94ca77c5 ("lib/kstrtox.c: add "false"/"true" support to kstrtobool()") > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Jagdish Gediya <jvgediya@linux.ibm.com> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Will Deacon <will@kernel.org>
Yuck, thanks for cleaning that up.
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> --- > .../admin-guide/kernel-parameters.txt | 2 ++ > arch/arm64/include/asm/setup.h | 17 +++++++++++++++++ > arch/arm64/mm/mmu.c | 18 ------------------ > init/main.c | 18 +++++++++++++++--- > 4 files changed, 34 insertions(+), 21 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index d7f30902fda02..426fa892d311a 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -5331,6 +5331,8 @@ > rodata= [KNL] > on Mark read-only kernel memory as read-only (default). > off Leave read-only kernel memory writable for debugging. > + full Mark read-only kernel memory and aliases as read-only > + [arm64] > > rockchip.usb_uart > Enable the uart passthrough on the designated usb port > diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h > index 6437df6617009..f4af547ef54ca 100644 > --- a/arch/arm64/include/asm/setup.h > +++ b/arch/arm64/include/asm/setup.h > @@ -3,6 +3,8 @@ > #ifndef __ARM64_ASM_SETUP_H > #define __ARM64_ASM_SETUP_H > > +#include <linux/string.h> > + > #include <uapi/asm/setup.h> > > void *get_early_fdt_ptr(void); > @@ -14,4 +16,19 @@ void early_fdt_map(u64 dt_phys); > extern phys_addr_t __fdt_pointer __initdata; > extern u64 __cacheline_aligned boot_args[4]; > > +static inline bool arch_parse_debug_rodata(char *arg) > +{ > + extern bool rodata_enabled; > + extern bool rodata_full; > + > + if (arg && !strcmp(arg, "full")) { > + rodata_enabled = true; > + rodata_full = true; > + return true; > + } > + > + return false; > +} > +#define arch_parse_debug_rodata arch_parse_debug_rodata > + > #endif > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index db7c4e6ae57bb..e7ad44585f40a 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -642,24 +642,6 @@ static void __init map_kernel_segment(pgd_t *pgdp, void *va_start, void *va_end, > vm_area_add_early(vma); > } > > -static int __init parse_rodata(char *arg) > -{ > - int ret = strtobool(arg, &rodata_enabled); > - if (!ret) { > - rodata_full = false; > - return 0; > - } > - > - /* permit 'full' in addition to boolean options */ > - if (strcmp(arg, "full")) > - return -EINVAL; > - > - rodata_enabled = true; > - rodata_full = true; > - return 0; > -} > -early_param("rodata", parse_rodata); > - > #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 > static int __init map_entry_trampoline(void) > { > diff --git a/init/main.c b/init/main.c > index 91642a4e69be6..1fe7942f5d4a8 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -1446,13 +1446,25 @@ static noinline void __init kernel_init_freeable(void); > > #if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) > bool rodata_enabled __ro_after_init = true; > + > +#ifndef arch_parse_debug_rodata > +static inline bool arch_parse_debug_rodata(char *str) { return false; } > +#endif > + > static int __init set_debug_rodata(char *str) > { > - if (strtobool(str, &rodata_enabled)) > + if (arch_parse_debug_rodata(str)) > + return 0; > + > + if (str && !strcmp(str, "on")) > + rodata_enabled = true; > + else if (str && !strcmp(str, "off")) > + rodata_enabled = false; > + else > pr_warn("Invalid option string for rodata: '%s'\n", str); > - return 1; > + return 0; > } > -__setup("rodata=", set_debug_rodata); > +early_param("rodata", set_debug_rodata); > #endif > > #ifdef CONFIG_STRICT_KERNEL_RWX > -- > 2.30.2 >
| |