Re: [PATCH 3/6] mm/page_owner: add open-coded page_owner iterator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bpf: buffer overflow in bpf_page_owner_stack_snprint() due to missing __sz suffix
- [High] bpf: Panic DoS and OOB read via untrusted stack depot handle in bpf_page_owner_stack_snprint()
--

commit f5344767004d7fb48b115dd782835818e60504bc
Author: Zhen Ni <zhen.ni@easystack.cn>

mm/page_owner: add open-coded page_owner iterator

This commit adds an open-coded bpf_iter for pages eligible for page_owner
output. It also introduces a bpf_page_owner_stack_snprint() kfunc to render
the stack trace for a stack_depot handle into a caller-provided buffer.

> diff --git a/mm/page_owner_iter.c b/mm/page_owner_iter.c
> index 33dc9dcd59dec..80693d4174077 100644
> --- a/mm/page_owner_iter.c
> +++ b/mm/page_owner_iter.c
[ ... ]
> @@ -114,3 +115,54 @@ int page_owner_iter_register(void)
[ ... ]
> +__bpf_kfunc int bpf_page_owner_stack_snprint(depot_stack_handle_t handle,
> +					     char *buf, u32 buf_size)
> +{

[Severity: Critical]
Does this kfunc bypass the verifier's bounds checking for the buffer?

Because the buffer size argument is named buf_size instead of using the
required __sz or __szk suffix (such as buf__sz), the BPF verifier treats it
as an independent integer scalar. It only validates that the buffer points to
a memory region of at least 1 byte.

If a BPF program passes a 1-byte valid buffer along with a maliciously large
buf_size, stack_depot_snprint() will blindly write past the end of the buffer,
which can corrupt adjacent stack, heap, or map memory.

[Severity: High]
Is it safe to accept an untrusted depot_stack_handle_t directly from the
BPF program here?

Since the BPF verifier does not track scalar bounds for opaque handles, a
program could pass a forged handle. If depot_fetch_stack() receives an
invalid pool index, it unconditionally triggers a kernel warning:

lib/stackdepot.c:depot_fetch_stack() {
	if (pool_index >= pools_num_cached) {
		WARN(1, "pool index %d out of bounds (%d) for stack id %08x\n",
		     pool_index, pools_num_cached, handle);
		return NULL;
	}
	...
}

This can panic systems configured with panic_on_warn=1.

Even if the index is valid, a maximized offset could cause the stack pointer
to point near the end of the pool allocation. Accessing the stack size and
entries would then result in an out-of-bounds read past the allocated pool.
This can cause a hard page fault panic, or format arbitrary kernel pointers
into the BPF-provided buffer, which the program could then exfiltrate.

Would it be better to change this kfunc to accept a trusted
struct page_owner_scan *scan instead of a raw scalar?

> +	return stack_depot_snprint(handle, buf, buf_size, 0);
> +}
> +
> +__bpf_kfunc_end_defs();

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