1. Patchew
  2. linux
  3. View series

[PATCH bpf-next v4] bpf: streamline allowed helpers between tracing and base sets

Feng Yang posted 1 patch 8 months ago 
kernel/trace/bpf_trace.c | 72 ----------------------------------------
1 file changed, 72 deletions(-)
[PATCH bpf-next v4] bpf: streamline allowed helpers between tracing and base sets
Posted by Feng Yang 8 months ago 
From: Feng Yang <yangfeng@kylinos.cn>

Many conditional checks in switch-case are redundant
with bpf_base_func_proto and should be removed.

Regarding the permission checks bpf_base_func_proto:
The permission checks in bpf_prog_load (as outlined below)
ensure that the trace has both CAP_BPF and CAP_PERFMON capabilities,
thus enabling the use of corresponding prototypes
in bpf_base_func_proto without adverse effects.
bpf_prog_load
	......
	bpf_cap = bpf_token_capable(token, CAP_BPF);
	......
	if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
	    type != BPF_PROG_TYPE_CGROUP_SKB &&
	    !bpf_cap)
		goto put_token;
	......
	if (is_perfmon_prog_type(type) && !bpf_token_capable(token, CAP_PERFMON))
		goto put_token;
	......

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
Acked-by: Song Liu <song@kernel.org>
---
Changes in v4:
- Only modify patch description information.
- At present, bpf_tracing_func_proto still has the following ID:
- BPF_FUNC_get_current_uid_gid
- BPF_FUNC_get_current_comm
- BPF_FUNC_get_smp_processor_id
- BPF_FUNC_perf_event_read
- BPF_FUNC_probe_read
- BPF_FUNC_probe_read_str
- BPF_FUNC_current_task_under_cgroup
- BPF_FUNC_send_signal
- BPF_FUNC_send_signal_thread
- BPF_FUNC_get_task_stack
- BPF_FUNC_copy_from_user
- BPF_FUNC_copy_from_user_task
- BPF_FUNC_task_storage_get
- BPF_FUNC_task_storage_delete
- BPF_FUNC_get_func_ip
- BPF_FUNC_get_branch_snapshot
- BPF_FUNC_find_vma
- BPF_FUNC_probe_write_user
- I'm not sure which ones can be used by all programs, as Zvi Effron said(https://lore.kernel.org/all/CAC1LvL2SOKojrXPx92J46fFEi3T9TAWb3mC1XKtYzwU=pzTEbQ@mail.gmail.com/)
- get_smp_processor_id also be retained(https://lore.kernel.org/all/CAADnVQ+WYLfoR1W6AsCJF6fNKEUgfxANXP01EQCJh1=99ZpoNw@mail.gmail.com/)

- Link to v3: https://lore.kernel.org/all/20250410070258.276759-1-yangfeng59949@163.com/

Changes in v3:
- Only modify patch description information.
- Link to v2: https://lore.kernel.org/all/20250408071151.229329-1-yangfeng59949@163.com/

Changes in v2:
- Only modify patch description information.
- Link to v1: https://lore.kernel.org/all/20250320032258.116156-1-yangfeng59949@163.com/
---
 kernel/trace/bpf_trace.c | 72 ----------------------------------------
 1 file changed, 72 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0f5906f43d7c..52c432a44aeb 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1430,56 +1430,14 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	const struct bpf_func_proto *func_proto;
 
 	switch (func_id) {
-	case BPF_FUNC_map_lookup_elem:
-		return &bpf_map_lookup_elem_proto;
-	case BPF_FUNC_map_update_elem:
-		return &bpf_map_update_elem_proto;
-	case BPF_FUNC_map_delete_elem:
-		return &bpf_map_delete_elem_proto;
-	case BPF_FUNC_map_push_elem:
-		return &bpf_map_push_elem_proto;
-	case BPF_FUNC_map_pop_elem:
-		return &bpf_map_pop_elem_proto;
-	case BPF_FUNC_map_peek_elem:
-		return &bpf_map_peek_elem_proto;
-	case BPF_FUNC_map_lookup_percpu_elem:
-		return &bpf_map_lookup_percpu_elem_proto;
-	case BPF_FUNC_ktime_get_ns:
-		return &bpf_ktime_get_ns_proto;
-	case BPF_FUNC_ktime_get_boot_ns:
-		return &bpf_ktime_get_boot_ns_proto;
-	case BPF_FUNC_tail_call:
-		return &bpf_tail_call_proto;
-	case BPF_FUNC_get_current_task:
-		return &bpf_get_current_task_proto;
-	case BPF_FUNC_get_current_task_btf:
-		return &bpf_get_current_task_btf_proto;
-	case BPF_FUNC_task_pt_regs:
-		return &bpf_task_pt_regs_proto;
 	case BPF_FUNC_get_current_uid_gid:
 		return &bpf_get_current_uid_gid_proto;
 	case BPF_FUNC_get_current_comm:
 		return &bpf_get_current_comm_proto;
-	case BPF_FUNC_trace_printk:
-		return bpf_get_trace_printk_proto();
 	case BPF_FUNC_get_smp_processor_id:
 		return &bpf_get_smp_processor_id_proto;
-	case BPF_FUNC_get_numa_node_id:
-		return &bpf_get_numa_node_id_proto;
 	case BPF_FUNC_perf_event_read:
 		return &bpf_perf_event_read_proto;
-	case BPF_FUNC_get_prandom_u32:
-		return &bpf_get_prandom_u32_proto;
-	case BPF_FUNC_probe_read_user:
-		return &bpf_probe_read_user_proto;
-	case BPF_FUNC_probe_read_kernel:
-		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
-		       NULL : &bpf_probe_read_kernel_proto;
-	case BPF_FUNC_probe_read_user_str:
-		return &bpf_probe_read_user_str_proto;
-	case BPF_FUNC_probe_read_kernel_str:
-		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
-		       NULL : &bpf_probe_read_kernel_str_proto;
 #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	case BPF_FUNC_probe_read:
 		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
@@ -1489,10 +1447,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		       NULL : &bpf_probe_read_compat_str_proto;
 #endif
 #ifdef CONFIG_CGROUPS
-	case BPF_FUNC_cgrp_storage_get:
-		return &bpf_cgrp_storage_get_proto;
-	case BPF_FUNC_cgrp_storage_delete:
-		return &bpf_cgrp_storage_delete_proto;
 	case BPF_FUNC_current_task_under_cgroup:
 		return &bpf_current_task_under_cgroup_proto;
 #endif
@@ -1500,20 +1454,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_send_signal_proto;
 	case BPF_FUNC_send_signal_thread:
 		return &bpf_send_signal_thread_proto;
-	case BPF_FUNC_perf_event_read_value:
-		return &bpf_perf_event_read_value_proto;
-	case BPF_FUNC_ringbuf_output:
-		return &bpf_ringbuf_output_proto;
-	case BPF_FUNC_ringbuf_reserve:
-		return &bpf_ringbuf_reserve_proto;
-	case BPF_FUNC_ringbuf_submit:
-		return &bpf_ringbuf_submit_proto;
-	case BPF_FUNC_ringbuf_discard:
-		return &bpf_ringbuf_discard_proto;
-	case BPF_FUNC_ringbuf_query:
-		return &bpf_ringbuf_query_proto;
-	case BPF_FUNC_jiffies64:
-		return &bpf_jiffies64_proto;
 	case BPF_FUNC_get_task_stack:
 		return prog->sleepable ? &bpf_get_task_stack_sleepable_proto
 				       : &bpf_get_task_stack_proto;
@@ -1521,12 +1461,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_copy_from_user_proto;
 	case BPF_FUNC_copy_from_user_task:
 		return &bpf_copy_from_user_task_proto;
-	case BPF_FUNC_snprintf_btf:
-		return &bpf_snprintf_btf_proto;
-	case BPF_FUNC_per_cpu_ptr:
-		return &bpf_per_cpu_ptr_proto;
-	case BPF_FUNC_this_cpu_ptr:
-		return &bpf_this_cpu_ptr_proto;
 	case BPF_FUNC_task_storage_get:
 		if (bpf_prog_check_recur(prog))
 			return &bpf_task_storage_get_recur_proto;
@@ -1535,18 +1469,12 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		if (bpf_prog_check_recur(prog))
 			return &bpf_task_storage_delete_recur_proto;
 		return &bpf_task_storage_delete_proto;
-	case BPF_FUNC_for_each_map_elem:
-		return &bpf_for_each_map_elem_proto;
-	case BPF_FUNC_snprintf:
-		return &bpf_snprintf_proto;
 	case BPF_FUNC_get_func_ip:
 		return &bpf_get_func_ip_proto_tracing;
 	case BPF_FUNC_get_branch_snapshot:
 		return &bpf_get_branch_snapshot_proto;
 	case BPF_FUNC_find_vma:
 		return &bpf_find_vma_proto;
-	case BPF_FUNC_trace_vprintk:
-		return bpf_get_trace_vprintk_proto();
 	default:
 		break;
 	}
-- 
2.43.0
Re: [PATCH bpf-next v4] bpf: streamline allowed helpers between tracing and base sets
Posted by Andrii Nakryiko 7 months, 4 weeks ago 
On Wed, Apr 23, 2025 at 12:33 AM Feng Yang <yangfeng59949@163.com> wrote:
>
> From: Feng Yang <yangfeng@kylinos.cn>
>
> Many conditional checks in switch-case are redundant
> with bpf_base_func_proto and should be removed.
>
> Regarding the permission checks bpf_base_func_proto:
> The permission checks in bpf_prog_load (as outlined below)
> ensure that the trace has both CAP_BPF and CAP_PERFMON capabilities,
> thus enabling the use of corresponding prototypes
> in bpf_base_func_proto without adverse effects.
> bpf_prog_load
>         ......
>         bpf_cap = bpf_token_capable(token, CAP_BPF);
>         ......
>         if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
>             type != BPF_PROG_TYPE_CGROUP_SKB &&
>             !bpf_cap)
>                 goto put_token;
>         ......
>         if (is_perfmon_prog_type(type) && !bpf_token_capable(token, CAP_PERFMON))
>                 goto put_token;
>         ......
>
> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> Acked-by: Song Liu <song@kernel.org>
> ---

