[PATCH bpf-next 0/5] bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup/sk_msg programs

[PATCH bpf-next 0/5] bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup/sk_msg programs

[Yonghong Song]

Currently bpf_get_current_pid_tgid() is allowed in tracing, cgroup
and sk_msg progs while bpf_get_ns_current_pid_tgid() is only allowed
in tracing progs.

We have an internal use case where for an application running
in a container (with pid namespace), user wants to get
the pid associated with the pid namespace in a cgroup bpf
program. Besides cgroup, the only prog type, supporting
bpf_get_current_pid_tgid() but not bpf_get_ns_current_pid_tgid(),
is sk_msg and this patch set added it as well.

Patch 1 added the kernel support and patches 2-5 added
the test for cgroup and sk_msg.

Yonghong Song (5):
  bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup and sk_msg
    programs
  selftests/bpf: Replace CHECK with ASSERT_* in ns_current_pid_tgid test
  selftests/bpf: Refactor out some functions in ns_current_pid_tgid test
  selftests/bpf: Add a cgroup prog bpf_get_ns_current_pid_tgid() test
  selftests/bpf: Add a sk_msg prog bpf_get_ns_current_pid_tgid() test

 kernel/bpf/cgroup.c                           |   2 +
 net/core/filter.c                             |   2 +
 .../bpf/prog_tests/ns_current_pid_tgid.c      | 195 +++++++++++++++---
 .../bpf/progs/test_ns_current_pid_tgid.c      |  31 ++-
 4 files changed, 196 insertions(+), 34 deletions(-)

-- 
2.43.0

[PATCH bpf-next 1/5] bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup and sk_msg programs

[Yonghong Song]

Currently bpf_get_current_pid_tgid() is allowed in tracing, cgroup
and sk_msg progs while bpf_get_ns_current_pid_tgid() is only allowed
in tracing progs.

We have an internal use case where for an application running
in a container (with pid namespace), user wants to get
the pid associated with the pid namespace in a cgroup bpf
program. Currently, cgroup bpf progs already allow
bpf_get_current_pid_tgid(). Let us allow bpf_get_ns_current_pid_tgid()
as well.

With auditing the code, bpf_get_current_pid_tgid() is also used
by sk_msg prog. So I added bpf_get_ns_current_pid_tgid()
support for sk_msg prog, so now for all places where
bpf_get_current_pid_tgid() can be used, bpf_get_ns_current_pid_tgid()
can be used as well.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/cgroup.c | 2 ++
 net/core/filter.c   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 5a568bbbeaeb..375b92204881 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -2577,6 +2577,8 @@ cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_current_uid_gid_proto;
 	case BPF_FUNC_get_current_pid_tgid:
 		return &bpf_get_current_pid_tgid_proto;
+	case BPF_FUNC_get_ns_current_pid_tgid:
+		return &bpf_get_ns_current_pid_tgid_proto;
 	case BPF_FUNC_get_current_comm:
 		return &bpf_get_current_comm_proto;
 #ifdef CONFIG_CGROUP_NET_CLASSID
diff --git a/net/core/filter.c b/net/core/filter.c
index 8adf95765cdd..d4e43303a66b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -8344,6 +8344,8 @@ sk_msg_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_current_uid_gid_proto;
 	case BPF_FUNC_get_current_pid_tgid:
 		return &bpf_get_current_pid_tgid_proto;
+	case BPF_FUNC_get_ns_current_pid_tgid:
+		return &bpf_get_ns_current_pid_tgid_proto;
 	case BPF_FUNC_sk_storage_get:
 		return &bpf_sk_storage_get_proto;
 	case BPF_FUNC_sk_storage_delete:
-- 
2.43.0

Re: [PATCH bpf-next 1/5] bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup and sk_msg programs

[Andrii Nakryiko]

On Thu, Mar 7, 2024 at 3:27 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> Currently bpf_get_current_pid_tgid() is allowed in tracing, cgroup
> and sk_msg progs while bpf_get_ns_current_pid_tgid() is only allowed
> in tracing progs.
>
> We have an internal use case where for an application running
> in a container (with pid namespace), user wants to get
> the pid associated with the pid namespace in a cgroup bpf
> program. Currently, cgroup bpf progs already allow
> bpf_get_current_pid_tgid(). Let us allow bpf_get_ns_current_pid_tgid()
> as well.
>
> With auditing the code, bpf_get_current_pid_tgid() is also used
> by sk_msg prog. So I added bpf_get_ns_current_pid_tgid()
> support for sk_msg prog, so now for all places where
> bpf_get_current_pid_tgid() can be used, bpf_get_ns_current_pid_tgid()
> can be used as well.
>

If tracing can call both bpf_get_current_pid_tgid() and
bpf_get_ns_current_pid_tgid(), can't we just add both into
bpf_base_func_proto() and have them available for all types of BPF
programs? If it's safe for tracing, it's safe for any program type, so
why not?

> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  kernel/bpf/cgroup.c | 2 ++
>  net/core/filter.c   | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 5a568bbbeaeb..375b92204881 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -2577,6 +2577,8 @@ cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>                 return &bpf_get_current_uid_gid_proto;
>         case BPF_FUNC_get_current_pid_tgid:
>                 return &bpf_get_current_pid_tgid_proto;
> +       case BPF_FUNC_get_ns_current_pid_tgid:
> +               return &bpf_get_ns_current_pid_tgid_proto;
>         case BPF_FUNC_get_current_comm:
>                 return &bpf_get_current_comm_proto;
>  #ifdef CONFIG_CGROUP_NET_CLASSID
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 8adf95765cdd..d4e43303a66b 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -8344,6 +8344,8 @@ sk_msg_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>                 return &bpf_get_current_uid_gid_proto;
>         case BPF_FUNC_get_current_pid_tgid:
>                 return &bpf_get_current_pid_tgid_proto;
> +       case BPF_FUNC_get_ns_current_pid_tgid:
> +               return &bpf_get_ns_current_pid_tgid_proto;
>         case BPF_FUNC_sk_storage_get:
>                 return &bpf_sk_storage_get_proto;
>         case BPF_FUNC_sk_storage_delete:
> --
> 2.43.0
>

Re: [PATCH bpf-next 1/5] bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup and sk_msg programs

[Yonghong Song]

On 3/8/24 5:06 PM, Andrii Nakryiko wrote:
> On Thu, Mar 7, 2024 at 3:27 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Currently bpf_get_current_pid_tgid() is allowed in tracing, cgroup
>> and sk_msg progs while bpf_get_ns_current_pid_tgid() is only allowed
>> in tracing progs.
>>
>> We have an internal use case where for an application running
>> in a container (with pid namespace), user wants to get
>> the pid associated with the pid namespace in a cgroup bpf
>> program. Currently, cgroup bpf progs already allow
>> bpf_get_current_pid_tgid(). Let us allow bpf_get_ns_current_pid_tgid()
>> as well.
>>
>> With auditing the code, bpf_get_current_pid_tgid() is also used
>> by sk_msg prog. So I added bpf_get_ns_current_pid_tgid()
>> support for sk_msg prog, so now for all places where
>> bpf_get_current_pid_tgid() can be used, bpf_get_ns_current_pid_tgid()
>> can be used as well.
>>
> If tracing can call both bpf_get_current_pid_tgid() and
> bpf_get_ns_current_pid_tgid(), can't we just add both into
> bpf_base_func_proto() and have them available for all types of BPF
> programs? If it's safe for tracing, it's safe for any program type, so
> why not?

Do we need any capability to control bpf_get_[ns_]current_pid_tgid()?
nothing or CAP_BPF or CAP_PERFMON? In my opinion, pid/tgid
is available to user space and there is no leaking kernel private
data here, so bpf prog should be able to use it in all prog types.
I will wait for a few days. If no people object, I will incorporate
this in v2.

>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   kernel/bpf/cgroup.c | 2 ++
>>   net/core/filter.c   | 2 ++
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> index 5a568bbbeaeb..375b92204881 100644
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
>> @@ -2577,6 +2577,8 @@ cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>>                  return &bpf_get_current_uid_gid_proto;
>>          case BPF_FUNC_get_current_pid_tgid:
>>                  return &bpf_get_current_pid_tgid_proto;
>> +       case BPF_FUNC_get_ns_current_pid_tgid:
>> +               return &bpf_get_ns_current_pid_tgid_proto;
>>          case BPF_FUNC_get_current_comm:
>>                  return &bpf_get_current_comm_proto;
>>   #ifdef CONFIG_CGROUP_NET_CLASSID
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 8adf95765cdd..d4e43303a66b 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -8344,6 +8344,8 @@ sk_msg_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>>                  return &bpf_get_current_uid_gid_proto;
>>          case BPF_FUNC_get_current_pid_tgid:
>>                  return &bpf_get_current_pid_tgid_proto;
>> +       case BPF_FUNC_get_ns_current_pid_tgid:
>> +               return &bpf_get_ns_current_pid_tgid_proto;
>>          case BPF_FUNC_sk_storage_get:
>>                  return &bpf_sk_storage_get_proto;
>>          case BPF_FUNC_sk_storage_delete:
>> --
>> 2.43.0
>>

Re: [PATCH bpf-next 1/5] bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup and sk_msg programs

[Alexei Starovoitov]

