[V4,3/4] arm64/mm: Hold memory hotplug lock while walking for kernel page table dump
diff mbox series

Message ID 1558329516-10445-4-git-send-email-anshuman.khandual@arm.com
State New
Headers show
Series
  • arm64/mm: Enable memory hot remove
Related show

Commit Message

Anshuman Khandual May 20, 2019, 5:18 a.m. UTC
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>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/ptdump_debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Michal Hocko May 21, 2019, 10:14 a.m. UTC | #1
On Mon 20-05-19 10:48:35, 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.

I've had a comment on this patch in the previous version which didn't
get answered completely AFAICS. If you really insist then please make
sure to describe why does this really matter because this will make
any further changes to the hotplug locking harder and I would to see
that it is worth the additional trouble.

Thanks!
Anshuman Khandual May 24, 2019, 4:52 a.m. UTC | #2
On 05/21/2019 03:44 PM, Michal Hocko wrote:
> On Mon 20-05-19 10:48:35, 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.
> 
> I've had a comment on this patch in the previous version which didn't
> get answered completely AFAICS. If you really insist then please make
> sure to describe why does this really matter because this will make
> any further changes to the hotplug locking harder and I would to see
> that it is worth the additional trouble.

Hello Michal,

I was under the impression (seems wrongful now) that the previous discussion
was complete. Nonetheless we can still discuss it further. Mark has responded
on the previous V3 thread [1] and because this particular patch does not have
any changes from last time, we can continue discussing this in that thread.

[1] https://lkml.org/lkml/2019/5/22/613   

- Anshuman

Patch
diff mbox series

diff --git a/arch/arm64/mm/ptdump_debugfs.c b/arch/arm64/mm/ptdump_debugfs.c
index 064163f..80171d1 100644
--- a/arch/arm64/mm/ptdump_debugfs.c
+++ b/arch/arm64/mm/ptdump_debugfs.c
@@ -7,7 +7,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);