LGTM, applied to bpf-next, thanks. See comments on remaining helpers below.

> Changes in v4:
> - Only modify patch description information.
> - At present, bpf_tracing_func_proto still has the following ID:
> - BPF_FUNC_get_current_uid_gid
> - BPF_FUNC_get_current_comm

I don't see why these two cannot be used in any program, after all, we
have bpf_get_current_task(), these are in the same family.

> - BPF_FUNC_get_smp_processor_id

Based on another thread, I think it's some filter programs that have
to use raw variant of it, right? All other should use non-raw
implementation. So I think the right next step would be to make sure
that bpf_base_func_proto returns non-raw implementation, and only
those few program types that are exceptions should use raw ones?

> - BPF_FUNC_perf_event_read

should be fine to use anywhere (and actually can be useful for
networking programs to measure its own packet processing overhead or
something like that). Checking implementation I don't see any
limitations, it's just PERF_EVENT_ARRAY map access

> - BPF_FUNC_probe_read
> - BPF_FUNC_probe_read_str

generic tracing helpers, should be OK to be used anywhere with
CAP_PERFMON capabilities

> - BPF_FUNC_current_task_under_cgroup

same as above current_comm, if there is CGROUP_ARRAY, this should be
fine (though I don't know, there might be cgroup-specific
restrictions, not sure)