On Sat, Mar 9, 2024 at 10:40 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 3/8/24 5:06 PM, Andrii Nakryiko wrote:
> > On Thu, Mar 7, 2024 at 3:27 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> >> Currently bpf_get_current_pid_tgid() is allowed in tracing, cgroup
> >> and sk_msg progs while bpf_get_ns_current_pid_tgid() is only allowed
> >> in tracing progs.
> >>
> >> We have an internal use case where for an application running
> >> in a container (with pid namespace), user wants to get
> >> the pid associated with the pid namespace in a cgroup bpf
> >> program. Currently, cgroup bpf progs already allow
> >> bpf_get_current_pid_tgid(). Let us allow bpf_get_ns_current_pid_tgid()
> >> as well.
> >>
> >> With auditing the code, bpf_get_current_pid_tgid() is also used
> >> by sk_msg prog. So I added bpf_get_ns_current_pid_tgid()
> >> support for sk_msg prog, so now for all places where
> >> bpf_get_current_pid_tgid() can be used, bpf_get_ns_current_pid_tgid()
> >> can be used as well.
> >>
> > If tracing can call both bpf_get_current_pid_tgid() and
> > bpf_get_ns_current_pid_tgid(), can't we just add both into
> > bpf_base_func_proto() and have them available for all types of BPF
> > programs? If it's safe for tracing, it's safe for any program type, so
> > why not?
>
> Do we need any capability to control bpf_get_[ns_]current_pid_tgid()?
> nothing or CAP_BPF or CAP_PERFMON? In my opinion, pid/tgid
> is available to user space and there is no leaking kernel private
> data here, so bpf prog should be able to use it in all prog types.
> I will wait for a few days. If no people object, I will incorporate
> this in v2.

Yeah. It's safe without extra cap-s.
There is ns_match() inside. Nothing can leak.
Let's just move it to base_func_proto.

Re: [PATCH bpf-next 1/5] bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup and sk_msg programs

[Yonghong Song]

On 3/9/24 11:10 AM, Alexei Starovoitov wrote:
> On Sat, Mar 9, 2024 at 10:40 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 3/8/24 5:06 PM, Andrii Nakryiko wrote:
>>> On Thu, Mar 7, 2024 at 3:27 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>>> Currently bpf_get_current_pid_tgid() is allowed in tracing, cgroup
>>>> and sk_msg progs while bpf_get_ns_current_pid_tgid() is only allowed
>>>> in tracing progs.
>>>>
>>>> We have an internal use case where for an application running
>>>> in a container (with pid namespace), user wants to get
>>>> the pid associated with the pid namespace in a cgroup bpf
>>>> program. Currently, cgroup bpf progs already allow
>>>> bpf_get_current_pid_tgid(). Let us allow bpf_get_ns_current_pid_tgid()
>>>> as well.
>>>>
>>>> With auditing the code, bpf_get_current_pid_tgid() is also used
>>>> by sk_msg prog. So I added bpf_get_ns_current_pid_tgid()
>>>> support for sk_msg prog, so now for all places where
>>>> bpf_get_current_pid_tgid() can be used, bpf_get_ns_current_pid_tgid()
>>>> can be used as well.
>>>>
>>> If tracing can call both bpf_get_current_pid_tgid() and
>>> bpf_get_ns_current_pid_tgid(), can't we just add both into
>>> bpf_base_func_proto() and have them available for all types of BPF
>>> programs? If it's safe for tracing, it's safe for any program type, so
>>> why not?
>> Do we need any capability to control bpf_get_[ns_]current_pid_tgid()?
>> nothing or CAP_BPF or CAP_PERFMON? In my opinion, pid/tgid
>> is available to user space and there is no leaking kernel private
>> data here, so bpf prog should be able to use it in all prog types.
>> I will wait for a few days. If no people object, I will incorporate
>> this in v2.
> Yeah. It's safe without extra cap-s.
> There is ns_match() inside. Nothing can leak.
> Let's just move it to base_func_proto.

Sounds good. Will move both helpers to base_func_proto.

[PATCH bpf-next 2/5] selftests/bpf: Replace CHECK with ASSERT_* in ns_current_pid_tgid test

[Yonghong Song]

Replace CHECK in selftest ns_current_pid_tgid with recommended ASSERT_* style.
I also shortened subtest name as the prefix of subtest name is covered
by the test name already.

