[PATCH 2/6] mm/page_owner: hold off memory offline while a page is read

[Gregory Price (Meta)]

Reading /sys/kernel/debug/page_owner while memory is removed can oops.
read_page_owner() checks a pfn with pfn_valid(), and print_page_owner()
can sleep in kmalloc() before it reads the struct page:

  CPU 1 (read page_owner)             CPU 2 (remove the memory block)
  pfn_valid(pfn)
  page = pfn_to_page(pfn)
  page_ext_get()
  page_ext_put()
  print_page_owner()
    kmalloc()
                                      offline_pages()
                                        remove_pfn_range_from_zone()
                                      arch_remove_memory()
                                        frees the vmemmap
    get_pageblock_migratetype(page)

  BUG: unable to handle page fault for address: ffd4000006000000
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  Oops: Oops: 0000 [#1] SMP KASAN
  RIP: 0010:__get_pfnblock_flags_mask+0x3e/0x2b0
  Call Trace:
   <TASK>
   get_pfnblock_migratetype+0xf/0x20
   read_page_owner+0x629/0xf00
   full_proxy_read+0x102/0x190
   vfs_read+0x153/0xcd0
   ksys_read+0xe4/0x1b0
   do_syscall_64+0xbd/0x630
   entry_SYSCALL_64_after_hwframe+0x4b/0x53

Offline alone faults the same way on DEBUG_VM kernels.  The page_ext is
safe; offline_page_ext() frees it after an RCU grace period.

Check the pfn with pfn_to_online_page() inside a pfn_walk read section,
which offline waits for, and move kmalloc() and copy_to_user() out of
it, since offline waits with the hotplug locks held.  A full read of
page_owner in a 5G guest takes 1.36 s before and 1.34 s after.

With a kprobe sleeping the reader on a DIMM pfn across an offline or a
removal, base faults in 3 of 3 runs of each and this patch in none.

Fixes: 48c96a368579 ("mm/page_owner: keep track of page owners")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
---
 mm/page_owner.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/mm/page_owner.c b/mm/page_owner.c
index cfc31c92d7657..d2c0b831fcd3b 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -13,6 +13,7 @@
 #include <linux/memcontrol.h>
 #include <linux/sched/clock.h>
 
+#include "internal.h"
 #include "page_alloc.h"
 
 /*
@@ -600,20 +601,14 @@ static inline int print_page_owner_memcg(char *kbuf, size_t count, int ret,
 #endif
 
 static ssize_t
-print_page_owner(char __user *buf, size_t count, unsigned long pfn,
+print_page_owner(char *kbuf, size_t count, unsigned long pfn,
 		struct page *page, struct page_owner *page_owner,
 		depot_stack_handle_t handle,
 		struct page_owner_filter_state *state)
 {
 	int ret, pageblock_mt, page_mt;
-	char *kbuf;
 	enum page_owner_print_mode print_mode;
 
-	count = min_t(size_t, count, PAGE_SIZE);
-	kbuf = kmalloc(count, GFP_KERNEL);
-	if (!kbuf)
-		return -ENOMEM;
-
 	print_mode = state->print_mode;
 
 	ret = scnprintf(kbuf, count,
@@ -637,14 +632,14 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 	if (print_mode != PAGE_OWNER_PRINT_HANDLE) {
 		ret += stack_depot_snprint(handle, kbuf + ret, count - ret, 0);
 		if (ret >= count)
-			goto err;
+			return -ENOMEM;
 	}
 
 	if (print_mode != PAGE_OWNER_PRINT_STACK) {
 		ret += scnprintf(kbuf + ret, count - ret, "handle: %u\n",
 				 handle);
 		if (ret >= count)
-			goto err;
+			return -ENOMEM;
 	}
 
 	if (page_owner->last_migrate_reason != MR_NEVER) {
@@ -657,17 +652,9 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 
 	ret += snprintf(kbuf + ret, count - ret, "\n");
 	if (ret >= count)
-		goto err;
-
-	if (copy_to_user(buf, kbuf, ret))
-		ret = -EFAULT;
+		return -ENOMEM;
 
-	kfree(kbuf);
 	return ret;
-
-err:
-	kfree(kbuf);
-	return -ENOMEM;
 }
 
 void __dump_page_owner(const struct page *page)
@@ -734,10 +721,17 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	struct page_owner *page_owner;
 	depot_stack_handle_t handle;
 	struct page_owner_filter_state *state = file->private_data;
+	ssize_t ret = 0;
+	char *kbuf;
 
 	if (!static_branch_unlikely(&page_owner_inited))
 		return -EINVAL;
 
+	count = min_t(size_t, count, PAGE_SIZE);
+	kbuf = kmalloc(count, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
 	page = NULL;
 	if (*ppos == 0)
 		pfn = min_low_pfn;
@@ -766,7 +760,12 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 			continue;
 		}
 
-		page = pfn_to_page(pfn);
+		/* Offline waits for us before it poisons the memmap */
+		guard(pfn_walk)();
+		page = pfn_to_online_page(pfn);
+		if (!page)
+			continue;
+
 		if (skip_buddy_pages(&pfn, page))
 			continue;
 
@@ -825,14 +824,18 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 
 		page_owner_tmp = *page_owner;
 		page_ext_put(page_ext);
-		return print_page_owner(buf, count, pfn, page,
+		ret = print_page_owner(kbuf, count, pfn, page,
 				&page_owner_tmp, handle, state);
+		break;
 ext_put_continue:
 		page_ext_put(page_ext);
 		cond_resched();
 	}
 
-	return 0;
+	if (ret > 0 && copy_to_user(buf, kbuf, ret))
+		ret = -EFAULT;
+	kfree(kbuf);
+	return ret;
 }
 
 static loff_t lseek_page_owner(struct file *file, loff_t offset, int orig)
-- 
2.53.0-Meta