Re: [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr.

[Dave Anderson <anderson@redhat.com>]

Right -- it's an obvious bug/regression, and an obvious fix.  Can
we please get this addressed before 5.4 is released?

Thanks,
  Dave


----- Original Message -----
> On 10/09/2019 02:09 PM, Chris von Recklinghausen wrote:
> > On 10/08/2019 01:54 PM, Chris von Recklinghausen wrote:
> >> With the reshuffling of kernel VA space to support 52 bits, the
> >> kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they
> >> are
> >> based on VA_START, but VA_START no longer points to the base of the kernel
> >> virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has
> >> default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on
> >> PAGE_OFFSET, so simply remove the arm64 definitions of them.
> >>
> >> Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space")
> >> ---
> >>  arch/arm64/include/asm/pgtable.h | 3 ---
> >>  1 file changed, 3 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/pgtable.h
> >> b/arch/arm64/include/asm/pgtable.h
> >> index 7576df00eb50..8330810f699e 100644
> >> --- a/arch/arm64/include/asm/pgtable.h
> >> +++ b/arch/arm64/include/asm/pgtable.h
> >> @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct
> >> vm_area_struct *vma,
> >>  
> >>  #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0)
> >>  
> >> -#define kc_vaddr_to_offset(v)	((v) & ~PAGE_END)
> >> -#define kc_offset_to_vaddr(o)	((o) | PAGE_END)
> >> -
> >>  #ifdef CONFIG_ARM64_PA_BITS_52
> >>  #define phys_to_ttbr(addr)	(((addr) | ((addr) >> 46)) &
> >>  TTBR_BADDR_MASK_52)
> >>  #else
> > This patch addresses the breakage of access to unity mapped kernel
> > addresses via /proc/kcore, which is a regression.
> >
> Description of problem as encountered last March by Dave Anderson with RHEL8
> development kernel updated and experiment repeated with 5.4.0-rc2 kernel:
> 
> For the last few days I've been attempting (unsuccessfully) to get the
> crash utility running with the test 52-bit VA kernel.  During
> initialization,
> it can read the mapped kernel text/static-data region OK, but as soon as
> any reads from the unity-mapped region are attempted, it returns garbage.
> Interestingly enough, one of the major changes in the kernel patch is that
> it moves the unity-mapped (aka the "direct linear map") region from the
> very top of kernel virtual address space to the very beginning.
> 
> Finally out of frustration, I figured I'd take the crash utility out of
> the picture, and sanity check /proc/kcore using gdb alone.  And as it turns
> out, it appears that something in the patch has broken /proc/kcore, at
> least w/respect to unity-mapped addresses.  (I haven't attempted to read
> module, vmalloc or the vmemmap regions, because I need to be able to
> access unity-mapped region as a prerequisite)
> 
> Here's my evidence...
> 
> First, these are the results of a "good" gdb session running with the
> 5.4.0-rc2 kernel, which I will compare with a session running with the
> 5.4.0-rc2 test kernel with kc_* change.  For a simple test, I'm
> looking at the data pointed to by the static kernel data symbol
> "vmcoreinfo_data":
> 
>   static unsigned char *vmcoreinfo_data;
> 
> It gets initialized via get_zeroed_page() with a page of unity-mapped
> memory:
> 
>   vmcoreinfo_data = (unsigned char *)get_zeroed_page(GFP_KERNEL);
> 
> It then gets filled in with various ASCII strings of kernel data
> that get passed in an ELF note in /proc/vmcore in the kdump kernel.
> The strings are all linefeed-terminated (i.e., not NULL-terminated),
> and the first string is always the "OSRELEASE=" string.
> 
> Here is the static data address of "vmcoreinfo_data" in the 5.4.0-rc2
> kernel:
> 
>   $ nm -Bn vmlinux | grep vmcoreinfo_data
>   ffff8000116cbad8 B vmcoreinfo_data
>   ...
> 
> Here's what a gdb session looks like on 5.4.0-rc2 running with the fix,
> where
> first the virtual address above is verified:
>  
>   $ gdb vmlinux /proc/kcore
>   ...
>   (gdb) p &vmcoreinfo_data
>   $1 = (unsigned char **) 0xffff8000116cbad8 <vmcoreinfo_data>
>   (gdb)
> 
> Here's the unity-mapped address that was returned by get_zeroed_page():
> 
>   (gdb) printf "%lx\n", vmcoreinfo_data
>   ffff009f43670000
>   (gdb)
> 
> And since it's just a giant string, it can be viewed like so:
> 
>   (gdb) printf "%s\n", vmcoreinfo_data
> OSRELEASE=5.4.0-rc2.tmp2+
> PAGESIZE=65536
> SYMBOL(init_uts_ns)=ffff8000113a56e8
> SYMBOL(node_online_map)=ffff80001139d570
> SYMBOL(swapper_pg_dir)=ffff800010ef0000
> SYMBOL(_stext)=ffff800010081000
> SYMBOL(vmap_area_list)=ffff80001141c360
> SYMBOL(mem_section)=ffff009f7eedc5c0
> LENGTH(mem_section)=1024
> SIZE(mem_section)=16
> OFFSET(mem_section.section_mem_map)=0
> SIZE(page)=64
> SIZE(pglist_data)=6720
> SIZE(zone)=1728
> SIZE(free_area)=88
> SIZE(list_head)=16
> SIZE(nodemask_t)=8
> OFFSET(page.flags)=0
> OFFSET(page._refcount)=52
> OFFSET(page.mapping)=24
> OFFSET(page.lru)=8
> OFFSET(page._mapcount)=48
> OFFSET(page.private)=40
> OFFSET(page.compound_dtor)=16
> OFFSET(page.compound_order)=17
> OFFSET(page.compound_head)=8
> OFFSET(pglist_data.node_zones)=0
> OFFSET(pglist_data.nr_zones)=5984
> OFFSET(pglist_data.node_start_pfn)=5992
> OFFSET(pglist_data.node_spanned_pages)=6008
> OFFSET(pglist_data.node_id)=6016
> OFFSET(zone.free_area)=192
> OFFSET(zone.vm_stat)=1536
> OFFSET(zone.spanned_pages)=104
> OFFSET(free_area.free_list)=0
> OFFSET(list_head.next)=0
> OFFSET(list_head.prev)=8
> OFFSET(vmap_area.va_start)=0
> OFFSET(vmap_area.list)=40
> LENGTH(zone.free_area)=14
> SYMBOL(log_buf)=ffff8000113ceeb8
> SYMBOL(log_buf_len)=ffff8000113ceeb0
> --Type <RET> for more, q to quit, c to continue without paging--q
> 
>  
> Now, let's try the same thing running with the kernel without the fix:
> 
>   $ nm -Bn vmlinux | grep vmcoreinfo_data
>   ffff8000116cbad8 B vmcoreinfo_data
>   ...
>   $ gdb vmlinux /proc/kcore
>   ...
>   (gdb) p &vmcoreinfo_data
>   $1 = (unsigned char **) 0xffff8000116cbad8 <vmcoreinfo_data>
>   (gdb)
> 
> The static kernel data address above matches -- let's look at what
> unity-mapped
> address it got from get_zeroed_page():
> 
>   (gdb) printf "%lx\n", vmcoreinfo_data
>   ffff009f43670000
>   (gdb)
> 
> Looks good -- so let's read it:
> 
>   (gdb) printf "%s\n", vmcoreinfo_data
> 
>   (gdb)
> 
> At first glance, it appeared maybe the vmcoreinfo string data had not been
> written yet.  But with each string that gets appended, there is an
> accompanying
> "vmcoreinfo_size" that gets bumped:
> 
>   (gdb) printf "%ld\n", vmcoreinfo_size
>   1864
>   (gdb)
> 
> Without fix:
> 
> [root@ampere-hr330a-01 linux-tmp]# gdb vmlinux /proc/kcore
> GNU gdb (GDB) Red Hat Enterprise Linux 8.2-6.el8
> Copyright (C) 2018 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "aarch64-redhat-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
>     <http://www.gnu.org/software/gdb/documentation/>.
> 
> For help, type "help".
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from vmlinux...done.
> [New process 1]
> Core was generated by `BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.4.0-rc2.tmp2+
> root=/dev/mapper/rhel_ampere--hr3'.
> #0  0x0000000000000000 in ?? ()
> (gdb) printf "%s\n", vmcoreinfo_data
> 
> (gdb) printf "%ld\n", vmcoreinfo_size
> 1864
> (gdb) quit
> 
> With fix:
> 
> [root@ampere-hr330a-01 linux-tmp]# gdb vmlinux /proc/kcore
> GNU gdb (GDB) Red Hat Enterprise Linux 8.2-6.el8
> Copyright (C) 2018 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "aarch64-redhat-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
>     <http://www.gnu.org/software/gdb/documentation/>.
> 
> For help, type "help".
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from vmlinux...done.
> [New process 1]
> Core was generated by `BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.4.0-rc2.tmp2+
> root=/dev/mapper/rhel_ampere--hr3'.
> #0  0x0000000000000000 in ?? ()
> (gdb) printf "%s\n", vmcoreinfo_data
> OSRELEASE=5.4.0-rc2.tmp2+
> PAGESIZE=65536
> SYMBOL(init_uts_ns)=ffff8000113a56e8
> SYMBOL(node_online_map)=ffff80001139d570
> SYMBOL(swapper_pg_dir)=ffff800010ef0000
> SYMBOL(_stext)=ffff800010081000
> SYMBOL(vmap_area_list)=ffff80001141c360
> SYMBOL(mem_section)=ffff009f7eedc5c0
> LENGTH(mem_section)=1024
> SIZE(mem_section)=16
> OFFSET(mem_section.section_mem_map)=0
> SIZE(page)=64
> SIZE(pglist_data)=6720
> SIZE(zone)=1728
> SIZE(free_area)=88
> SIZE(list_head)=16
> SIZE(nodemask_t)=8
> OFFSET(page.flags)=0
> OFFSET(page._refcount)=52
> OFFSET(page.mapping)=24
> OFFSET(page.lru)=8
> OFFSET(page._mapcount)=48
> OFFSET(page.private)=40
> OFFSET(page.compound_dtor)=16
> OFFSET(page.compound_order)=17
> OFFSET(page.compound_head)=8
> OFFSET(pglist_data.node_zones)=0
> OFFSET(pglist_data.nr_zones)=5984
> OFFSET(pglist_data.node_start_pfn)=5992
> OFFSET(pglist_data.node_spanned_pages)=6008
> OFFSET(pglist_data.node_id)=6016
> OFFSET(zone.free_area)=192
> OFFSET(zone.vm_stat)=1536
> OFFSET(zone.spanned_pages)=104
> OFFSET(free_area.free_list)=0
> OFFSET(list_head.next)=0
> OFFSET(list_head.prev)=8
> OFFSET(vmap_area.va_start)=0
> OFFSET(vmap_area.list)=40
> LENGTH(zone.free_area)=14
> SYMBOL(log_buf)=ffff8000113ceeb8
> SYMBOL(log_buf_len)=ffff8000113ceeb0
> --Type <RET> for more, q to quit, c to continue without paging--
> SYMBOL(log_first_idx)=ffff8000115c55ac
> SYMBOL(clear_idx)=ffff8000115c55b8
> SYMBOL(log_next_idx)=ffff8000115c55a8
> SIZE(printk_log)=16
> OFFSET(printk_log.ts_nsec)=0
> OFFSET(printk_log.len)=8
> OFFSET(printk_log.text_len)=10
> OFFSET(printk_log.dict_len)=12
> LENGTH(free_area.free_list)=5
> NUMBER(NR_FREE_PAGES)=0
> NUMBER(PG_lru)=4
> NUMBER(PG_private)=13
> NUMBER(PG_swapcache)=10
> NUMBER(PG_swapbacked)=19
> NUMBER(PG_slab)=9
> NUMBER(PG_hwpoison)=22
> NUMBER(PG_head_mask)=65536
> NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE)=-129
> NUMBER(HUGETLB_PAGE_DTOR)=2
> NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE)=-257
> NUMBER(VA_BITS)=52
> NUMBER(kimage_voffset)=0xffff7fff80000000
> NUMBER(PHYS_OFFSET)=0x80000000
> KERNELOFFSET=0
> 
> (gdb) printf "%ld\n", vmcoreinfo_size
> 1864
> (gdb) quit
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel