[PATCH bpf-next 0/2] Introduce a new bpf helper of bpf_task_under_cgroup

[PATCH bpf-next 0/2] Introduce a new bpf helper of bpf_task_under_cgroup

[Feng zhou]

From: Feng Zhou <zhoufeng.zf@bytedance.com>

Trace sched related functions, such as enqueue_task_fair, it is necessary to
specify a task instead of the current task which within a given cgroup to a map.

Feng Zhou (2):
  bpf: Add bpf_task_under_cgroup helper
  selftests/bpf: Add testcase for bpf_task_under_cgroup

 include/uapi/linux/bpf.h                      | 13 +++++
 kernel/bpf/verifier.c                         |  4 +-
 kernel/trace/bpf_trace.c                      | 31 ++++++++++++
 tools/include/uapi/linux/bpf.h                | 13 +++++
 .../bpf/prog_tests/task_under_cgroup.c        | 49 +++++++++++++++++++
 .../bpf/progs/test_task_under_cgroup.c        | 31 ++++++++++++
 6 files changed, 140 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_task_under_cgroup.c

-- 
2.20.1

[PATCH bpf-next 1/2] bpf: Add bpf_task_under_cgroup helper

[Feng zhou]

From: Feng Zhou <zhoufeng.zf@bytedance.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@bytedance.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);
+}
+
+static const struct bpf_func_proto bpf_task_under_cgroup_proto = {
+	.func           = bpf_task_under_cgroup,
+	.gpl_only       = false,
+	.ret_type       = RET_INTEGER,
+	.arg1_type      = ARG_CONST_MAP_PTR,
+	.arg2_type	= ARG_PTR_TO_BTF_ID,
+	.arg2_btf_id	= &btf_tracing_ids[BTF_TRACING_TYPE_TASK],
+	.arg3_type      = ARG_ANYTHING,
+};
+
 struct send_signal_irq_work {
 	struct irq_work irq_work;
 	struct task_struct *task;
@@ -1510,6 +1539,8 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_find_vma_proto;
 	case BPF_FUNC_trace_vprintk:
 		return bpf_get_trace_vprintk_proto();
+	case BPF_FUNC_task_under_cgroup:
+		return &bpf_task_under_cgroup_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4b20a7269bee..3d31ddb39e10 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/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
-- 
2.20.1

[PATCH bpf-next 2/2] selftests/bpf: Add testcase for bpf_task_under_cgroup

[Feng zhou]

From: Feng Zhou <zhoufeng.zf@bytedance.com>

test_progs:
Tests new ebpf helpers bpf_task_under_cgroup.

The bpf program saves the pid which call the getuid syscall within a
given cgroup to a map to the remote_pid, which is convenient for the
user-mode program to verify the test correctness.

The user-mode program creates its own mount namespace, and mounts the
cgroupsv2 hierarchy in there, call the getuid syscall, then check if
remote_pid and local_pid are equal.

Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
---
 .../bpf/prog_tests/task_under_cgroup.c        | 49 +++++++++++++++++++
 .../bpf/progs/test_task_under_cgroup.c        | 31 ++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_task_under_cgroup.c

diff --git a/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
new file mode 100644
index 000000000000..4dd704b11a95
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/task_under_cgroup.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Bytedance */
+
+#include <test_progs.h>
+#include "test_task_under_cgroup.skel.h"
+
+#define FOO	"/foo"
+
+void test_task_under_cgroup(void)
+{
+	struct test_task_under_cgroup *skel;
+	int ret, foo = -1, idx = 0;
+
+	skel = test_task_under_cgroup__open();
+	if (!ASSERT_OK_PTR(skel, "test_task_under_cgroup__open"))
+		return;
+
+	skel->rodata->local_pid = getpid();
+
+	ret = test_task_under_cgroup__load(skel);
+	if (!ASSERT_OK(ret, "test_task_under_cgroup__load"))
+		goto cleanup;
+
+	ret = test_task_under_cgroup__attach(skel);
+	if (!ASSERT_OK(ret, "test_task_under_cgroup__attach"))
+		goto cleanup;
+
+	foo = test__join_cgroup(FOO);
+	if (!ASSERT_OK(foo < 0, "cgroup_join_foo"))
+		goto cleanup;
+
+	ret = bpf_map_update_elem(bpf_map__fd(skel->maps.cgroup_map), &idx,
+				  &foo, BPF_ANY);
+	if (!ASSERT_OK(ret < 0, "cgroup_map update"))
+		goto cleanup;
+
+	syscall(__NR_getuid);
+
+	test_task_under_cgroup__detach(skel);
+
+	ASSERT_EQ(skel->bss->remote_pid, skel->rodata->local_pid,
+		  "test task_under_cgroup");
+
+cleanup:
+	if (foo)
+		close(foo);
+
+	test_task_under_cgroup__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
new file mode 100644
index 000000000000..0f3d53f636de
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_task_under_cgroup.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Bytedance */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+const volatile int local_pid;
+int remote_pid;
+
+struct {
+	__uint(type, BPF_MAP_TYPE_CGROUP_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u32);
+} cgroup_map SEC(".maps");
+
+SEC("tp/syscalls/sys_enter_getuid")
+int sysenter_getuid(const void *ctx)
+{
+	if (local_pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+
+	if (!bpf_task_under_cgroup(&cgroup_map, bpf_get_current_task_btf(), 0))
+		return 0;
+
+	remote_pid = local_pid;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.20.1

Re: [PATCH bpf-next 1/2] bpf: Add bpf_task_under_cgroup helper

[Alexei Starovoitov]

On Thu, Apr 20, 2023 at 12:27 AM Feng zhou <zhoufeng.zf@bytedance.com> wrote:
>
> From: Feng Zhou <zhoufeng.zf@bytedance.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@bytedance.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.

Re: [External] Re: [PATCH bpf-next 1/2] bpf: Add bpf_task_under_cgroup helper

[Feng Zhou]

在 2023/4/21 02:22, Alexei Starovoitov 写道:
> On Thu, Apr 20, 2023 at 12:27 AM Feng zhou <zhoufeng.zf@bytedance.com> wrote:
>> From: Feng Zhou <zhoufeng.zf@bytedance.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@bytedance.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.
Will do, thanks.