| Message ID | 20220827213009.44316-1-alexander.sverdlin@nokia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: kasan: Only map modules if CONFIG_KASAN_VMALLOC=n | expand |
On Sat, Aug 27, 2022 at 11:32 PM Alexander A Sverdlin <alexander.sverdlin@nokia.com> wrote: > - create_mapping((void *)MODULES_VADDR, (void *)(PKMAP_BASE + PMD_SIZE)); > + if (!IS_ENABLED(CONFIG_KASAN_VMALLOC) && IS_ENABLED(CONFIG_MODULES)) > + create_mapping((void *)MODULES_VADDR, (void *)(MODULES_END)); So the way I understand it is that modules are first and foremost loaded into the area MODULES_VADDR .. MODULES_END, and then after that is out, they get loaded into VMALLOC. See arch/arm/kernel/module.c, module_alloc(). If you do this, how are the addresses between MODULES_VADDR..MODULES_END shadowed when using CONFIG_KASAN_VMALLOC? > + create_mapping((void *)PKMAP_BASE, (void *)(PKMAP_BASE + PMD_SIZE)); (Splitting this in two steps if probably good in any case.) Pls keep me on CC for Kasan ARM patches, thanks! (Maybe I should add some MAINTAINERS blurb.) Yours, Linus Walleij
Hello Linus, On 31/08/2022 11:30, Linus Walleij wrote: >> - create_mapping((void *)MODULES_VADDR, (void *)(PKMAP_BASE + PMD_SIZE)); >> + if (!IS_ENABLED(CONFIG_KASAN_VMALLOC) && IS_ENABLED(CONFIG_MODULES)) >> + create_mapping((void *)MODULES_VADDR, (void *)(MODULES_END)); > So the way I understand it is that modules are first and foremost loaded into > the area MODULES_VADDR .. MODULES_END, and then after that is out, > they get loaded into VMALLOC. See arch/arm/kernel/module.c, module_alloc(). yes, but both areas are managed by __vmalloc_node_range(). > If you do this, how are the addresses between MODULES_VADDR..MODULES_END > shadowed when using CONFIG_KASAN_VMALLOC? That's the thing, __vmalloc_node_range() doesn't differentiate between address ranges and tries first to recreate [already existing] shadow mapping, and then vfree() unconditionally frees the mapping and the page. vmalloc() KASAN handling is generic, module_alloc() implemented via vmalloc() is however ARM-specific. Even though we could teach vmalloc() about MODULES_VADDR and MODULES_END (and don't call kasan_ instrumentation on these), but, this is ARM-specifics that it's used for this range. >> + create_mapping((void *)PKMAP_BASE, (void *)(PKMAP_BASE + PMD_SIZE)); > (Splitting this in two steps if probably good in any case.) > > Pls keep me on CC for Kasan ARM patches, thanks! (Maybe I should add some > MAINTAINERS blurb.)
On Thu, Sep 1, 2022 at 10:42 AM Alexander Sverdlin <alexander.sverdlin@nokia.com> wrote: > >> - create_mapping((void *)MODULES_VADDR, (void *)(PKMAP_BASE + PMD_SIZE)); > >> + if (!IS_ENABLED(CONFIG_KASAN_VMALLOC) && IS_ENABLED(CONFIG_MODULES)) > >> + create_mapping((void *)MODULES_VADDR, (void *)(MODULES_END)); > > So the way I understand it is that modules are first and foremost loaded into > > the area MODULES_VADDR .. MODULES_END, and then after that is out, > > they get loaded into VMALLOC. See arch/arm/kernel/module.c, module_alloc(). > > yes, but both areas are managed by __vmalloc_node_range(). Owww! > > If you do this, how are the addresses between MODULES_VADDR..MODULES_END > > shadowed when using CONFIG_KASAN_VMALLOC? > > That's the thing, __vmalloc_node_range() doesn't differentiate between address > ranges and tries first to recreate [already existing] shadow mapping, and then > vfree() unconditionally frees the mapping and the page. > > vmalloc() KASAN handling is generic, module_alloc() implemented via vmalloc() > is however ARM-specific. Even though we could teach vmalloc() about MODULES_VADDR > and MODULES_END (and don't call kasan_ instrumentation on these), but, this is > ARM-specifics that it's used for this range. OK I get it. Maybe this warrants a comment in the code explaining the above behaviour (also in commitlog) so nobody gets confused. With that: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours. Linus Walleij
Hello Linus, On 31/08/2022 11:30, Linus Walleij wrote: > Pls keep me on CC for Kasan ARM patches, thanks! (Maybe I should add some > MAINTAINERS blurb.) there is one patch which barely triggered any interest, but the problem has been spotted during KASAN usage on ARM ("ARM: module: Teach unwinder about PLTs"): https://lore.kernel.org/linux-arm-kernel/2bb016da-363d-5aac-fe7c-066cfe52d738@nokia.com/t/ Would you like to review it so that I can add it to Russel's patch system?
diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c index 29caee9c79ce3..64790661bdc40 100644 --- a/arch/arm/mm/kasan_init.c +++ b/arch/arm/mm/kasan_init.c @@ -268,12 +268,14 @@ void __init kasan_init(void) /* * 1. The module global variables are in MODULES_VADDR ~ MODULES_END, - * so we need to map this area. + * so we need to map this area if CONFIG_KASAN_VMALLOC=n. * 2. PKMAP_BASE ~ PKMAP_BASE+PMD_SIZE's shadow and MODULES_VADDR * ~ MODULES_END's shadow is in the same PMD_SIZE, so we can't * use kasan_populate_zero_shadow. */ - create_mapping((void *)MODULES_VADDR, (void *)(PKMAP_BASE + PMD_SIZE)); + if (!IS_ENABLED(CONFIG_KASAN_VMALLOC) && IS_ENABLED(CONFIG_MODULES)) + create_mapping((void *)MODULES_VADDR, (void *)(MODULES_END)); + create_mapping((void *)PKMAP_BASE, (void *)(PKMAP_BASE + PMD_SIZE)); /* * KAsan may reuse the contents of kasan_early_shadow_pte directly, so