This patch does fix a testing issue. Currently even if bss->user_{pid,tgid}
is not correct, the test still passed since the clone func returns 0.
I fixed it to return a non-zero value if bss->user_{pid,tgid} is incorrect.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../bpf/prog_tests/ns_current_pid_tgid.c      | 36 ++++++++++---------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
index 24d493482ffc..3a0664a86243 100644
--- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
@@ -20,19 +20,19 @@ static int test_current_pid_tgid(void *args)
 {
 	struct test_ns_current_pid_tgid__bss  *bss;
 	struct test_ns_current_pid_tgid *skel;
-	int err = -1, duration = 0;
+	int ret = -1, err;
 	pid_t tgid, pid;
 	struct stat st;
 
 	skel = test_ns_current_pid_tgid__open_and_load();
-	if (CHECK(!skel, "skel_open_load", "failed to load skeleton\n"))
-		goto cleanup;
+	if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open_and_load"))
+		goto out;
 
 	pid = syscall(SYS_gettid);
 	tgid = getpid();
 
 	err = stat("/proc/self/ns/pid", &st);
-	if (CHECK(err, "stat", "failed /proc/self/ns/pid: %d\n", err))
+	if (!ASSERT_OK(err, "stat /proc/self/ns/pid"))
 		goto cleanup;
 
 	bss = skel->bss;
@@ -42,24 +42,26 @@ static int test_current_pid_tgid(void *args)
 	bss->user_tgid = 0;
 
 	err = test_ns_current_pid_tgid__attach(skel);
-	if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+	if (!ASSERT_OK(err, "test_ns_current_pid_tgid__attach"))
 		goto cleanup;
 
 	/* trigger tracepoint */
 	usleep(1);
-	ASSERT_EQ(bss->user_pid, pid, "pid");
-	ASSERT_EQ(bss->user_tgid, tgid, "tgid");
-	err = 0;
+	if (!ASSERT_EQ(bss->user_pid, pid, "pid"))
+		goto cleanup;
+	if (!ASSERT_EQ(bss->user_tgid, tgid, "tgid"))
+		goto cleanup;
+	ret = 0;
 
 cleanup:
-	 test_ns_current_pid_tgid__destroy(skel);
-
-	return err;
+	test_ns_current_pid_tgid__destroy(skel);
+out:
+	return ret;
 }
 
 static void test_ns_current_pid_tgid_new_ns(void)
 {
-	int wstatus, duration = 0;
+	int wstatus;
 	pid_t cpid;
 
 	/* Create a process in a new namespace, this process
@@ -68,21 +70,21 @@ static void test_ns_current_pid_tgid_new_ns(void)
 	cpid = clone(test_current_pid_tgid, child_stack + STACK_SIZE,
 		     CLONE_NEWPID | SIGCHLD, NULL);
 
-	if (CHECK(cpid == -1, "clone", "%s\n", strerror(errno)))
+	if (!ASSERT_NEQ(cpid, -1, "clone"))
 		return;
 
-	if (CHECK(waitpid(cpid, &wstatus, 0) == -1, "waitpid", "%s\n", strerror(errno)))
+	if (!ASSERT_NEQ(waitpid(cpid, &wstatus, 0), -1, "waitpid"))
 		return;
 
-	if (CHECK(WEXITSTATUS(wstatus) != 0, "newns_pidtgid", "failed"))
+	if (!ASSERT_OK(WEXITSTATUS(wstatus), "newns_pidtgid"))
 		return;
 }
 
 /* TODO: use a different tracepoint */
 void serial_test_ns_current_pid_tgid(void)
 {
-	if (test__start_subtest("ns_current_pid_tgid_root_ns"))
+	if (test__start_subtest("root_ns_tp"))
 		test_current_pid_tgid(NULL);
-	if (test__start_subtest("ns_current_pid_tgid_new_ns"))
+	if (test__start_subtest("new_ns_tp"))
 		test_ns_current_pid_tgid_new_ns();
 }
-- 
2.43.0

[PATCH bpf-next 3/5] selftests/bpf: Refactor out some functions in ns_current_pid_tgid test

[Yonghong Song]

Refactor some functions in both user space code and bpf program
as these functions are used by later cgroup/sk_msg tests.
Another change is to mark tp program optional loading as later
patches will use optional loading as well since they have quite
different attachment and testing logic.

There is no functionality change.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../bpf/prog_tests/ns_current_pid_tgid.c      | 53 ++++++++++++-------
 .../bpf/progs/test_ns_current_pid_tgid.c      | 10 ++--
 2 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
index 3a0664a86243..847d7b70e290 100644
--- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
@@ -16,30 +16,46 @@
 #define STACK_SIZE (1024 * 1024)
 static char child_stack[STACK_SIZE];
 
-static int test_current_pid_tgid(void *args)
+static int get_pid_tgid(pid_t *pid, pid_t *tgid,
+			struct test_ns_current_pid_tgid__bss *bss)
 {
-	struct test_ns_current_pid_tgid__bss  *bss;
-	struct test_ns_current_pid_tgid *skel;
-	int ret = -1, err;
-	pid_t tgid, pid;
 	struct stat st;
+	int err;
 
-	skel = test_ns_current_pid_tgid__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open_and_load"))
-		goto out;
-
-	pid = syscall(SYS_gettid);
-	tgid = getpid();
+	*pid = syscall(SYS_gettid);
+	*tgid = getpid();
 
 	err = stat("/proc/self/ns/pid", &st);
 	if (!ASSERT_OK(err, "stat /proc/self/ns/pid"))
-		goto cleanup;
+		return err;
 
-	bss = skel->bss;
 	bss->dev = st.st_dev;
 	bss->ino = st.st_ino;
 	bss->user_pid = 0;
 	bss->user_tgid = 0;
+	return 0;
+}
+
+static int test_current_pid_tgid_tp(void *args)
+{
+	struct test_ns_current_pid_tgid__bss  *bss;
+	struct test_ns_current_pid_tgid *skel;
+	int ret = -1, err;
+	pid_t tgid, pid;
+
+	skel = test_ns_current_pid_tgid__open();
+	if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open"))
+		return ret;
+
+	bpf_program__set_autoload(skel->progs.tp_handler, true);
+
+	err = test_ns_current_pid_tgid__load(skel);
+	if (!ASSERT_OK(err, "test_ns_current_pid_tgid__load"))
+		goto cleanup;
+
+	bss = skel->bss;
+	if (get_pid_tgid(&pid, &tgid, bss))
+		goto cleanup;
 
 	err = test_ns_current_pid_tgid__attach(skel);
 	if (!ASSERT_OK(err, "test_ns_current_pid_tgid__attach"))
@@ -55,11 +71,10 @@ static int test_current_pid_tgid(void *args)
 
 cleanup:
 	test_ns_current_pid_tgid__destroy(skel);
-out:
 	return ret;
 }
 
-static void test_ns_current_pid_tgid_new_ns(void)
+static void test_ns_current_pid_tgid_new_ns(int (*fn)(void *), void *arg)
 {
 	int wstatus;
 	pid_t cpid;
@@ -67,8 +82,8 @@ static void test_ns_current_pid_tgid_new_ns(void)
 	/* Create a process in a new namespace, this process
 	 * will be the init process of this new namespace hence will be pid 1.
 	 */
-	cpid = clone(test_current_pid_tgid, child_stack + STACK_SIZE,
-		     CLONE_NEWPID | SIGCHLD, NULL);
+	cpid = clone(fn, child_stack + STACK_SIZE,
+		     CLONE_NEWPID | SIGCHLD, arg);
 
 	if (!ASSERT_NEQ(cpid, -1, "clone"))
 		return;
@@ -84,7 +99,7 @@ static void test_ns_current_pid_tgid_new_ns(void)
 void serial_test_ns_current_pid_tgid(void)
 {
 	if (test__start_subtest("root_ns_tp"))
-		test_current_pid_tgid(NULL);
+		test_current_pid_tgid_tp(NULL);
 	if (test__start_subtest("new_ns_tp"))
-		test_ns_current_pid_tgid_new_ns();
+		test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_tp, NULL);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
index 0763d49f9c42..aa3ec7ca16d9 100644
--- a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
@@ -10,17 +10,21 @@ __u64 user_tgid = 0;
 __u64 dev = 0;
 __u64 ino = 0;
 
-SEC("tracepoint/syscalls/sys_enter_nanosleep")
-int handler(const void *ctx)
+static void get_pid_tgid(void)
 {
 	struct bpf_pidns_info nsdata;
 
 	if (bpf_get_ns_current_pid_tgid(dev, ino, &nsdata, sizeof(struct bpf_pidns_info)))
-		return 0;
+		return;
 
 	user_pid = nsdata.pid;
 	user_tgid = nsdata.tgid;
+}
 
+SEC("?tracepoint/syscalls/sys_enter_nanosleep")
+int tp_handler(const void *ctx)
+{
+	get_pid_tgid();
 	return 0;
 }
 
-- 
2.43.0

[PATCH bpf-next 4/5] selftests/bpf: Add a cgroup prog bpf_get_ns_current_pid_tgid() test

[Yonghong Song]

Add a cgroup bpf program test where the bpf program is running
in a pid namespace. The test is successfully:
  #165/3   ns_current_pid_tgid/new_ns_cgrp:OK

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../bpf/prog_tests/ns_current_pid_tgid.c      | 54 +++++++++++++++++++
 .../bpf/progs/test_ns_current_pid_tgid.c      |  7 +++
 2 files changed, 61 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
index 847d7b70e290..a307303e01ee 100644
--- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
@@ -12,6 +12,7 @@
 #include <sys/wait.h>
 #include <sys/mount.h>
 #include <sys/fcntl.h>
+#include "network_helpers.h"
 
 #define STACK_SIZE (1024 * 1024)
 static char child_stack[STACK_SIZE];
@@ -74,6 +75,50 @@ static int test_current_pid_tgid_tp(void *args)
 	return ret;
 }
 
+static int test_current_pid_tgid_cgrp(void *args)
+{
+	struct test_ns_current_pid_tgid__bss *bss;
+	struct test_ns_current_pid_tgid *skel;
+	int server_fd = -1, ret = -1, err;
+	int cgroup_fd = *(int *)args;
+	pid_t tgid, pid;
+
+	skel = test_ns_current_pid_tgid__open();
+	if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open"))
+		return ret;
+
+	bpf_program__set_autoload(skel->progs.cgroup_bind4, true);
+
+	err = test_ns_current_pid_tgid__load(skel);
+	if (!ASSERT_OK(err, "test_ns_current_pid_tgid__load"))
+		goto cleanup;
+
+	bss = skel->bss;
+	if (get_pid_tgid(&pid, &tgid, bss))
+		goto cleanup;
+
+	skel->links.cgroup_bind4 = bpf_program__attach_cgroup(
+		skel->progs.cgroup_bind4, cgroup_fd);
+	if (!ASSERT_OK_PTR(skel->links.cgroup_bind4, "bpf_program__attach_cgroup"))
+		goto cleanup;
+
+	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
+	if (!ASSERT_GE(server_fd, 0, "start_server"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(bss->user_pid, pid, "pid"))
+		goto cleanup;
+	if (!ASSERT_EQ(bss->user_tgid, tgid, "tgid"))
+		goto cleanup;
+	ret = 0;
+
+cleanup:
+	if (server_fd >= 0)
+		close(server_fd);
+	test_ns_current_pid_tgid__destroy(skel);
+	return ret;
+}
+
 static void test_ns_current_pid_tgid_new_ns(int (*fn)(void *), void *arg)
 {
 	int wstatus;
@@ -102,4 +147,13 @@ void serial_test_ns_current_pid_tgid(void)
 		test_current_pid_tgid_tp(NULL);
 	if (test__start_subtest("new_ns_tp"))
 		test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_tp, NULL);
+	if (test__start_subtest("new_ns_cgrp")) {
+		int cgroup_fd = -1;
+
+		cgroup_fd = test__join_cgroup("/sock_addr");
+		if (ASSERT_GE(cgroup_fd, 0, "join_cgroup")) {
+			test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_cgrp, &cgroup_fd);
+			close(cgroup_fd);
+		}
+	}
 }
