Skip to contentSkip to conversation

BPF/random32 updates

PATCH In review Daniel Borkmann 4 participants 13 messages Oct 7, 2015 – Oct 8, 2015

Subsystems (7)
lore
Share & tools
lore

Daniel Borkmann

author maintainer
#0 lore raw

v2 cover BPF/random32 updates

BPF update to split the prandom state apart, and to move the *once helpers to the core. For details, please see individual patches. Given the changes and since it's in the tree for quite some time, net-next is a better choice in our opinion.

v1 -> v2:
- Make DO_ONCE() type-safe, remove the kvec helper. Credits go to Alexei Starovoitov for the __VA_ARGS__ hint, thanks!
- Add a comment to the DO_ONCE() helper as suggested by Alexei.
- Rework prandom_init_once() helper to the new API.
- Keep Alexei's Acked-by on the last patch.

Thanks!

Daniel Borkmann (3):
random32: add prandom_seed_full_state helper
random32: add prandom_init_once helper for own rngs
bpf: split state from prandom_u32() and consolidate {c,e}BPF prngs

Hannes Frederic Sowa (2):
net: move net_get_random_once to lib
once: make helper generic for calling function once

 include/linux/bpf.h    |  4 ++++
 include/linux/net.h    | 21 ++++-------------
 include/linux/once.h   | 57 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/random.h |  6 +++++
 kernel/bpf/core.c      | 26 +++++++++++++++++++++
 kernel/bpf/helpers.c   |  7 +-----
 kernel/bpf/syscall.c   |  2 ++
 lib/Makefile           |  3 ++-
 lib/once.c             | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/random32.c         | 37 +++++++++++++++++-------------
 net/core/filter.c      |  9 ++------
 net/core/utils.c       | 49 ---------------------------------------
 12 files changed, 187 insertions(+), 96 deletions(-)
 create mode 100644 include/linux/once.h
 create mode 100644 lib/once.c
Signature · 1 line
1.9.3
author maintainer
#1 lore raw

PATCH v2 1/5 net: move net_get_random_once to lib

From: Hannes Frederic Sowa <[email protected]>

There's no good reason why users outside of networking should not be using this facility, f.e. for initializing their seeds.

Therefore, make it accessible from there as get_random_once().

---
 include/linux/net.h  | 21 ++++----------------
 include/linux/once.h | 24 +++++++++++++++++++++++
 lib/Makefile         |  3 ++-
 lib/once.c           | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/core/utils.c     | 49 -----------------------------------------------
 5 files changed, 84 insertions(+), 67 deletions(-)
 create mode 100644 include/linux/once.h
 create mode 100644 lib/once.c
include/linux/net.h +4 -17
index 049d4b0..70ac5e2 100644
@@ -24,7 +24,8 @@
 #include <linux/fcntl.h>	/* For O_CLOEXEC and O_NONBLOCK */
 #include <linux/kmemcheck.h>
 #include <linux/rcupdate.h>
-#include <linux/jump_label.h>
+#include <linux/once.h>
+
 #include <uapi/linux/net.h>
 
 struct poll_table_struct;
@@ -250,22 +251,8 @@ do {								\
 	} while (0)
 #endif
 
-bool __net_get_random_once(void *buf, int nbytes, bool *done,
-			   struct static_key *done_key);
-
-#define net_get_random_once(buf, nbytes)				\
-	({								\
-		bool ___ret = false;					\
-		static bool ___done = false;				\
-		static struct static_key ___once_key =			\
-			STATIC_KEY_INIT_TRUE;				\
-		if (static_key_true(&___once_key))			\
-			___ret = __net_get_random_once(buf,		\
-						       nbytes,		\
-						       &___done,	\
-						       &___once_key);	\
-		___ret;							\
-	})
+#define net_get_random_once(buf, nbytes)			\
+	get_random_once((buf), (nbytes))
 
 int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec *vec,
 		   size_t num, size_t len);
