netdev - Re: [PATCH v5 bpf-next 2/3] bpf: implement CAP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 May 2020 11:29:44 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, bpf@...r.kernel.org,
        kernel-team@...com, linux-security-module@...r.kernel.org,
        acme@...hat.com, jamorris@...ux.microsoft.com, jannh@...gle.com,
        kpsingh@...gle.com
Subject: Re: [PATCH v5 bpf-next 2/3] bpf: implement CAP_BPF

On Tue, May 12, 2020 at 05:05:12PM +0200, Daniel Borkmann wrote:
> > -	env->allow_ptr_leaks = is_priv;
> > +	env->allow_ptr_leaks = perfmon_capable();
> > +	env->bpf_capable = bpf_capable();
> 
> Probably more of a detail, but it feels weird to tie perfmon_capable() into the BPF
> core and use it in various places there. I would rather make this a proper bpf_*
> prefixed helper and add a more descriptive name (what does it have to do with perf
> or monitoring directly?). For example, all the main functionality could be under
> `bpf_base_capable()` and everything with potential to leak pointers or mem to user
> space as `bpf_leak_capable()`. Then inside include/linux/capability.h this can still
> resolve under the hood to something like:
> 
> static inline bool bpf_base_capable(void)
> {
> 	return capable(CAP_BPF) || capable(CAP_SYS_ADMIN);
> }

I don't like the 'base' in the name, since 'base' implies common subset,
but it's not the case. Also 'base' implies that something else is additive,
but it's not the case either. The real base is unpriv. cap_bpf adds to it.
So bpf_capable() in capability.h is the most appropriate.
It also matches perfmon_capable() and other *_capable()

> static inline bool bpf_leak_capable(void)
> {
> 	return perfmon_capable();
> }

This is ok, but not in capability.h. I can put it into bpf_verifier.h