Re: [RFC PATCH v3 1/3] bpf: cgroup: Introduce helper cgroup_bpf_current_enabled()

[Yonghong Song]

On 12/13/23 6:38 AM, Michael Weiß wrote:
> This helper can be used to check if a cgroup-bpf specific program is
> active for the current task.
>
> Signed-off-by: Michael Weiß <michael.weiss@xxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@xxxxxxxxxxxxx>
> ---
>   include/linux/bpf-cgroup.h |  2 ++
>   kernel/bpf/cgroup.c        | 14 ++++++++++++++
>   2 files changed, 16 insertions(+)
>
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index a789266feac3..7cb49bde09ff 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -191,6 +191,8 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
>   	return array != &bpf_empty_prog_array.hdr;
>   }
>   
> +bool cgroup_bpf_current_enabled(enum cgroup_bpf_attach_type type);
> +
>   /* Wrappers for __cgroup_bpf_run_filter_skb() guarded by cgroup_bpf_enabled. */
>   #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk, skb)			      \
>   ({									      \
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 491d20038cbe..9007165abe8c 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -24,6 +24,20 @@
>   DEFINE_STATIC_KEY_ARRAY_FALSE(cgroup_bpf_enabled_key, MAX_CGROUP_BPF_ATTACH_TYPE);
>   EXPORT_SYMBOL(cgroup_bpf_enabled_key);
>   
> +bool cgroup_bpf_current_enabled(enum cgroup_bpf_attach_type type)
> +{
> +	struct cgroup *cgrp;
> +	struct bpf_prog_array *array;
> +
> +	rcu_read_lock();
> +	cgrp = task_dfl_cgroup(current);
> +	rcu_read_unlock();
> +
> +	array = rcu_access_pointer(cgrp->bpf.effective[type]);

This seems wrong here. The cgrp could become invalid once leaving
rcu critical section.

> +	return array != &bpf_empty_prog_array.hdr;

I guess you need include 'array' usage as well in the rcu cs.
So overall should look like:

	rcu_read_lock();
	cgrp = task_dfl_cgroup(current);
	array = rcu_access_pointer(cgrp->bpf.effective[type]);
	bpf_prog_exists = array != &bpf_empty_prog_array.hdr;
	rcu_read_unlock();

	return bpf_prog_exists;

> +}
> +EXPORT_SYMBOL(cgroup_bpf_current_enabled);
> +
>   /* __always_inline is necessary to prevent indirect call through run_prog
>    * function pointer.
>    */