Thread (50 messages) flat view 50 messages, 5 authors, 2017-04-20

Re: [PATCH net-next v6 01/11] bpf: Add eBPF program subtype and is_valid_subtype() verifier

From: Kees Cook <hidden>
Date: 2017-04-18 21:48:05
Also in: linux-security-module, lkml, netdev

On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün [off-list ref] wrote:
quoted hunk ↗ jump to hunk
The goal of the program subtype is to be able to have different static
fine-grained verifications for a unique program type.

The struct bpf_verifier_ops gets a new optional function:
is_valid_subtype(). This new verifier is called at the beginning of the
eBPF program verification to check if the (optional) program subtype is
valid.

For now, only Landlock eBPF programs are using a program subtype (see
next commit) but this could be used by other program types in the future.

Changes since v5:
* use a prog_subtype pointer and make it future-proof
* add subtype test
* constify bpf_load_program()'s subtype argument
* cleanup subtype initialization
* rebase

Changes since v4:
* replace the "status" field with "version" (more generic)
* replace the "access" field with "ability" (less confusing)

Changes since v3:
* remove the "origin" field
* add an "option" field
* cleanup comments

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Link: https://lkml.kernel.org/r/20160827205559.GA43880@ast-mbp.thefacebook.com
---
[...]
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index c35ebfe6d84d..3d07b10ade5e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -843,6 +879,26 @@ static int bpf_prog_load(union bpf_attr *attr)
        if (err < 0)
                goto free_prog;

+       /* copy eBPF program subtype from user space */
+       if (attr->prog_subtype) {
+               __u32 size;
+
+               size = check_user_buf((void __user *)attr->prog_subtype,
+                                     attr->prog_subtype_size,
+                                     sizeof(prog->subtype));
+               if (size < 0) {
+                       err = size;
+                       goto free_prog;
+               }
+               /* prog->subtype is __GFP_ZERO */
+               if (copy_from_user(&prog->subtype,
+                                  u64_to_user_ptr(attr->prog_subtype), size)
+                                  != 0)
It might be worth adding a comment here about how the ToCToU of the
check-then-copy doesn't matter in this case, since it's just a
future-proofing of bits, etc.

-Kees

-- 
Kees Cook
Pixel Security
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help