On Wed, May 3, 2023 at 11:28 AM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote: > > On 05/02, Andrii Nakryiko wrote: > > Make each bpf() syscall command a bit more self-contained, making it > > easier to further enhance it. We move sysctl_unprivileged_bpf_disabled > > handling down to map_create() and bpf_prog_load(), two special commands > > in this regard. > > > > Also swap the order of checks, calling bpf_capable() only if > > sysctl_unprivileged_bpf_disabled is true, avoiding unnecessary audit > > messages. > > > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > --- > > kernel/bpf/syscall.c | 37 ++++++++++++++++++++++--------------- > > 1 file changed, 22 insertions(+), 15 deletions(-) > > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 14f39c1e573e..d5009fafe0f4 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -1132,6 +1132,17 @@ static int map_create(union bpf_attr *attr) > > int f_flags; > > int err; > > [..] > > > + /* Intent here is for unprivileged_bpf_disabled to block key object > > + * creation commands for unprivileged users; other actions depend > > + * of fd availability and access to bpffs, so are dependent on > > + * object creation success. Capabilities are later verified for > > + * operations such as load and map create, so even with unprivileged > > + * BPF disabled, capability checks are still carried out for these > > + * and other operations. > > + */ > > + if (sysctl_unprivileged_bpf_disabled && !bpf_capable()) > > + return -EPERM; > > + > > Does it make sense to have something like unpriv_bpf_capable() to avoid > the copy-paste? It's a simple if condition used in exactly two places, so I had preference to keep permissions checks grouped together in the same block of code in respective MAP_CREATE and PROG_LOAD handlers. I can factor it out into a function, but I see little value in that, tbh.
