Re: [PATCH 6/6] selftests/bpf: add tests for the page_owner bpf_iter target

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: missing __sz suffix in kfunc buffer size parameters bypasses verifier bounds check
--

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

selftests/bpf: add tests for the page_owner bpf_iter target

This commit adds a sleepable BPF program and a userspace runner to test the
page_owner BPF iterator. It exercises the seq_file and open-coded iterator
paths and verifies the contracts of the get_nid, stack_snprint, and
get_memcg_info kfuncs.

> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_page_owner.c b/tools/testing/selftests/bpf/progs/bpf_iter_page_owner.c
> new file mode 100644
> index 0000000000000..fada6a314a157
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_page_owner.c

[ ... ]

> +extern int bpf_iter_page_owner_new(struct bpf_iter_page_owner *it) __ksym;
> +extern struct page_owner_scan *
> +bpf_iter_page_owner_next(struct bpf_iter_page_owner *it) __ksym;
> +extern void bpf_iter_page_owner_destroy(struct bpf_iter_page_owner *it) __ksym;
> +extern int bpf_page_owner_stack_snprint(depot_stack_handle_t handle,
> +					char *buf, u32 buf_size) __ksym;
> +extern int bpf_page_owner_get_nid(struct page_owner_scan *scan) __ksym;
> +extern int bpf_page_owner_get_memcg_info(struct page_owner_scan *scan,
> +					 struct memcg_info *info,
> +					 char *buf, u32 size) __ksym;

[Severity: High]
Does the kfunc ABI exercised here bypass verifier memory bounds checking?

The BPF verifier requires size parameters for memory buffers in kfuncs to be
explicitly annotated with a __sz or __szk suffix (e.g., buf__sz). Without this
suffix, the verifier treats the size parameter as an independent scalar and
enforces no relationship between it and the buffer pointer.

In mm/page_owner_iter.c (which this newly added test relies upon), the kfuncs
bpf_page_owner_stack_snprint and bpf_page_owner_get_memcg_info define
buffer size arguments as u32 buf_size and u32 size, respectively, omitting
the required suffix:

mm/page_owner_iter.c:bpf_page_owner_stack_snprint()
    __bpf_kfunc int bpf_page_owner_stack_snprint(depot_stack_handle_t handle,
                                                 char *buf, u32 buf_size)

mm/page_owner_iter.c:bpf_page_owner_get_memcg_info()
    __bpf_kfunc int bpf_page_owner_get_memcg_info(struct page_owner_scan *scan,
                                                  struct memcg_info *info,
                                                  char *buf, u32 size)

Consequently, the verifier only validates that buf points to a 1-byte valid
memory region (based on the char type) while allowing arbitrarily large scalar
values to be passed as the size.

A BPF program could exploit this by providing a small buffer alongside a large
size argument, causing the underlying kernel functions (stack_depot_snprint
and get_page_memcg_info) to write past the bounds of the provided buffer,
potentially leading to kernel memory corruption.

Could these kfunc size arguments in the kernel be renamed to include the __sz
suffix to ensure proper verifier bounds checking?

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