Messages in this thread | | | From | Song Liu <> | Date | Mon, 7 Mar 2022 10:23:36 -0800 | Subject | Re: [PATCH bpf-next v2 01/28] bpf: add new is_sys_admin_prog_type() helper |
| |
On Sat, Mar 5, 2022 at 2:07 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > On Sat, Mar 5, 2022 at 12:12 AM Song Liu <song@kernel.org> wrote: > > > > On Fri, Mar 4, 2022 at 9:30 AM Benjamin Tissoires > > <benjamin.tissoires@redhat.com> wrote: > > > > > > LIRC_MODE2 does not really need net_admin capability, but only sys_admin. > > > > > > Extract a new helper for it, it will be also used for the HID bpf > > > implementation. > > > > > > Cc: Sean Young <sean@mess.org> > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > > > --- > > > > > > new in v2 > > > --- > > > kernel/bpf/syscall.c | 14 +++++++++++++- > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > > index db402ebc5570..cc570891322b 100644 > > > --- a/kernel/bpf/syscall.c > > > +++ b/kernel/bpf/syscall.c > > > @@ -2165,7 +2165,6 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type) > > > case BPF_PROG_TYPE_LWT_SEG6LOCAL: > > > case BPF_PROG_TYPE_SK_SKB: > > > case BPF_PROG_TYPE_SK_MSG: > > > - case BPF_PROG_TYPE_LIRC_MODE2: > > > case BPF_PROG_TYPE_FLOW_DISSECTOR: > > > case BPF_PROG_TYPE_CGROUP_DEVICE: > > > case BPF_PROG_TYPE_CGROUP_SOCK: > > > @@ -2202,6 +2201,17 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type) > > > } > > > } > > > > > > +static bool is_sys_admin_prog_type(enum bpf_prog_type prog_type) > > > +{ > > > + switch (prog_type) { > > > + case BPF_PROG_TYPE_LIRC_MODE2: > > > + case BPF_PROG_TYPE_EXT: /* extends any prog */ > > > + return true; > > > + default: > > > + return false; > > > + } > > > +} > > > > I am not sure whether we should do this. This is a behavior change, that may > > break some user space. Also, BPF_PROG_TYPE_EXT is checked in > > is_perfmon_prog_type(), and this change will make that case useless. > > Sure, I can drop it from v3 and make this function appear for HID only. > > Regarding BPF_PROG_TYPE_EXT, it was already in both > is_net_admin_prog_type() and is_perfmon_prog_type(), so I duplicated > it here, but I agree, given that it's already in the first function > there, CPA_SYS_ADMIN is already checked.
I think with current code, a user with CAP_BPF, CAP_NET_ADMIN, and CAP_PERFMON (but not CAP_SYS_ADMIN) can load programs of type BPF_PROG_TYPE_EXT. But after the patch, the same user will not be able to do it. Did I misread it? It is not a common case though.
| |