On 09/15/2019 08:05 AM, Balbir Singh wrote: > > > On 3/9/19 7:45 pm, Anshuman Khandual wrote: >> The arm64 page table dump code can race with concurrent modification of the >> kernel page tables. When a leaf entries are modified concurrently, the dump >> code may log stale or inconsistent information for a VA range, but this is >> otherwise not harmful. >> >> When intermediate levels of table are freed, the dump code will continue to >> use memory which has been freed and potentially reallocated for another >> purpose. In such cases, the dump code may dereference bogus addresses, >> leading to a number of potential problems. >> >> Intermediate levels of table may by freed during memory hot-remove, >> which will be enabled by a subsequent patch. To avoid racing with >> this, take the memory hotplug lock when walking the kernel page table. >> >> Acked-by: David Hildenbrand <david@xxxxxxxxxx> >> Acked-by: Mark Rutland <mark.rutland@xxxxxxx> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> >> --- >> arch/arm64/mm/ptdump_debugfs.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm64/mm/ptdump_debugfs.c b/arch/arm64/mm/ptdump_debugfs.c >> index 064163f25592..b5eebc8c4924 100644 >> --- a/arch/arm64/mm/ptdump_debugfs.c >> +++ b/arch/arm64/mm/ptdump_debugfs.c >> @@ -1,5 +1,6 @@ >> // SPDX-License-Identifier: GPL-2.0 >> #include <linux/debugfs.h> >> +#include <linux/memory_hotplug.h> >> #include <linux/seq_file.h> >> >> #include <asm/ptdump.h> >> @@ -7,7 +8,10 @@ >> static int ptdump_show(struct seq_file *m, void *v) >> { >> struct ptdump_info *info = m->private; >> + >> + get_online_mems(); >> ptdump_walk_pgd(m, info); >> + put_online_mems(); > > Looks sane, BTW, checking other arches they might have the same race. The problem can be present on other architectures which can dump kernel page table during memory hot-remove operation where it actually frees up page table pages. If there is no freeing involved the race condition here could cause inconsistent or garbage information capture for a given VA range. Same is true even for concurrent vmalloc() operations as well. But removal of page tables pages can make it worse. Freeing page table pages during hot-remove is a platform decision, so would be adding these locks while walking kernel page table during ptdump. > Is there anything special about the arch? AFAICS, no. > > Acked-by: Balbir Singh <bsingharora@xxxxxxxxx> > >