diff --git a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
index aa3ec7ca16d9..d0010e698f66 100644
--- a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
@@ -28,4 +28,11 @@ int tp_handler(const void *ctx)
 	return 0;
 }
 
+SEC("?cgroup/bind4")
+int cgroup_bind4(struct bpf_sock_addr *ctx)
+{
+	get_pid_tgid();
+	return 1;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH bpf-next 4/5] selftests/bpf: Add a cgroup prog bpf_get_ns_current_pid_tgid() test

[Martin KaFai Lau]

On 3/7/24 3:27 PM, Yonghong Song wrote:
> Add a cgroup bpf program test where the bpf program is running
> in a pid namespace. The test is successfully:
>    #165/3   ns_current_pid_tgid/new_ns_cgrp:OK
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>   .../bpf/prog_tests/ns_current_pid_tgid.c      | 54 +++++++++++++++++++
>   .../bpf/progs/test_ns_current_pid_tgid.c      |  7 +++
>   2 files changed, 61 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
> index 847d7b70e290..a307303e01ee 100644
> --- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
> +++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
> @@ -12,6 +12,7 @@
>   #include <sys/wait.h>
>   #include <sys/mount.h>
>   #include <sys/fcntl.h>
> +#include "network_helpers.h"
>   
>   #define STACK_SIZE (1024 * 1024)
>   static char child_stack[STACK_SIZE];
> @@ -74,6 +75,50 @@ static int test_current_pid_tgid_tp(void *args)
>   	return ret;
>   }
>   
> +static int test_current_pid_tgid_cgrp(void *args)
> +{
> +	struct test_ns_current_pid_tgid__bss *bss;
> +	struct test_ns_current_pid_tgid *skel;
> +	int server_fd = -1, ret = -1, err;
> +	int cgroup_fd = *(int *)args;
> +	pid_t tgid, pid;
> +
> +	skel = test_ns_current_pid_tgid__open();
> +	if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open"))
> +		return ret;
> +
> +	bpf_program__set_autoload(skel->progs.cgroup_bind4, true);
> +
> +	err = test_ns_current_pid_tgid__load(skel);
> +	if (!ASSERT_OK(err, "test_ns_current_pid_tgid__load"))
> +		goto cleanup;
> +
> +	bss = skel->bss;
> +	if (get_pid_tgid(&pid, &tgid, bss))
> +		goto cleanup;
> +
> +	skel->links.cgroup_bind4 = bpf_program__attach_cgroup(
> +		skel->progs.cgroup_bind4, cgroup_fd);
> +	if (!ASSERT_OK_PTR(skel->links.cgroup_bind4, "bpf_program__attach_cgroup"))
> +		goto cleanup;
> +
> +	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);

