diff mbox series

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

Message ID 20240307232705.1116787-1-yonghong.song@linux.dev (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf: Allow helper bpf_get_ns_current_pid_tgid() in cgroup/sk_msg programs | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 968 this patch: 968
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 11 maintainers not CCed: haoluo@google.com song@kernel.org pabeni@redhat.com eddyz87@gmail.com netdev@vger.kernel.org sdf@google.com martin.lau@linux.dev kpsingh@kernel.org kuba@kernel.org edumazet@google.com jolsa@kernel.org
netdev/build_clang success Errors and warnings before: 957 this patch: 957
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 984 this patch: 984
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc

Commit Message

Yonghong Song March 7, 2024, 11:27 p.m. UTC 
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(+)

Comments

Andrii Nakryiko March 9, 2024, 1:06 a.m. UTC | #1 
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
>
Yonghong Song March 9, 2024, 6:39 p.m. UTC | #2 
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
>>
Alexei Starovoitov March 9, 2024, 7:10 p.m. UTC | #3 
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.
Yonghong Song March 9, 2024, 9:03 p.m. UTC | #4 
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.
diff mbox series

Patch

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: