Re: [PATCH 5/6] mm/page_owner: add bpf_page_owner_get_memcg_info() kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bpf: Missing __sz annotation allows BPF stack overflow in memcg kfunc
- [High] bpf: missing __sz annotation on size param causes stack buffer overflow
--

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

mm/page_owner: add bpf_page_owner_get_memcg_info() kfunc

This commit introduces a new BPF kfunc to expose per-page memcg information,
including the cgroup path. It extracts memcg data collection into a new
helper function that the kfunc calls directly.

> diff --git a/mm/page_owner_iter.c b/mm/page_owner_iter.c
> index 907517a291443..e7a8ca1e90e8d 100644
> --- a/mm/page_owner_iter.c
> +++ b/mm/page_owner_iter.c
[ ... ]
> @@ -170,4 +170,11 @@ __bpf_kfunc int bpf_page_owner_get_nid(struct page_owner_scan *scan)
>  	return page_owner_get_nid(scan->page);
>  }
>  
> +__bpf_kfunc int bpf_page_owner_get_memcg_info(struct page_owner_scan *scan,
> +					      struct memcg_info *info,
> +					      char *buf, u32 size)

[Severity: Critical]
Does this size parameter need the __sz suffix for the verifier to track
memory bounds correctly?

Because the parameter is named size rather than buf__sz, it appears the
verifier might treat buf as a fixed 1-byte allocation and size as an
unconstrained scalar.

Could this allow a BPF program to pass a 1-byte stack buffer alongside an
arbitrarily large size value? If so, when this calls into
get_page_memcg_info() in mm/page_owner.c and executes:

get_page_memcg_info()
    ...
    if (buf && size)
        cgroup_path(memcg->css.cgroup, buf, size);

can it overwrite the BPF stack and corrupt adjacent kernel memory?

[Severity: High]
Do the buf and info parameters also need the __uninit suffix?

Without this suffix, the verifier might improperly force BPF programs to
pre-initialize these output variables just to pass verification.

> +{
> +	return get_page_memcg_info(scan->page, info, buf, size);
> +}
> +
>  __bpf_kfunc_end_defs();

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