nit. This binds to ADDR_ANY. Regardless, it is a good idea to create a netns for 
this subtest. It will be one less thing to worry about when removing "serial_" 
from this test eventually. The same for the subtest in patch 5 also.

> +	if (!ASSERT_GE(server_fd, 0, "start_server"))
> +		goto cleanup;
> +
> +	if (!ASSERT_EQ(bss->user_pid, pid, "pid"))
> +		goto cleanup;
> +	if (!ASSERT_EQ(bss->user_tgid, tgid, "tgid"))
> +		goto cleanup;
> +	ret = 0;
> +
> +cleanup:
> +	if (server_fd >= 0)
> +		close(server_fd);
> +	test_ns_current_pid_tgid__destroy(skel);
> +	return ret;
> +}
> +
>   static void test_ns_current_pid_tgid_new_ns(int (*fn)(void *), void *arg)
>   {
>   	int wstatus;
> @@ -102,4 +147,13 @@ void serial_test_ns_current_pid_tgid(void)
>   		test_current_pid_tgid_tp(NULL);
>   	if (test__start_subtest("new_ns_tp"))
>   		test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_tp, NULL);
> +	if (test__start_subtest("new_ns_cgrp")) {
> +		int cgroup_fd = -1;

nit. no need to "= -1;"

Other patches lgtm.

> +
> +		cgroup_fd = test__join_cgroup("/sock_addr");
> +		if (ASSERT_GE(cgroup_fd, 0, "join_cgroup")) {
> +			test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_cgrp, &cgroup_fd);
> +			close(cgroup_fd);
> +		}
> +	}
>   }
> diff --git a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
> index aa3ec7ca16d9..d0010e698f66 100644
> --- a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
> +++ b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
> @@ -28,4 +28,11 @@ int tp_handler(const void *ctx)
>   	return 0;
>   }
>   
> +SEC("?cgroup/bind4")
> +int cgroup_bind4(struct bpf_sock_addr *ctx)
> +{
> +	get_pid_tgid();
> +	return 1;
> +}
> +
>   char _license[] SEC("license") = "GPL";

