Messages in this thread | | | Subject | Re: [RFC/PATCH v3] arm64: define MODULES_VADDR by module_alloc_base | From | Miles Chen <> | Date | Thu, 19 Oct 2017 19:08:02 +0800 |
| |
On Tue, 2017-10-17 at 15:43 +0100, Will Deacon wrote: > On Wed, Aug 30, 2017 at 07:46:33PM +0800, Miles Chen wrote: > > After the kernel ASLR, the module virtual address is moved to > > [module_alloc_base, module_alloc_base + MODULES_VSIZE). > > However, the MODULES_VADDR is still defined as a constant and functions > > like is_vmalloc_or_module_addr() or dump functions will not able to > > use correct module range information. > > > > The patch omits modules information in virtual kenrel memory layout if > > the modules area is fully mapped whitin vmalloc area. > > > > Use module_alloc_base to define MODULES_VADDR. I tested the patch under > > three different conditions: > > 1.CONFIG_RANDOMIZE_BASE=y, seed=0 > > CONFIG_KASAN=n > > 2.CONFIG_RANDOMIZE_BASE=y, seed=0x2304909023333333 > > CONFIG_KASAN=n > > 3.CONFIG_RANDOMIZE_BASE=y, seed=0x2304909023333333 > > CONFIG_KASAN=y > > > > test log: > > [...] > > > diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c > > index ca74a2a..18be771 100644 > > --- a/arch/arm64/mm/dump.c > > +++ b/arch/arm64/mm/dump.c > > @@ -29,15 +29,36 @@ > > #include <asm/pgtable-hwdef.h> > > #include <asm/ptdump.h> > > > > -static const struct addr_marker address_markers[] = { > > +enum marker { > > +#ifdef CONFIG_KASAN > > + E_KASAN_SHADOW_START, > > + E_KASAN_SHADOW_END, > > +#endif > > + E_MODULES_VADDR, > > + E_MODULES_END, > > + E_VMALLOC_START, > > + E_VMALLOC_END, > > + E_FIXADDR_START, > > + E_FIXADDR_TOP, > > + E_PCI_IO_START, > > + E_PCI_IO_END, > > +#ifdef CONFIG_SPARSEMEM_VMEMMAP > > + E_VMEMMAP_START, > > + E_VMEMMAP_END, > > +#endif > > + E_PAGE_OFFSET, > > + E_NR_MARKER, > > +}; > > + > > +static struct addr_marker address_markers[] = { > > #ifdef CONFIG_KASAN > > { KASAN_SHADOW_START, "Kasan shadow start" }, > > { KASAN_SHADOW_END, "Kasan shadow end" }, > > #endif > > - { MODULES_VADDR, "Modules start" }, > > - { MODULES_END, "Modules end" }, > > - { VMALLOC_START, "vmalloc() Area" }, > > - { VMALLOC_END, "vmalloc() End" }, > > + { -1, "Modules start" }, > > + { -1, "Modules end" }, > > + { -1, "vmalloc() Area" }, > > + { -1, "vmalloc() End" }, > > { FIXADDR_START, "Fixmap start" }, > > { FIXADDR_TOP, "Fixmap end" }, > > { PCI_IO_START, "PCI I/O start" }, > > @@ -362,10 +383,32 @@ void ptdump_walk_pgd(struct seq_file *m, struct ptdump_info *info) > > note_page(&st, 0, 0, 0); > > } > > > > +static void fixup_markers(void) > > +{ > > + int i; > > + > > + address_markers[E_MODULES_VADDR].start_address = MODULES_VADDR; > > + address_markers[E_MODULES_END].start_address = MODULES_END; > > + address_markers[E_VMALLOC_START].start_address = VMALLOC_START; > > + address_markers[E_VMALLOC_END].start_address = VMALLOC_END; > > + > > + if (MODULES_VADDR < VMALLOC_START) { > > + address_markers[E_MODULES_END].start_address = > > + (MODULES_END < VMALLOC_START) ? > > + MODULES_END : VMALLOC_START; > > + } else { > > + /* modules is contains in vamlloc area, don't show them */ > > + for (i = E_MODULES_VADDR; i <= E_NR_MARKER - 2; i++) > > + address_markers[i] = address_markers[i + 2]; > > + } > > +} > > This all seems a bit over-engineered to me and we end up having to maintain > enum marker in conjunction with the address_markers array. Fixing the array > up at runtime also worries me slightly, because we end up with the last two > entries being duplicated. That doesn't seem to hurt for now, but it's weird > and I can imagine it causing problems in the future. > > Is there not a simpler fix here? > > Will
Sorry for the late reply.
How about using static defined STATIC_MODULES_VADDR, and STATIC_MODULES_END, so we do not need to fixed the address_markers[].
- { MODULES_VADDR, "Modules start" }, - { MODULES_END, "Modules end" }, + { STATIC_MODULES_VADDR, "Static modules start" }, + { STATIC_MODULES_END, "Static modules end" },
and for the modules: 1. set the [STATIC_MODULES_VADDR,STATIC_MODULES_END) as the default module area (e.g., KASAN=n + KASLR=y with no random seed; KASLR=y and KASN=y or KASLR=n)
2. with KASAN=n + KASLR=y with valid random seed, kernel modules are loaded to vmalloc area. We can still the STATIC_MODULES_* entries in the address_markers[]. We should find nothing between [STATIC_MODULES_VADDR,STATIC_MODULES_END) in this case.
Miles
| |