Re: [PATCH bpf-next v5 1/2] mm/bpf: Add bpf_proactive_reclaim kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] BPF kfunc `bpf_proactive_reclaim` can cause kernel deadlocks by triggering memory reclaim with `GFP_KERNEL` from sleepable contexts where filesystem or IO locks may be held.
- [Medium] Integer overflow in calculating the returned reclaimed bytes on 32-bit architectures.
--

commit ef066c78e33635cc7e9aa0fc78438725da7dc30b
Author: Hui Zhu <zhuhui@kylinos.cn>

mm/bpf: Add bpf_proactive_reclaim kfunc

This commit adds a sleepable kfunc bpf_proactive_reclaim() which performs
one proactive reclaim pass on a given memory cgroup. It refuses to reclaim
if the calling task is already in a reclaim context to avoid corrupting the
outer reclaim state.

> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
> index 716df49d76477..297ff7f050427 100644
> --- a/mm/bpf_memcontrol.c
> +++ b/mm/bpf_memcontrol.c

[ ... ]

> @@ -159,6 +160,49 @@ __bpf_kfunc void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg)
>  	mem_cgroup_flush_stats(memcg);
>  }

[ ... ]

> +__bpf_kfunc unsigned long bpf_proactive_reclaim(struct mem_cgroup *memcg,
> +						unsigned long size)
> +{
> +	unsigned long nr_reclaimed;
> +
> +	if (size < PAGE_SIZE || unlikely(bpf_in_reclaim_context()))
> +		return 0;
> +
> +	nr_reclaimed = try_to_free_mem_cgroup_pages(memcg, size / PAGE_SIZE,
> +						    GFP_KERNEL,

[Severity: High]
Can this cause a kernel deadlock when called from sleepable contexts where
filesystem or IO locks may be held?

Since GFP_KERNEL includes __GFP_FS and __GFP_IO, the direct reclaim path is
allowed to invoke filesystem shrinkers. If a shrinker attempts to acquire a
lock already held by the task (e.g., if a sleepable BPF program attached to
an LSM hook like inode_rmdir or security_file_open calls
bpf_proactive_reclaim), a self-deadlock could occur.

Would it be safer to wrap the reclaim call in memalloc_nofs_save() and
memalloc_nofs_restore() to guarantee safety, as the verifier cannot
statically enforce that no filesystem locks are held?

> +						    MEMCG_RECLAIM_MAY_SWAP |
> +						    MEMCG_RECLAIM_PROACTIVE,
> +						    NULL);
> +
> +	return nr_reclaimed * PAGE_SIZE;

[Severity: Medium]
Does this calculation overflow the 32-bit boundary before returning to the
caller?

On 32-bit architectures, unsigned long is 32 bits, making ULONG_MAX
approximately 4GB. If a user requests a reclaim pass with size close to
ULONG_MAX, the target pages will be close to 1,048,575.

try_to_free_mem_cgroup_pages() reclaims in batches and can easily overshoot
the target.

If nr_reclaimed exceeds 1,048,575 pages, multiplying it by PAGE_SIZE (4096)
will wrap around the 32-bit limit, yielding a severely truncated result.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787826402.git.zhuhui@kylinos.cn?part=1