Re: [PATCH bpf-next 4/5] selftests/bpf: Add a cgroup prog bpf_get_ns_current_pid_tgid() test

[Yonghong Song]

On 3/8/24 11:08 AM, Martin KaFai Lau wrote:
> On 3/7/24 3:27 PM, Yonghong Song wrote:
>> Add a cgroup bpf program test where the bpf program is running
>> in a pid namespace. The test is successfully:
>>    #165/3   ns_current_pid_tgid/new_ns_cgrp:OK
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   .../bpf/prog_tests/ns_current_pid_tgid.c      | 54 +++++++++++++++++++
>>   .../bpf/progs/test_ns_current_pid_tgid.c      |  7 +++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git 
>> a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c 
>> b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
>> index 847d7b70e290..a307303e01ee 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
>> @@ -12,6 +12,7 @@
>>   #include <sys/wait.h>
>>   #include <sys/mount.h>
>>   #include <sys/fcntl.h>
>> +#include "network_helpers.h"
>>     #define STACK_SIZE (1024 * 1024)
>>   static char child_stack[STACK_SIZE];
>> @@ -74,6 +75,50 @@ static int test_current_pid_tgid_tp(void *args)
>>       return ret;
>>   }
>>   +static int test_current_pid_tgid_cgrp(void *args)
>> +{
>> +    struct test_ns_current_pid_tgid__bss *bss;
>> +    struct test_ns_current_pid_tgid *skel;
>> +    int server_fd = -1, ret = -1, err;
>> +    int cgroup_fd = *(int *)args;
>> +    pid_t tgid, pid;
>> +
>> +    skel = test_ns_current_pid_tgid__open();
>> +    if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open"))
>> +        return ret;
>> +
>> +    bpf_program__set_autoload(skel->progs.cgroup_bind4, true);
>> +
>> +    err = test_ns_current_pid_tgid__load(skel);
>> +    if (!ASSERT_OK(err, "test_ns_current_pid_tgid__load"))
>> +        goto cleanup;
>> +
>> +    bss = skel->bss;
>> +    if (get_pid_tgid(&pid, &tgid, bss))
>> +        goto cleanup;
>> +
>> +    skel->links.cgroup_bind4 = bpf_program__attach_cgroup(
>> +        skel->progs.cgroup_bind4, cgroup_fd);
>> +    if (!ASSERT_OK_PTR(skel->links.cgroup_bind4, 
>> "bpf_program__attach_cgroup"))
>> +        goto cleanup;
>> +
>> +    server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
>
> nit. This binds to ADDR_ANY. Regardless, it is a good idea to create a 
> netns for this subtest. It will be one less thing to worry about when 
> removing "serial_" from this test eventually. The same for the subtest 
> in patch 5 also.

Good point. Will do.


>
>> +    if (!ASSERT_GE(server_fd, 0, "start_server"))
>> +        goto cleanup;
>> +
>> +    if (!ASSERT_EQ(bss->user_pid, pid, "pid"))
>> +        goto cleanup;
>> +    if (!ASSERT_EQ(bss->user_tgid, tgid, "tgid"))
>> +        goto cleanup;
>> +    ret = 0;
>> +
>> +cleanup:
>> +    if (server_fd >= 0)
>> +        close(server_fd);
>> +    test_ns_current_pid_tgid__destroy(skel);
>> +    return ret;
>> +}
>> +
>>   static void test_ns_current_pid_tgid_new_ns(int (*fn)(void *), void 
>> *arg)
>>   {
>>       int wstatus;
>> @@ -102,4 +147,13 @@ void serial_test_ns_current_pid_tgid(void)
>>           test_current_pid_tgid_tp(NULL);
>>       if (test__start_subtest("new_ns_tp"))
>> test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_tp, NULL);
>> +    if (test__start_subtest("new_ns_cgrp")) {
>> +        int cgroup_fd = -1;
>
> nit. no need to "= -1;"
Ack. My mistake. Will change.
>
> Other patches lgtm.
>
>> +
>> +        cgroup_fd = test__join_cgroup("/sock_addr");
>> +        if (ASSERT_GE(cgroup_fd, 0, "join_cgroup")) {
>> + test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_cgrp, 
>> &cgroup_fd);
>> +            close(cgroup_fd);
>> +        }
>> +    }
>>   }
>> diff --git 
>> a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c 
>> b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
>> index aa3ec7ca16d9..d0010e698f66 100644
>> --- a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
>> +++ b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
>> @@ -28,4 +28,11 @@ int tp_handler(const void *ctx)
>>       return 0;
>>   }
>>   +SEC("?cgroup/bind4")
>> +int cgroup_bind4(struct bpf_sock_addr *ctx)
>> +{
>> +    get_pid_tgid();
>> +    return 1;
>> +}
>> +
>>   char _license[] SEC("license") = "GPL";
>

