| Message ID | 1567503958-25831-3-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/mm: Enable memory hot remove | expand |
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@redhat.com> > Acked-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > 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. Is there anything special about the arch? Acked-by: Balbir Singh <bsingharora@gmail.com>
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@redhat.com> >> Acked-by: Mark Rutland <mark.rutland@arm.com> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> 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@gmail.com> > >
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(); return 0; } DEFINE_SHOW_ATTRIBUTE(ptdump);