On 27-Mär 08:27, Stephen Smalley wrote: > On 3/26/20 8:24 PM, James Morris wrote: > > On Thu, 26 Mar 2020, KP Singh wrote: > > > > > +int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog, > > > + const struct bpf_prog *prog) > > > +{ > > > + /* Only CAP_MAC_ADMIN users are allowed to make changes to LSM hooks > > > + */ > > > + if (!capable(CAP_MAC_ADMIN)) > > > + return -EPERM; > > > + > > > > Stephen, can you confirm that your concerns around this are resolved > > (IIRC, by SELinux implementing a bpf_prog callback) ? > > I guess the only residual concern I have is that CAP_MAC_ADMIN means > something different to SELinux (ability to get/set file security contexts > unknown to the currently loaded policy), so leaving the CAP_MAC_ADMIN check > here (versus calling a new security hook here and checking CAP_MAC_ADMIN in > the implementation of that hook for the modules that want that) conflates > two very different things. Prior to this patch, there are no users of > CAP_MAC_ADMIN outside of individual security modules; it is only checked in > module-specific logic within apparmor, safesetid, selinux, and smack, so the > meaning was module-specific. As we had discussed, We do have a security hook as well: https://lore.kernel.org/bpf/20200324180652.GA11855@xxxxxxxxxxxx/ The bpf_prog hook which can check for BPF_PROG_TYPE_LSM and implement module specific logic for LSM programs. I thougt that was okay? Kees was in favor of keeping the CAP_MAC_ADMIN check here: https://lore.kernel.org/bpf/202003241133.16C02BE5B@keescook If you feel strongly and Kees agrees, we can remove the CAP_MAC_ADMIN check here, but given that we already have a security hook that meets the requirements, we probably don't need another one. - KP > > > >