> - BPF_FUNC_send_signal
> - BPF_FUNC_send_signal_thread

fine to do from NMI, so should be fine to do anywhere (with
CAP_PERFMON, presumably)

> - BPF_FUNC_get_task_stack

seems fine (again, if it works under NMI and doesn't use any
context-dependent things, should be fine for any program type)

> - BPF_FUNC_copy_from_user
> - BPF_FUNC_copy_from_user_task

same as probe_read/probe_read_str (but only for sleepable)

> - BPF_FUNC_task_storage_get
> - BPF_FUNC_task_storage_delete

this is designed to work anywhere, so yeah, why not?

> - BPF_FUNC_get_func_ip

nope, very context dependent, definitely not generic (and just doesn't
make sense for most program types)

> - BPF_FUNC_get_branch_snapshot

NMI-enabled and not context-dependent, good to be used anywhere

> - BPF_FUNC_find_vma

non-sleepable, but other than that doesn't really make any assumptions
about program type, should be fine everywhere (NMI-safe, I believe?)

> - BPF_FUNC_probe_write_user

it's just like probe_read_user, CAP_PERFMON, so we can enable it
anywhere for completeness, but I'm not sure if that is a good idea...

> - I'm not sure which ones can be used by all programs, as Zvi Effron said(https://lore.kernel.org/all/CAC1LvL2SOKojrXPx92J46fFEi3T9TAWb3mC1XKtYzwU=pzTEbQ@mail.gmail.com/)
> - get_smp_processor_id also be retained(https://lore.kernel.org/all/CAADnVQ+WYLfoR1W6AsCJF6fNKEUgfxANXP01EQCJh1=99ZpoNw@mail.gmail.com/)

yep, I saw the discussion, that's fine

>
> - Link to v3: https://lore.kernel.org/all/20250410070258.276759-1-yangfeng59949@163.com/
>
> Changes in v3:
> - Only modify patch description information.
> - Link to v2: https://lore.kernel.org/all/20250408071151.229329-1-yangfeng59949@163.com/
>
> Changes in v2:
> - Only modify patch description information.
> - Link to v1: https://lore.kernel.org/all/20250320032258.116156-1-yangfeng59949@163.com/
> ---
>  kernel/trace/bpf_trace.c | 72 ----------------------------------------
>  1 file changed, 72 deletions(-)
>

[...]

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.