bpf: split state from prandom_u32() and consolidate {c, e}BPF prngs
authorDaniel Borkmann <daniel@iogearbox.net>
Wed, 7 Oct 2015 23:20:39 +0000 (01:20 +0200)
committerDavid S. Miller <davem@davemloft.net>
Thu, 8 Oct 2015 12:26:39 +0000 (05:26 -0700)
While recently arguing on a seccomp discussion that raw prandom_u32()
access shouldn't be exposed to unpriviledged user space, I forgot the
fact that SKF_AD_RANDOM extension actually already does it for some time
in cBPF via commit 4cd3675ebf74 ("filter: added BPF random opcode").

Since prandom_u32() is being used in a lot of critical networking code,
lets be more conservative and split their states. Furthermore, consolidate
eBPF and cBPF prandom handlers to use the new internal PRNG. For eBPF,
bpf_get_prandom_u32() was only accessible for priviledged users, but
should that change one day, we also don't want to leak raw sequences
through things like eBPF maps.

One thought was also to have own per bpf_prog states, but due to ABI
reasons this is not easily possible, i.e. the program code currently
cannot access bpf_prog itself, and copying the rnd_state to/from the
stack scratch space whenever a program uses the prng seems not really
worth the trouble and seems too hacky. If needed, taus113 could in such
cases be implemented within eBPF using a map entry to keep the state
space, or get_random_bytes() could become a second helper in cases where
performance would not be critical.

Both sides can trigger a one-time late init via prandom_init_once() on
the shared state. Performance-wise, there should even be a tiny gain
as bpf_user_rnd_u32() saves one function call. The PRNG needs to live
inside the BPF core since kernels could have a NET-less config as well.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
Cc: Chema Gonzalez <chema@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/linux/bpf.h
kernel/bpf/core.c
kernel/bpf/helpers.c
kernel/bpf/syscall.c
net/core/filter.c

index c915a6b54570cddd07aad894230a53f89cc39eae..3697ad5638996e086e79e4560f6042ca3d35b4de 100644 (file)
@@ -200,4 +200,8 @@ extern const struct bpf_func_proto bpf_get_current_comm_proto;
 extern const struct bpf_func_proto bpf_skb_vlan_push_proto;
 extern const struct bpf_func_proto bpf_skb_vlan_pop_proto;
 
+/* Shared helpers among cBPF and eBPF. */
+void bpf_user_rnd_init_once(void);
+u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
+
 #endif /* _LINUX_BPF_H */
index c8855c2a7a480dfd2908b02ec63569e1c448f9fa..80864712d2c405d077644604831685d1a94529eb 100644 (file)
@@ -731,6 +731,32 @@ void bpf_prog_free(struct bpf_prog *fp)
 }
 EXPORT_SYMBOL_GPL(bpf_prog_free);
 
+/* RNG for unpriviledged user space with separated state from prandom_u32(). */
+static DEFINE_PER_CPU(struct rnd_state, bpf_user_rnd_state);
+
+void bpf_user_rnd_init_once(void)
+{
+       prandom_init_once(&bpf_user_rnd_state);
+}
+
+u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+       /* Should someone ever have the rather unwise idea to use some
+        * of the registers passed into this function, then note that
+        * this function is called from native eBPF and classic-to-eBPF
+        * transformations. Register assignments from both sides are
+        * different, f.e. classic always sets fn(ctx, A, X) here.
+        */
+       struct rnd_state *state;
+       u32 res;
+
+       state = &get_cpu_var(bpf_user_rnd_state);
+       res = prandom_u32_state(state);
+       put_cpu_var(state);
+
+       return res;
+}
+
 /* Weak definitions of helper functions in case we don't have bpf syscall. */
 const struct bpf_func_proto bpf_map_lookup_elem_proto __weak;
 const struct bpf_func_proto bpf_map_update_elem_proto __weak;
index 1447ec09421ebc4905c922f4f4fa2b65b52b474a..4504ca66118da0c0735bdb56d9204ba6cb79994b 100644 (file)
@@ -93,13 +93,8 @@ const struct bpf_func_proto bpf_map_delete_elem_proto = {
        .arg2_type      = ARG_PTR_TO_MAP_KEY,
 };
 
-static u64 bpf_get_prandom_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
-{
-       return prandom_u32();
-}
-
 const struct bpf_func_proto bpf_get_prandom_u32_proto = {
-       .func           = bpf_get_prandom_u32,
+       .func           = bpf_user_rnd_u32,
        .gpl_only       = false,
        .ret_type       = RET_INTEGER,
 };
index 5f35f420c12fad1612fa0fda2932b41781da67f5..c868cafbc00c659939a6ec8ac9b808db5f8cd1f6 100644 (file)
@@ -404,6 +404,8 @@ static void fixup_bpf_calls(struct bpf_prog *prog)
 
                        if (insn->imm == BPF_FUNC_get_route_realm)
                                prog->dst_needed = 1;
+                       if (insn->imm == BPF_FUNC_get_prandom_u32)
+                               bpf_user_rnd_init_once();
                        if (insn->imm == BPF_FUNC_tail_call) {
                                /* mark bpf_tail_call as different opcode
                                 * to avoid conditional branch in
index 8f4603c712cd08ff9899cb7766c7bfed9184ad7d..342e6c8fc415bb6a565b62d22363161208aae8e0 100644 (file)
@@ -149,12 +149,6 @@ static u64 __get_raw_cpu_id(u64 ctx, u64 a, u64 x, u64 r4, u64 r5)
        return raw_smp_processor_id();
 }
 
-/* note that this only generates 32-bit random numbers */
-static u64 __get_random_u32(u64 ctx, u64 a, u64 x, u64 r4, u64 r5)
-{
-       return prandom_u32();
-}
-
 static u32 convert_skb_access(int skb_field, int dst_reg, int src_reg,
                              struct bpf_insn *insn_buf)
 {
@@ -313,7 +307,8 @@ static bool convert_bpf_extensions(struct sock_filter *fp,
                        *insn = BPF_EMIT_CALL(__get_raw_cpu_id);
                        break;
                case SKF_AD_OFF + SKF_AD_RANDOM:
-                       *insn = BPF_EMIT_CALL(__get_random_u32);
+                       *insn = BPF_EMIT_CALL(bpf_user_rnd_u32);
+                       bpf_user_rnd_init_once();
                        break;
                }
                break;