On Sat, Apr 26, 2025 at 11:39 PM Feng Yang <yangfeng59949@xxxxxxx> wrote: > > From: Feng Yang <yangfeng@xxxxxxxxxx> > > if it works under NMI and doesn't use any context-dependent things, > should be fine for any program type. The detailed discussion is in [1]. > > [1] https://lore.kernel.org/all/CAEf4Bza6gK3dsrTosk6k3oZgtHesNDSrDd8sdeQ-GiS6oJixQg@xxxxxxxxxxxxxx/ > > Suggested-by: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> > Signed-off-by: Feng Yang <yangfeng@xxxxxxxxxx> > --- > Changes in v2: > - not expose compat probe read APIs to more program types. > - Remove the prog->sleepable check added for copy_from_user, > - or the summarization_freplace/might_sleep_with_might_sleep test will fail with the error "program of this type cannot use helper bpf_copy_from_user" > - Link to v1: https://lore.kernel.org/all/20250425080032.327477-1-yangfeng59949@xxxxxxx/ > --- > kernel/bpf/cgroup.c | 6 ------ > kernel/bpf/helpers.c | 38 +++++++++++++++++++++++++++++++++++++ > kernel/trace/bpf_trace.c | 41 ++++------------------------------------ > net/core/filter.c | 2 -- > 4 files changed, 42 insertions(+), 45 deletions(-) > > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c > index 84f58f3d028a..dbdad5f42761 100644 > --- a/kernel/bpf/cgroup.c > +++ b/kernel/bpf/cgroup.c > @@ -2607,16 +2607,10 @@ const struct bpf_func_proto * > cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > { > switch (func_id) { > - 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; > #ifdef CONFIG_CGROUP_NET_CLASSID > case BPF_FUNC_get_cgroup_classid: > return &bpf_get_cgroup_classid_curr_proto; > #endif this is the only one left, and again, it's just current-dependent, so I'd just move this into base set and got rid of cgroup_current_func_proto altogether (there are 5 callers, let's clean them up) > - 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 e3a2662f4e33..a01a2e55e17d 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -23,6 +23,7 @@ > #include <linux/btf_ids.h> > #include <linux/bpf_mem_alloc.h> > #include <linux/kasan.h> > +#include <linux/bpf_verifier.h> why do we need this include? [...] > @@ -2057,6 +2074,27 @@ bpf_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > return bpf_get_trace_vprintk_proto(); > case BPF_FUNC_perf_event_read_value: > return bpf_get_perf_event_read_value_proto(); > + case BPF_FUNC_perf_event_read: > + return &bpf_perf_event_read_proto; > + case BPF_FUNC_send_signal: > + return &bpf_send_signal_proto; > + case BPF_FUNC_send_signal_thread: > + return &bpf_send_signal_thread_proto; > + case BPF_FUNC_get_task_stack: > + return prog->sleepable ? &bpf_get_task_stack_sleepable_proto > + : &bpf_get_task_stack_proto; > + case BPF_FUNC_task_storage_get: > + if (bpf_prog_check_recur(prog)) > + return &bpf_task_storage_get_recur_proto; > + return &bpf_task_storage_get_proto; > + case BPF_FUNC_task_storage_delete: > + if (bpf_prog_check_recur(prog)) > + return &bpf_task_storage_delete_recur_proto; > + return &bpf_task_storage_delete_proto; task_storage_{get,delete} probably should be guarded just by CAP_BPF, no need for CAP_PERFMON, IMO. Can you please move them up a bit? Also, we should probably get rid of bpf_scx_get_func_proto() in kernel/sched/ext.c, given it only adds these two on top of the base set? But that's probably a separate patch against sched_ext tree? cc'ing Tejun pw-bot: cr > + case BPF_FUNC_get_branch_snapshot: > + return &bpf_get_branch_snapshot_proto; > + case BPF_FUNC_find_vma: > + return &bpf_find_vma_proto; > default: > return NULL; > } [...]
