linux-kernel - Re: [PATCH bpf-next 1/2] bpf: Add bpf_task_under

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 20 Apr 2023 11:22:42 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Feng zhou <zhoufeng.zf@...edance.com>
Cc:     Martin KaFai Lau <martin.lau@...ux.dev>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
        bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>, yangzhenze@...edance.com,
        Dongdong Wang <wangdongdong.6@...edance.com>
Subject: Re: [PATCH bpf-next 1/2] bpf: Add bpf_task_under_cgroup helper

On Thu, Apr 20, 2023 at 12:27 AM Feng zhou <zhoufeng.zf@...edance.com> wrote:
>
> From: Feng Zhou <zhoufeng.zf@...edance.com>
>
> This adds a bpf helper that's similar to the
> bpf_current_task_under_cgroup. The difference is that it is a
> designated task.
>
> When hook sched related functions, sometimes it is necessary to
> specify a task instead of the current task.
>
> Signed-off-by: Feng Zhou <zhoufeng.zf@...edance.com>
> ---
>  include/uapi/linux/bpf.h       | 13 +++++++++++++
>  kernel/bpf/verifier.c          |  4 +++-
>  kernel/trace/bpf_trace.c       | 31 +++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h | 13 +++++++++++++
>  4 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4b20a7269bee..3d31ddb39e10 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5550,6 +5550,18 @@ union bpf_attr {
>   *             0 on success.
>   *
>   *             **-ENOENT** if the bpf_local_storage cannot be found.
> + *
> + * long bpf_task_under_cgroup(struct bpf_map *map, struct task_struct *task, u32 index)
> + *     Description
> + *             Check whether the probe is being run is the context of a given
> + *             subset of the cgroup2 hierarchy. The cgroup2 to test is held by
> + *             *map* of type **BPF_MAP_TYPE_CGROUP_ARRAY**, at *index*.
> + *     Return
> + *             The return value depends on the result of the test, and can be:
> + *
> + *             * 1, if assigned task belongs to the cgroup2.
> + *             * 0, if assigned task does not belong to the cgroup2.
> + *             * A negative error code, if an error occurred.
>   */
>  #define ___BPF_FUNC_MAPPER(FN, ctx...)                 \
>         FN(unspec, 0, ##ctx)                            \
> @@ -5764,6 +5776,7 @@ union bpf_attr {
>         FN(user_ringbuf_drain, 209, ##ctx)              \
>         FN(cgrp_storage_get, 210, ##ctx)                \
>         FN(cgrp_storage_delete, 211, ##ctx)             \
> +       FN(task_under_cgroup, 212, ##ctx)               \
>         /* */
>
>  /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1e05355facdc..1e2c3c3e8d5f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7771,7 +7771,8 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
>                 break;
>         case BPF_MAP_TYPE_CGROUP_ARRAY:
>                 if (func_id != BPF_FUNC_skb_under_cgroup &&
> -                   func_id != BPF_FUNC_current_task_under_cgroup)
> +                   func_id != BPF_FUNC_current_task_under_cgroup &&
> +                   func_id != BPF_FUNC_task_under_cgroup)
>                         goto error;
>                 break;
>         case BPF_MAP_TYPE_CGROUP_STORAGE:
> @@ -7902,6 +7903,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
>                         goto error;
>                 break;
>         case BPF_FUNC_current_task_under_cgroup:
> +       case BPF_FUNC_task_under_cgroup:
>         case BPF_FUNC_skb_under_cgroup:
>                 if (map->map_type != BPF_MAP_TYPE_CGROUP_ARRAY)
>                         goto error;
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index bcf91bc7bf71..b02a04768824 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -814,6 +814,35 @@ static const struct bpf_func_proto bpf_current_task_under_cgroup_proto = {
>         .arg2_type      = ARG_ANYTHING,
>  };
>
> +BPF_CALL_3(bpf_task_under_cgroup, struct bpf_map *, map, struct task_struct *,
> +          task, u32, idx)
> +{
> +       struct bpf_array *array = container_of(map, struct bpf_array, map);
> +       struct cgroup *cgrp;
> +
> +       if (unlikely(!task))
> +               return -ENOENT;
> +
> +       if (unlikely(idx >= array->map.max_entries))
> +               return -E2BIG;
> +
> +       cgrp = READ_ONCE(array->ptrs[idx]);
> +       if (unlikely(!cgrp))
> +               return -EAGAIN;
> +
> +       return task_under_cgroup_hierarchy(task, cgrp);

We don't add helpers anymore.
Please wrap task_under_cgroup_hierarchy() as a kfunc
that takes two TRUSTED pointers task and cgroup.