include/linux/once.h new file +24 -0
new file mode 100644
index 0000000..2a83b53
@@ -0,0 +1,24 @@
+#ifndef _LINUX_ONCE_H
+#define _LINUX_ONCE_H
+
+#include <linux/types.h>
+#include <linux/jump_label.h>
+
+bool __get_random_once(void *buf, int nbytes, bool *done,
+		       struct static_key *once_key);
+
+#define get_random_once(buf, nbytes)					\
+	({								\
+		bool ___ret = false;					\
+		static bool ___done = false;				\
+		static struct static_key ___once_key =			\
+			STATIC_KEY_INIT_TRUE;				\
+		if (static_key_true(&___once_key))			\
+			___ret = __get_random_once((buf),		\
+						   (nbytes),		\
+						   &___done,		\
+						   &___once_key);	\
+		___ret;							\
+	})
+
+#endif /* _LINUX_ONCE_H */
lib/Makefile +2 -1
index 13a7c6a..8de3b01 100644
@@ -26,7 +26,8 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
 	 bust_spinlocks.o kasprintf.o bitmap.o scatterlist.o \
 	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \
 	 bsearch.o find_bit.o llist.o memweight.o kfifo.o \
-	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
+	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o \
+	 once.o
 obj-y += string_helpers.o
 obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
 obj-y += hexdump.o
lib/once.c new file +54 -0

54 lines not on this page · raw message

net/core/utils.c +0 -49

52 lines not on this page · raw message

Signature · 1 line
1.9.3
Acked-by on v2 1/5
#2 lore raw

Re: [PATCH net-next v2 1/5] net: move net_get_random_once to lib

Quoted 9 lines · Daniel Borkmann

From: Hannes Frederic Sowa<[email protected]>

There's no good reason why users outside of networking should not be using this facility, f.e. for initializing their seeds.

Therefore, make it accessible from there as get_random_once().

#3 lore raw

Re: [PATCH net-next v2 1/5] net: move net_get_random_once to lib

In reply to Daniel Borkmann
config: mips-ip27_defconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross #…

Hi Hannes,