[PATCH bpf-next 5/5] selftests/bpf: Add a sk_msg prog bpf_get_ns_current_pid_tgid() test

[Yonghong Song]

Add a sk_msg bpf program test where the program is running in a pid
namespace. The test is successful:
  #165/4   ns_current_pid_tgid/new_ns_sk_msg:OK

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../bpf/prog_tests/ns_current_pid_tgid.c      | 62 +++++++++++++++++++
 .../bpf/progs/test_ns_current_pid_tgid.c      | 14 +++++
 2 files changed, 76 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
index a307303e01ee..f5d48549738a 100644
--- a/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
@@ -119,6 +119,66 @@ static int test_current_pid_tgid_cgrp(void *args)
 	return ret;
 }
 
+static int test_current_pid_tgid_sk_msg(void *args)
+{
+	int verdict, map, server_fd = -1, client_fd = -1;
+	struct test_ns_current_pid_tgid__bss *bss;
+	static const char send_msg[] = "message";
+	struct test_ns_current_pid_tgid *skel;
+	int ret = -1, err, key = 0;
+	pid_t tgid, pid;
+
+	skel = test_ns_current_pid_tgid__open();
+	if (!ASSERT_OK_PTR(skel, "test_ns_current_pid_tgid__open"))
+		return ret;
+
+	bpf_program__set_autoload(skel->progs.sk_msg, true);
+
+	err = test_ns_current_pid_tgid__load(skel);
+	if (!ASSERT_OK(err, "test_ns_current_pid_tgid__load"))
+		goto cleanup;
+
+	bss = skel->bss;
+	if (get_pid_tgid(&pid, &tgid, skel->bss))
+		goto cleanup;
+
+	verdict = bpf_program__fd(skel->progs.sk_msg);
+	map = bpf_map__fd(skel->maps.sock_map);
+	err = bpf_prog_attach(verdict, map, BPF_SK_MSG_VERDICT, 0);
+	if (!ASSERT_OK(err, "prog_attach"))
+		goto cleanup;
+
+	server_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0);
+	if (!ASSERT_GE(server_fd, 0, "start_server"))
+		goto cleanup;
+
+	client_fd = connect_to_fd(server_fd, 0);
+	if (!ASSERT_GE(client_fd, 0, "connect_to_fd"))
+		goto cleanup;
+
+	err = bpf_map_update_elem(map, &key, &client_fd, BPF_ANY);
+	if (!ASSERT_OK(err, "bpf_map_update_elem"))
+		goto cleanup;
+
+	err = send(client_fd, send_msg, sizeof(send_msg), 0);
+	if (!ASSERT_EQ(err, sizeof(send_msg), "send(msg)"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(bss->user_pid, pid, "pid"))
+		goto cleanup;
+	if (!ASSERT_EQ(bss->user_tgid, tgid, "tgid"))
+		goto cleanup;
+	ret = 0;
+
+cleanup:
+	if (server_fd >= 0)
+		close(server_fd);
+	if (client_fd >= 0)
+		close(client_fd);
+	test_ns_current_pid_tgid__destroy(skel);
+	return ret;
+}
+
 static void test_ns_current_pid_tgid_new_ns(int (*fn)(void *), void *arg)
 {
 	int wstatus;
@@ -156,4 +216,6 @@ void serial_test_ns_current_pid_tgid(void)
 			close(cgroup_fd);
 		}
 	}
+	if (test__start_subtest("new_ns_sk_msg"))
+		test_ns_current_pid_tgid_new_ns(test_current_pid_tgid_sk_msg, NULL);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
index d0010e698f66..386315afad65 100644
--- a/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
+++ b/tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
@@ -5,6 +5,13 @@
 #include <stdint.h>
 #include <bpf/bpf_helpers.h>
 
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKMAP);
+	__uint(max_entries, 2);
+	__type(key, __u32);
+	__type(value, __u32);
+} sock_map SEC(".maps");
+
 __u64 user_pid = 0;
 __u64 user_tgid = 0;
 __u64 dev = 0;
@@ -35,4 +42,11 @@ int cgroup_bind4(struct bpf_sock_addr *ctx)
 	return 1;
 }
 
+SEC("?sk_msg")
+int sk_msg(struct sk_msg_md *msg)
+{
+	get_pid_tgid();
+	return SK_PASS;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0