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

["zhen.ni" <zhen.ni@easystack.cn>]

在 2026/10/9 20:00, 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.
> 

The size argument will be renamed to buf__sz.

> [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();
> 

I will take the trusted struct page_owner_scan * returned by
bpf_iter_page_owner_next() and use scan->po_snap.handle.



Thanks,
Zhen