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: 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@vger.kernel.org Signed-off-by: Gregory Price (Meta) --- 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 #include +#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