Re: [PATCH 2/6] mm/page_owner: add bpf_iter target "page_owner"

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] mm: vmemmap use-after-free in page_owner_next_eligible() during memory hot-remove
- [Medium] mm: bpf_iter page_owner skips pages when seq_file buffer fills
- [Medium] bpf: missing EOF notification for page_owner bpf_iter
- [Medium] bpf: page_owner_iter skips records during chunked seq_read
- [Medium] bpf: page_owner_iter lacks EOF notification and yields stale po_snap
--

commit fd543af9dd8ff743b641757854c444d98608421d
Author: Zhen Ni <zhen.ni@easystack.cn>
Subject: mm/page_owner: add bpf_iter target "page_owner"

This commit registers a bpf_iter target page_owner that walks the pages
eligible for page_owner output. The attached BPF program receives the pfn,
the page, and the page_owner record to filter and format the output.

> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index bafe474f33602..eb420803a434a 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
[ ... ]
> @@ -741,7 +717,7 @@ void __dump_page_owner(const struct page *page)
>   * must advance scan->pfn past the hit before calling again to resume
>   * the scan.
>   */
> -static bool page_owner_next_eligible(struct page_owner_scan *scan)
> +bool page_owner_next_eligible(struct page_owner_scan *scan)

[Severity: High]
Does exposing this function to the BPF iterator API introduce a use-after-free
regression during memory hot-remove?

Inside page_owner_next_eligible(), pfn_valid() is used to verify memory
sections. However, since the iteration runs without holding memory hotplug
locks or a continuous rcu_read_lock(), the memory section can be
concurrently hot-removed by another CPU as soon as pfn_valid() returns.

If the vmemmap pages backing the page struct are unmapped and freed during
this race, the subsequent call will dereference the unmapped page struct:

	page = pfn_to_page(pfn);
	if (skip_buddy_pages(&pfn, page))
		continue;

Can this cause a kernel page fault and panic?

While this logic already existed in the /sys/kernel/debug/page_owner read
path, this patch relies on it and exposes the buggy code path to the new
BPF iterator API.

> diff --git a/mm/page_owner_iter.c b/mm/page_owner_iter.c
> new file mode 100644
> index 0000000000000..33dc9dcd59dec
> --- /dev/null
> +++ b/mm/page_owner_iter.c
[ ... ]
> +static void *page_owner_seq_start(struct seq_file *seq, loff_t *posp)
> +{
> +	struct page_owner_scan *scan = seq->private;
> +
> +	if (*posp == 0)
> +		scan->pfn = min_low_pfn;
> +	else
> +		scan->pfn++;

[Severity: Medium]
Can this unconditionally skip elements during chunked reads?

During a seq_read iteration, if a formatted record overflows the output
buffer, the iterator discards the overflowed record, returns to userspace,
and later resumes by calling start() with *posp > 0.

Since next() already advanced scan->pfn to point to the overflowed record,
start() increments it again here. Does this mean userspace tools reading
the iterator output in chunks (like cat or bpftool iter) will silently
drop one valid page owner record at every buffer chunk boundary?

> +
> +	if (!page_owner_next_eligible(scan))
> +		return NULL;
> +	return scan->page;
> +}
> +
[ ... ]
> +static void page_owner_seq_stop(struct seq_file *seq, void *v)
> +{
> +}

[Severity: Medium]
Are we missing an EOF notification here?

Standard bpf_iter implementations invoke the BPF program with v == NULL
during seq_ops->stop() to notify the program of EOF, enabling summary
aggregate output. Because this function is empty, BPF programs attached to
the page_owner iterator cannot detect the end of iteration.

Does this mean any logic relying on the standard EOF signal to output
summary statistics or perform cleanup will never execute?

> +
> +static int page_owner_prog_seq_show(struct bpf_prog *prog,
> +				    struct bpf_iter_meta *meta, void *v)
> +{
> +	struct page_owner_scan *scan = meta->seq->private;
> +	struct bpf_iter__page_owner ctx;
> +
> +	/* @po: hit-time snapshot, no RCU window needed. */
> +	ctx.meta = meta;
> +	ctx.page = (struct page *)v;
> +	ctx.po = &scan->po_snap;

[Severity: Medium]
If EOF were properly signaled with v == NULL, would this unconditional
assignment provide stale data to the BPF program?

By unconditionally assigning ctx.po to the snapshot pointer, ctx.po is
never NULL, even when v (the page) is NULL. Does this mean BPF programs
cannot reliably use if (!ctx->po) to detect EOF, and might operate on stale
snapshot data from the last valid page?

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