On Mon, May 15, 2023 at 9:42 AM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Thu, May 11, 2023 at 9:21 AM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Tue, May 2, 2023 at 4:09 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > > > @@ -18878,7 +18882,12 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3 > > > env->prog = *prog; > > > env->ops = bpf_verifier_ops[env->prog->type]; > > > env->fd_array = make_bpfptr(attr->fd_array, uattr.is_kernel); > > > - is_priv = bpf_capable(); > > > + > > > + env->allow_ptr_leaks = bpf_allow_ptr_leaks(*prog); > > > + env->allow_uninit_stack = bpf_allow_uninit_stack(*prog); > > > + env->bypass_spec_v1 = bpf_bypass_spec_v1(*prog); > > > + env->bypass_spec_v4 = bpf_bypass_spec_v4(*prog); > > > + env->bpf_capable = is_priv = (*prog)->aux->bpf_capable; > > > > Just remembered that moving all CAP* checks early > > (before they actually needed) > > might be problematic. > > See > > https://lore.kernel.org/all/20230511142535.732324-10-cgzones@xxxxxxxxxxxxxx/ > > > > This patch set is reducing the number of cap* checks which is > > a good thing from audit pov, but it calls them early before the cap > > is actually needed and that part is misleading for audit. > > I'm afraid we cannot do one big switch for all map types after bpf_capable. > > The bpf_capable for maps needs to be done on demand. > > For progs we should also do it on demand too. > > socket_filter and cg_skb should proceed without cap* checks. > > Ok, fair enough. With BPF token approach this shouldn't be a big deal anyways. > > Does patch #5 ("bpf: drop unnecessary bpf_capable() check in > BPF_MAP_FREEZE command") look good? I think it makes sense to land > either way, given any other map-related operation isn't privileged > once user has map FD. I'll send it separately. Yeah. I think the whole cleanup makes sense, just need to make sure we do on-demand cap checks. I hope the patches won't change much.
