Re: [PATCH v2 08/23] mm: introduce BPF kfuncs to deal with memcg pointers

[Roman Gushchin <roman.gushchin@linux.dev>]

bot+bpf-ci@kernel.org writes:

> ```
> commit ec8e2e1769704b4f69261f36e6aaf035d47c5f0f
> Author: Roman Gushchin <roman.gushchin@linux.dev>
>
> mm: introduce BPF kfuncs to deal with memcg pointers
>
> This commit introduces two new BPF kfuncs for converting css pointers
> to memcg pointers with proper reference counting: bpf_get_mem_cgroup()
> and bpf_put_mem_cgroup().
>
>> diff --git a/mm/Makefile b/mm/Makefile
>> index 051e88c69..2d8f9beb3 100644
>> --- a/mm/Makefile
>> +++ b/mm/Makefile
>> @@ -107,6 +107,7 @@ obj-$(CONFIG_MEMCG) += swap_cgroup.o
>>  endif
>>  ifdef CONFIG_BPF_SYSCALL
>>  obj-y += bpf_oom.o
>> +obj-$(CONFIG_MEMCG) += bpf_memcontrol.o
>>  endif
>>  obj-$(CONFIG_CGROUP_HUGETLB) += hugetlb_cgroup.o
>>  obj-$(CONFIG_GUP_TEST) += gup_test.o
>> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
>> new file mode 100644
>> index 000000000..1e4609774
>> --- /dev/null
>> +++ b/mm/bpf_memcontrol.c
>> @@ -0,0 +1,88 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Memory Controller-related BPF kfuncs and auxiliary code
>> + *
>> + * Author: Roman Gushchin <roman.gushchin@linux.dev>
>> + */
>> +
>> +#include <linux/memcontrol.h>
>> +#include <linux/bpf.h>
>> +
>> +__bpf_kfunc_start_defs();
>> +
>> +/**
>> + * bpf_get_mem_cgroup - Get a reference to a memory cgroup
>> + * @css: pointer to the css structure
>> + *
>> + * Returns a pointer to a mem_cgroup structure after bumping
>> + * the corresponding css's reference counter.
>> + *
>> + * It's fine to pass a css which belongs to any cgroup controller,
>> + * e.g. unified hierarchy's main css.
>> + *
>> + * Implements KF_ACQUIRE semantics.
>> + */
>> +__bpf_kfunc struct mem_cgroup *
>> +bpf_get_mem_cgroup(struct cgroup_subsys_state *css)
>> +{
>> +	struct mem_cgroup *memcg = NULL;
>> +	bool rcu_unlock = false;
>> +
>> +	if (!root_mem_cgroup)
>> +		return NULL;
>> +
>> +	if (root_mem_cgroup->css.ss != css->ss) {
>                                             ^^^^^^^
>
> Can this dereference a NULL css pointer?  The function checks css for
> NULL at line 42 with "if (css && css_tryget(css))", which suggests css
> can be NULL.  If a BPF program calls this function with a NULL css,
> wouldn't it crash here before reaching that check?

Here css passed as an argument to bpf_get_mem_cgroup() can't be NULL,
because the BPF verifier guarantees that it's a valid css pointer.

However the result of rcu_dereference_raw(cgroup->subsys[ssid]) can be
NULL, this is why the "if (css && css_tryget(css))" check is required
down below.