linux-kernel - Re: [PATCH bpf-next v5 2/2] bpf: allow bpf_current_task_under_cgroup() with BPF_CGROUP

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]
Message-ID: <CAEf4BzZiO1XdiRcrNu6n=_cdxT+0zRnByCuL7zNi0Aw7ohUCnA@mail.gmail.com>
Date: Thu, 15 Aug 2024 15:23:50 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Matteo Croce <technoboy85@...il.com>
Cc: Andrii Nakryiko <andrii@...nel.org>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Steven Rostedt <rostedt@...dmis.org>, 
	Masami Hiramatsu <mhiramat@...nel.org>, bpf@...r.kernel.org, 
	linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Matteo Croce <teknoraver@...a.com>
Subject: Re: [PATCH bpf-next v5 2/2] bpf: allow bpf_current_task_under_cgroup()
 with BPF_CGROUP_*

On Tue, Aug 13, 2024 at 6:28 AM Matteo Croce <technoboy85@...il.com> wrote:
>
> From: Matteo Croce <teknoraver@...a.com>
>
> The helper bpf_current_task_under_cgroup() currently is only allowed for
> tracing programs, allow its usage also in the BPF_CGROUP_* program types.
>
> Move the code from kernel/trace/bpf_trace.c to kernel/bpf/helpers.c,
> so it compiles also without CONFIG_BPF_EVENTS.
>
> This will be used in systemd-networkd to monitor the sysctl writes,
> and filter it's own writes from others:
> https://github.com/systemd/systemd/pull/32212
>
> Signed-off-by: Matteo Croce <teknoraver@...a.com>
> ---
>  include/linux/bpf.h      |  1 +
>  kernel/bpf/cgroup.c      |  2 ++
>  kernel/bpf/helpers.c     | 23 +++++++++++++++++++++++
>  kernel/trace/bpf_trace.c | 23 -----------------------
>  4 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b9425e410bcb..f0192c173ed8 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3206,6 +3206,7 @@ extern const struct bpf_func_proto bpf_sock_hash_update_proto;
>  extern const struct bpf_func_proto bpf_get_current_cgroup_id_proto;
>  extern const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto;
>  extern const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto;
> +extern const struct bpf_func_proto bpf_current_task_under_cgroup_proto;
>  extern const struct bpf_func_proto bpf_msg_redirect_hash_proto;
>  extern const struct bpf_func_proto bpf_msg_redirect_map_proto;
>  extern const struct bpf_func_proto bpf_sk_redirect_hash_proto;
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 8ba73042a239..e7113d700b87 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -2581,6 +2581,8 @@ cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>         case BPF_FUNC_get_cgroup_classid:
>                 return &bpf_get_cgroup_classid_curr_proto;
>  #endif
> +       case BPF_FUNC_current_task_under_cgroup:
> +               return &bpf_current_task_under_cgroup_proto;
>         default:
>                 return NULL;
>         }
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 0d1d97d968b0..8502cfed2926 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2458,6 +2458,29 @@ __bpf_kfunc long bpf_task_under_cgroup(struct task_struct *task,
>         return ret;
>  }
>
> +BPF_CALL_2(bpf_current_task_under_cgroup, struct bpf_map *, map, u32, idx)
> +{
> +       struct bpf_array *array = container_of(map, struct bpf_array, map);
> +       struct cgroup *cgrp;
> +
> +       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(current, cgrp);
> +}
> +
> +const struct bpf_func_proto bpf_current_task_under_cgroup_proto = {
> +       .func           = bpf_current_task_under_cgroup,
> +       .gpl_only       = false,
> +       .ret_type       = RET_INTEGER,
> +       .arg1_type      = ARG_CONST_MAP_PTR,
> +       .arg2_type      = ARG_ANYTHING,
> +};
> +
>  /**
>   * bpf_task_get_cgroup1 - Acquires the associated cgroup of a task within a
>   * specific cgroup1 hierarchy. The cgroup1 hierarchy is identified by its
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d557bb11e0ff..e4e1f5d8b2a6 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -797,29 +797,6 @@ const struct bpf_func_proto bpf_task_pt_regs_proto = {
>         .ret_btf_id     = &bpf_task_pt_regs_ids[0],
>  };
>
> -BPF_CALL_2(bpf_current_task_under_cgroup, struct bpf_map *, map, u32, idx)
> -{
> -       struct bpf_array *array = container_of(map, struct bpf_array, map);
> -       struct cgroup *cgrp;
> -
> -       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(current, cgrp);
> -}
> -
> -static const struct bpf_func_proto bpf_current_task_under_cgroup_proto = {
> -       .func           = bpf_current_task_under_cgroup,
> -       .gpl_only       = false,
> -       .ret_type       = RET_INTEGER,
> -       .arg1_type      = ARG_CONST_MAP_PTR,
> -       .arg2_type      = ARG_ANYTHING,
> -};
> -

Seems like you need to adjust bpf_tracing_func_proto() function and
move BPF_FUNC_current_task_under_cgroup into #ifdef CONFIG_CGROUPS
block together with cgrp_storage_get and delete.

>  struct send_signal_irq_work {
>         struct irq_work irq_work;
>         struct task_struct *task;
> --
> 2.46.0
>