[auto build test WARNING on net-next/master -- if it's inappropriate base, please ignore]

config: mips-ip27_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips

All warnings (new ones prefixed by >>):

mips-linux-gnu-ld: lib/lockref.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/lockref.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bcd.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bcd.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/div64.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/div64.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/sort.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/sort.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/parser.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/parser.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/halfmd4.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/halfmd4.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/debug_locks.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/debug_locks.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/random32.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/random32.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bust_spinlocks.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bust_spinlocks.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/kasprintf.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/kasprintf.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bitmap.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bitmap.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/scatterlist.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/scatterlist.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/gcd.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/gcd.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/lcm.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/lcm.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/list_sort.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/list_sort.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/uuid.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/uuid.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/flex_array.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/flex_array.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/iov_iter.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/iov_iter.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/clz_ctz.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/clz_ctz.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bsearch.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bsearch.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/find_bit.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/find_bit.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/llist.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/llist.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/memweight.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/memweight.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/kfifo.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/kfifo.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/percpu-refcount.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/percpu-refcount.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/percpu_ida.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/percpu_ida.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/rhashtable.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/rhashtable.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/reciprocal_div.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/reciprocal_div.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags

>> mips-linux-gnu-ld: lib/once.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
>> mips-linux-gnu-ld: lib/once.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags

mips-linux-gnu-ld: lib/string_helpers.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/string_helpers.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/hexdump.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/hexdump.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/kstrtox.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/kstrtox.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/pci_iomap.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/pci_iomap.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/iomap_copy.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/iomap_copy.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/devres.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/devres.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/check_signature.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/check_signature.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/hweight.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/hweight.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/assoc_array.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/assoc_array.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bitrev.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/bitrev.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/rational.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/rational.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/crc16.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/crc16.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/crc32.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/crc32.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/percpu_counter.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/percpu_counter.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/syscall.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/syscall.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/nlattr.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/nlattr.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/cpu_rmap.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/cpu_rmap.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/dynamic_queue_limits.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/dynamic_queue_limits.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/net_utils.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/net_utils.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author maintainer
#4 lore raw

Re: [PATCH net-next v2 1/5] net: move net_get_random_once to lib

Quoted 15 lines · kbuild test robot

Hi Hannes,

[auto build test WARNING on net-next/master -- if it's inappropriate base, please ignore]

config: mips-ip27_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips

All warnings (new ones prefixed by >>):

mips-linux-gnu-ld: lib/lockref.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/lockref.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags

[...]

Quoted 4 lines

mips-linux-gnu-ld: lib/reciprocal_div.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/reciprocal_div.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags

Quoted 2 lines
Quoted 2 lines

mips-linux-gnu-ld: lib/once.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/once.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags

Looks like a known issue on MIPS given the warning is not new also on other object files. Here was what was discussed recently on this subject on a refactoring patch from David Ahern:

https://patchwork.ozlabs.org/patch/525102/

Thanks!

Quoted 2 lines

mips-linux-gnu-ld: lib/string_helpers.o: warning: Inconsistent ISA between e_flags and .MIPS.abiflags
mips-linux-gnu-ld: lib/string_helpers.o: warning: Inconsistent ISA extensions between e_flags and .MIPS.abiflags

[...]

author maintainer
#5 lore raw

PATCH v2 2/5 once: make helper generic for calling functions once

In reply to Daniel Borkmann

From: Hannes Frederic Sowa <[email protected]>

Make the get_random_once() helper generic enough, so that functions in general would only be called once, where one user of this is then net_get_random_once().

The only implementation specific call is to get_random_bytes(), all the rest of this *_once() facility would be duplicated among different subsystems otherwise. The new DO_ONCE() helper will be used by prandom() later on, but might also be useful for other scenarios/subsystems as well where a one-time initialization in often-called, possibly fast path code could occur.

---
 include/linux/once.h | 61 ++++++++++++++++++++++++++++++++++++++++------------
 lib/once.c           | 50 ++++++++++++++++++++++++------------------
 2 files changed, 76 insertions(+), 35 deletions(-)
include/linux/once.h +47 -14
index 2a83b53..285f12c 100644
@@ -4,21 +4,54 @@
 #include <linux/types.h>
 #include <linux/jump_label.h>
 
-bool __get_random_once(void *buf, int nbytes, bool *done,
-		       struct static_key *once_key);
+bool __do_once_start(bool *done, unsigned long *flags);
+void __do_once_done(bool *done, struct static_key *once_key,
+		    unsigned long *flags);
 
-#define get_random_once(buf, nbytes)					\
-	({								\
-		bool ___ret = false;					\
-		static bool ___done = false;				\
-		static struct static_key ___once_key =			\
-			STATIC_KEY_INIT_TRUE;				\
-		if (static_key_true(&___once_key))			\
-			___ret = __get_random_once((buf),		\
-						   (nbytes),		\
-						   &___done,		\
-						   &___once_key);	\
-		___ret;							\
+/* Call a function exactly once. The idea of DO_ONCE() is to perform
+ * a function call such as initialization of random seeds, etc, only
+ * once, where DO_ONCE() can live in the fast-path. After @func has
+ * been called with the passed arguments, the static key will patch
+ * out the condition into a nop. DO_ONCE() guarantees type safety of
+ * arguments!
+ *
+ * Not that the following is not equivalent ...
+ *
+ *   DO_ONCE(func, arg);
+ *   DO_ONCE(func, arg);
+ *
+ * ... to this version:
+ *
+ *   void foo(void)
+ *   {
+ *     DO_ONCE(func, arg);
+ *   }
+ *
+ *   foo();
+ *   foo();
+ *
+ * In case the one-time invocation could be triggered from multiple
+ * places, then a common helper function must be defined, so that only
+ * a single static key will be placed there!
+ */
+#define DO_ONCE(func, ...)						     \
+	({								     \
+		bool ___ret = false;					     \
+		static bool ___done = false;				     \
+		static struct static_key ___once_key = STATIC_KEY_INIT_TRUE; \
+		if (static_key_true(&___once_key)) {			     \
+			unsigned long ___flags;				     \
+			___ret = __do_once_start(&___done, &___flags);	     \
+			if (unlikely(___ret)) {				     \
+				func(__VA_ARGS__);			     \
+				__do_once_done(&___done, &___once_key,	     \
+					       &___flags);		     \
+			}						     \
+		}							     \
+		___ret;							     \
 	})
 
+#define get_random_once(buf, nbytes)					     \
+	DO_ONCE(get_random_bytes, (buf), (nbytes))
+
 #endif /* _LINUX_ONCE_H */
lib/once.c +29 -21

81 lines not on this page · raw message

Signature · 1 line
1.9.3
Acked-by on v2 2/5
#6 lore raw

Re: [PATCH net-next v2 2/5] once: make helper generic for calling functions once

Quoted 13 lines · Daniel Borkmann

From: Hannes Frederic Sowa<[email protected]>

Make the get_random_once() helper generic enough, so that functions in general would only be called once, where one user of this is then net_get_random_once().

The only implementation specific call is to get_random_bytes(), all the rest of this *_once() facility would be duplicated among different subsystems otherwise. The new DO_ONCE() helper will be used by prandom() later on, but might also be useful for other scenarios/subsystems as well where a one-time initialization in often-called, possibly fast path code could occur.

Quoted 2 lines

nice! you even added sparse tags.

author maintainer
#7 lore raw

PATCH v2 3/5 random32: add prandom_seed_full_state helper

In reply to Daniel Borkmann

Factor out the full reseed handling code that populates the state through get_random_bytes() and runs prandom_warmup(). The resulting prandom_seed_full_state() will be used later on in more than the current __prandom_reseed() user. Fix also two minor whitespace issues along the way.

---
 lib/random32.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)
lib/random32.c +21 -16
index 0bee183..36c09fb 100644
@@ -181,7 +181,7 @@ void prandom_seed(u32 entropy)
 	 * No locking on the CPUs, but then somewhat random results are, well,
 	 * expected.
 	 */
-	for_each_possible_cpu (i) {
+	for_each_possible_cpu(i) {
 		struct rnd_state *state = &per_cpu(net_rand_state, i);
 
 		state->s1 = __seed(state->s1 ^ entropy, 2U);
@@ -201,7 +201,7 @@ static int __init prandom_init(void)
 	prandom_state_selftest();
 
 	for_each_possible_cpu(i) {
-		struct rnd_state *state = &per_cpu(net_rand_state,i);
+		struct rnd_state *state = &per_cpu(net_rand_state, i);
 		u32 weak_seed = (i + jiffies) ^ random_get_entropy();
 
 		prandom_seed_early(state, weak_seed, true);
@@ -238,13 +238,30 @@ static void __init __prandom_start_seed_timer(void)
 	add_timer(&seed_timer);
 }
 
+static void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state)
+{
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct rnd_state *state = per_cpu_ptr(pcpu_state, i);
+		u32 seeds[4];
+
+		get_random_bytes(&seeds, sizeof(seeds));
+		state->s1 = __seed(seeds[0],   2U);
+		state->s2 = __seed(seeds[1],   8U);
+		state->s3 = __seed(seeds[2],  16U);
+		state->s4 = __seed(seeds[3], 128U);
+
+		prandom_warmup(state);
+	}
+}
+
 /*
  *	Generate better values after random number generator
  *	is fully initialized.
  */
 static void __prandom_reseed(bool late)
 {
-	int i;
 	unsigned long flags;
 	static bool latch = false;
 	static DEFINE_SPINLOCK(lock);
@@ -266,19 +283,7 @@ static void __prandom_reseed(bool late)
 		goto out;
 
 	latch = true;
-
-	for_each_possible_cpu(i) {
-		struct rnd_state *state = &per_cpu(net_rand_state,i);
-		u32 seeds[4];
-
-		get_random_bytes(&seeds, sizeof(seeds));
-		state->s1 = __seed(seeds[0],   2U);
-		state->s2 = __seed(seeds[1],   8U);
-		state->s3 = __seed(seeds[2],  16U);
-		state->s4 = __seed(seeds[3], 128U);
-
-		prandom_warmup(state);
-	}
+	prandom_seed_full_state(&net_rand_state);
 out:
 	spin_unlock_irqrestore(&lock, flags);
 }
Signature · 1 line
1.9.3
Acked-by on v2 3/5
#8 lore raw

Re: [PATCH net-next v2 3/5] random32: add prandom_seed_full_state helper

Quoted 8 lines · Daniel Borkmann

Factor out the full reseed handling code that populates the state through get_random_bytes() and runs prandom_warmup(). The resulting prandom_seed_full_state() will be used later on in more than the current __prandom_reseed() user. Fix also two minor whitespace issues along the way.

author maintainer
#9 lore raw

PATCH v2 4/5 random32: add prandom_init_once helper for own rngs

In reply to Daniel Borkmann

Add a prandom_init_once() facility that works on the rnd_state, so that users that are keeping their own state independent from prandom_u32() can initialize their taus113 per cpu states.

The motivation here is similar to net_get_random_once(): initialize the state as late as possible in the hope that enough entropy has been collected for the seeding. prandom_init_once() makes use of the recently introduced prandom_seed_full_state() helper and is generic enough so that it could also be used on fast-paths due to the DO_ONCE().

---
 include/linux/random.h | 6 ++++++
 lib/random32.c         | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)
include/linux/random.h +6 -0
index e651874..a75840c 100644
@@ -7,6 +7,8 @@
 #define _LINUX_RANDOM_H
 
 #include <linux/list.h>
+#include <linux/once.h>
+
 #include <uapi/linux/random.h>
 
 struct random_ready_callback {
@@ -45,6 +47,10 @@ struct rnd_state {
 
 u32 prandom_u32_state(struct rnd_state *state);
 void prandom_bytes_state(struct rnd_state *state, void *buf, size_t nbytes);
+void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state);
+
+#define prandom_init_once(pcpu_state)			\
+	DO_ONCE(prandom_seed_full_state, (pcpu_state))
 
 /**
  * prandom_u32_max - returns a pseudo-random number in interval [0, ep_ro)
lib/random32.c +1 -1
index 36c09fb..1211191 100644
@@ -238,7 +238,7 @@ static void __init __prandom_start_seed_timer(void)
 	add_timer(&seed_timer);
 }
 
-static void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state)
+void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state)
 {
 	int i;
 
Signature · 1 line
1.9.3
Acked-by on v2 4/5
#10 lore raw

Re: [PATCH net-next v2 4/5] random32: add prandom_init_once helper for own rngs

Quoted 4 lines · Daniel Borkmann

Add a prandom_init_once() facility that works on the rnd_state, so that users that are keeping their own state independent from prandom_u32() can initialize their taus113 per cpu states.

Quoted 8 lines

The motivation here is similar to net_get_random_once(): initialize the state as late as possible in the hope that enough entropy has been collected for the seeding. prandom_init_once() makes use of the recently introduced prandom_seed_full_state() helper and is generic enough so that it could also be used on fast-paths due to the DO_ONCE().

author maintainer
#11 lore raw

PATCH v2 5/5 bpf: split state from prandom_u32() and consolidate {c,e}BPF prngs

In reply to Daniel Borkmann

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.

---
 include/linux/bpf.h  |  4 ++++
 kernel/bpf/core.c    | 26 ++++++++++++++++++++++++++
 kernel/bpf/helpers.c |  7 +------
 kernel/bpf/syscall.c |  2 ++
 net/core/filter.c    |  9 ++-------
 5 files changed, 35 insertions(+), 13 deletions(-)
include/linux/bpf.h +4 -0
index c915a6b..3697ad5 100644
@@ -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 */
kernel/bpf/core.c +26 -0
index c8855c2..8086471 100644
@@ -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;
kernel/bpf/helpers.c +1 -6
index 1447ec0..4504ca6 100644
@@ -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,
 };
kernel/bpf/syscall.c +2 -0
index 5f35f42..c868caf 100644
@@ -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
net/core/filter.c +2 -7
index 8f4603c..342e6c8 100644
@@ -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;
Signature · 1 line
1.9.3
maintainer
#12 lore raw
In reply to Daniel Borkmann

From: Daniel Borkmann <[email protected]>
Date: Thu, 8 Oct 2015 01:20:34 +0200

Quoted 11 lines

BPF update to split the prandom state apart, and to move the *once helpers to the core. For details, please see individual patches. Given the changes and since it's in the tree for quite some time, net-next is a better choice in our opinion.

v1 -> v2:
- Make DO_ONCE() type-safe, remove the kvec helper. Credits go to Alexei Starovoitov for the __VA_ARGS__ hint, thanks!
- Add a comment to the DO_ONCE() helper as suggested by Alexei.
- Rework prandom_init_once() helper to the new API.
- Keep Alexei's Acked-by on the last patch.

Series applied, thanks everyone.