Re: [PATCH v2 4/6] mm/page_owner: record the NUMA node in the scan cursor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: incomplete fix leaves page_owner vulnerable to unmapped page dereference
--

commit 7b174b26adeea5712aaa10bd465514b5ce7dbe36
Author: Zhen Ni <zhen.ni@easystack.cn>
Subject: mm/page_owner: record the NUMA node in the scan cursor

This commit computes the page's NUMA node in page_owner_next_eligible()
and stores it in the scan cursor to avoid dereferencing a potentially
unmapped page later. It updates the debugfs nid filter and BPF program
to use this stored node value.

> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 617346a4ab22a..113e4e7e0b3a1 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
[ ... ]
> @@ -810,18 +824,8 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  	/* Find an allocated page */
>  	while (page_owner_next_eligible(&scan)) {
>  		if (state->nid_filter_enabled) {
> -			int nid;
> -			memdesc_flags_t page_flags =
> -				READ_ONCE(scan.page->flags);
> -
> -			/*
> -			 * Bypass PF_POISONED_CHECK() in page_to_nid() to avoid
> -			 * VM_BUG_ON when accessing poisoned pages.
> -			 */
> -			if (page_flags.f == PAGE_POISON_PATTERN)
> -				goto skip_continue;
> -			nid = memdesc_nid(&page_flags);
> -			if (!node_isset(nid, state->nid_filter))
> +			if (scan.nid < 0 ||
> +			    !node_isset(scan.nid, state->nid_filter))
>  				goto skip_continue;
>  		}
>  

[Severity: High]
Does this fix leave scan.page vulnerable to an unmapped page dereference
shortly after this block?

Although this patch successfully avoids dereferencing scan.page when checking
the node filter, page_owner_next_eligible() drops the RCU lock via
page_ext_put() before returning.

Just below the quoted block in read_page_owner(), scan.page is still
passed directly to print_page_owner():

read_page_owner() {
    ...
    while (page_owner_next_eligible(&scan)) {
        ...
        return print_page_owner(buf, count, scan.pfn, scan.page,
                                &scan.po_snap, scan.po_snap.handle, state);
    ...
}

Inside print_page_owner(), the pointer is dereferenced by scnprintf using
the %pGp formatter on &page->flags.f.

If a concurrent memory hot-remove operation frees the vmemmap backing this
struct page after the RCU lock is dropped, will this %pGp format access
trigger a kernel panic?

Additionally, a follow-up patch in this series (Patch 5) appears to introduce
get_page_memcg_info() which also unconditionally dereferences scan.page
outside the RCU lock from read_page_owner(). Does that face the same
use-after-free risk?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013122125.843451-1-zhen.ni@easystack.cn?part=4