Messages in this thread |  | | Subject | Re: [PATCH] arm64: correct modules range of kernel virtual memory layout | From | Miles Chen <> | Date | Tue, 8 Aug 2017 13:27:25 +0800 |
| |
On Tue, 2017-08-08 at 12:44 +0800, Miles Chen wrote: > On Mon, 2017-08-07 at 15:01 +0100, Will Deacon wrote: > > On Mon, Aug 07, 2017 at 02:18:00PM +0100, Ard Biesheuvel wrote: > > > On 7 August 2017 at 14:16, Will Deacon <will.deacon@arm.com> wrote: > > > > On Mon, Aug 07, 2017 at 07:04:46PM +0800, Miles Chen wrote: > > > >> The commit f80fb3a3d508 ("arm64: add support for kernel ASLR") > > > >> moved module virtual address to > > > >> [module_alloc_base, module_alloc_base + MODULES_VSIZE). > > > >> > > > >> Display module information of the virtual kernel > > > >> memory layout by using module_alloc_base. > > > >> > > > >> testing output: > > > >> 1) Current implementation: > > > >> Virtual kernel memory layout: > > > >> modules : 0xffffff8000000000 - 0xffffff8008000000 ( 128 MB) > > > >> 2) this patch + KASLR: > > > >> Virtual kernel memory layout: > > > >> modules : 0xffffff8000560000 - 0xffffff8008560000 ( 128 MB) > > > >> 3) this patch + KASLR and a dummy seed: > > > >> Virtual kernel memory layout: > > > >> modules : 0xffffffa7df637000 - 0xffffffa7e7637000 ( 128 MB) > > > >> > > > >> Signed-off-by: Miles Chen <miles.chen@mediatek.com> > > > >> --- > > > >> arch/arm64/mm/init.c | 5 +++-- > > > >> 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > Does this mean the modules code in our pt dumper is busted > > > > (arch/arm64/mm/dump.c)? Also, what about KASAN, which uses these addresses > > > > too (in kasan_init)? Should we just remove MODULES_VADDR and MODULES_END > > > > altogether? > > > > > > > > > > I don't think we need this patch. The 'module' line simply prints the > > > VA region that is reserved for modules. The fact that we end up > > > putting them elsewhere when running randomized does not necessarily > > > mean this line should reflect that. > > > > I was more concerned by other users of MODULES_VADDR tbh, although I see > > now that we don't randomize the module region if kasan is enabled. Still, > > the kcore code adds the modules region as a separate area (distinct from > > vmalloc) if MODULES_VADDR is defined, the page table dumping code uses > > MODULES_VADDR to identify the module region and I think we'll get false > > positives from is_vmalloc_or_module_addr, which again uses the static > > region. > > > > So, given that MODULES_VADDR never points at the module area, can't we get > > rid of it? > > Agreed.MODULES_VADDR should be phased out. Considering the kernel > modules live somewhere between [VMALLOC_START, VMALLOC_END) now: > (arch/arm64/kernel/module.c:module_alloc). I suggest the following > changes: > > 1. is_vmalloc_or_module_addr() should return is_vmalloc_addr() directly > 2. arch/arm64/mm/dump.c does not need MODULES_VADDR and MODULES_END. > 3. kasan uses [module_alloc_base, module_alloc_base + MODULES_VSIZE) to > get the shadow memory? (the kernel modules still live in this range when > kasan is enabled) > 4. remove modules line in kernel memory layout > (optional, thanks for Ard's feedback) > 5. remove MODULE_VADDR, MODULES_END definition
I was wrong about this. is_vmalloc_or_module_addr() is defined in mm/vmalloc and it uses MODULES_VADDR and MODULES_END. May it is better to give MODULES_VADDR and MODULES_END proper values, not remove them.
> Miles > > > > Will > >
|  |