On Tue, 2019-05-28 at 14:04 -0700, Song Liu wrote:
> > if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
> > type != BPF_PROG_TYPE_CGROUP_SKB &&
>
> You should extend this if () statement instead of adding another
> if () below.
Reworking the if-statement is possible but the result is something like:
if ((type != BPF_PROG_TYPE_SOCKET_FILTER &&
type != BPF_PROG_TYPE_CGROUP_SKB &&
!capable(CAP_SYS_ADMIN)) &&
!((type == BPF_PROG_TYPE_SCHED_CLS ||
type == BPF_PROG_TYPE_XDP) &&
capable(CAP_NET_ADMIN)))
return -EPERM;
This is not really readable and I do prefer an easy to verify code
when it comes to security, so how about the following version:
Signed-off-by: Andreas Steinmetz <ast@xxxxxxxx>
--- a/kernel/bpf/syscall.c 2019-05-28 18:00:40.472841432 +0200
+++ b/kernel/bpf/syscall.c 2019-06-05 12:34:48.197107612 +0200
@@ -1559,10 +1559,18 @@ static int bpf_prog_load(union bpf_attr
if (attr->insn_cnt == 0 || attr->insn_cnt > BPF_MAXINSNS)
return -E2BIG;
- if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
- type != BPF_PROG_TYPE_CGROUP_SKB &&
- !capable(CAP_SYS_ADMIN))
- return -EPERM;
+ switch (type) {
+ case BPF_PROG_TYPE_SOCKET_FILTER:
+ case BPF_PROG_TYPE_CGROUP_SKB:
+ break;
+ case BPF_PROG_TYPE_SCHED_CLS:
+ case BPF_PROG_TYPE_XDP:
+ if (capable(CAP_NET_ADMIN))
+ break;
+ default:
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ }
bpf_prog_load_fixup_attach_type(attr);
if (bpf_prog_load_check_attach_type(type, attr->expected_attach_type))
