On Wed, Apr 12, 2023 at 9:53 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Wed, Apr 12, 2023 at 12:33 AM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
> >
> > Add new LSM hook, bpf_btf_load_security, that allows custom LSM security
> > policies controlling BTF data loading permissions (BPF_BTF_LOAD command
> > of bpf() syscall) granularly and precisely.
> >
> > This complements bpf_map_create_security LSM hook added earlier and
> > follow the same semantics: 0 means perform standard kernel capabilities-based
> > checks, negative error rejects BTF object load, while positive one skips
> > CAP_BPF check and allows BTF data object creation.
> >
> > With this hook, together with bpf_map_create_security, we now can also allow
> > trusted unprivileged process to create BPF maps that require BTF, which
> > we take advantaged in the next patch to improve the coverage of added
> > BPF selftest.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > ---
> > include/linux/lsm_hook_defs.h | 1 +
> > include/linux/lsm_hooks.h | 13 +++++++++++++
> > include/linux/security.h | 6 ++++++
> > kernel/bpf/bpf_lsm.c | 1 +
> > kernel/bpf/syscall.c | 10 ++++++++++
> > security/security.c | 4 ++++
> > 6 files changed, 35 insertions(+)
>
> ...
>
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 42d8473237ab..bbf70bddc770 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -4449,12 +4449,22 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
> >
> > static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size)
> > {
> > + int err;
> > +
> > if (CHECK_ATTR(BPF_BTF_LOAD))
> > return -EINVAL;
> >
> > + /* security checks */
> > + err = security_bpf_btf_load(attr);
> > + if (err < 0)
> > + return err;
> > + if (err > 0)
> > + goto skip_priv_checks;
> > +
> > if (!bpf_capable())
> > return -EPERM;
> >
> > +skip_priv_checks:
> > return btf_new_fd(attr, uattr, uattr_size);
> > }
>
> Beyond the objection I brought up in the patchset cover letter, I
> believe the work of the security_bpf_btf_load() hook presented here
> could be done by the existing security_bpf() LSM hook. If you believe
> that not to be the case, please let me know.
security_bpf() could prevent BTF object loading only, but
security_bpf_btf_load() can *also* allow *trusted* (according to LSM
policy) unprivileged process to proceed. So it doesn't seem like they
are interchangeable.
>
> --
> paul-moore.com
