binfmt_elf.c: use get_random_int() to fix entropy depleting
Subsystems (2)
PATCH v4 1/1 binfmt_elf.c: use get_random_int() to fix entropy depleting
Hello,
The problems have been fixed in this version as per Kees's comments for v3.
Hi Kees,
Would you please ACK this patch if you think it is ok except the strength of these various RNGs you are concerned?
Changes:
--------v4->v3:
- s/random_stack_user()/get_atrandom_bytes()/
- Move this function to ahead of its use to avoid the predeclaration.
v3->v2:
- Tweak code comments of random_stack_user().
- Remove redundant bits mask and shift upon the random variable.
v2->v1:
- Fix random copy to check up buffer length that are not 4-byte multiples.
v3 can be found at:
http://www.spinics.net/lists/linux-fsdevel/msg59597.html
v2 can be found at:
http://www.spinics.net/lists/linux-fsdevel/msg59418.html
v1 can be found at:
http://www.spinics.net/lists/linux-fsdevel/msg59128.html
Thanks,
-Jeff
Entropy is quickly depleted under normal operations like ls(1), cat(1), etc... between 2.6.30 to current mainline, for instance:
$ cat /proc/sys/kernel/random/entropy_avail 3428 $ cat /proc/sys/kernel/random/entropy_avail 2911 $cat /proc/sys/kernel/random/entropy_avail 2620
We observed this problem has been occurring since 2.6.30 with fs/binfmt_elf.c: create_elf_tables()->get_random_bytes(), introduced by f06295b44c296c8f ("ELF: implement AT_RANDOM for glibc PRNG seeding").
/*
* Generate 16 random bytes for userspace PRNG seeding. */get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));
The patch introduces a wrapper around get_random_int() which has lower overhead than calling get_random_bytes() directly.
With this patch applied:
$ cat /proc/sys/kernel/random/entropy_avail 2731 $ cat /proc/sys/kernel/random/entropy_avail 2802 $ cat /proc/sys/kernel/random/entropy_avail 2878
Analyzed by John Sobecki.
--- fs/binfmt_elf.c | 21 ++++++++++++++++++++- 1 files changed, 20 insertions(+), 1 deletions(-)
fs/binfmt_elf.c +20 -1
@@ -139,6 +139,25 @@ static int padzero(unsigned long elf_bss)#define ELF_BASE_PLATFORM NULL#endif +/* +* Use get_random_int() to implement AT_RANDOM while avoiding depletion +* of the entropy pool. +*/ +static void get_atrandom_bytes (unsigned char *buf,size_t nbytes) +{ +unsigned char *p = buf; + +while (nbytes) { +unsigned int random_variable; +size_t chunk =min (nbytes,sizeof (random_variable)); + + random_variable =get_random_int (); +memcpy (p, &random_variable, chunk); + p += chunk; + nbytes -= chunk; + } +} +static int create_elf_tables (struct linux_binprm *bprm,struct elfhdr *exec,unsigned long load_addr,unsigned long interp_load_addr)
@@ -200,7 +219,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,/* * Generate 16 random bytes for userspace PRNG seeding. */ -get_random_bytes (k_rand_bytes,sizeof (k_rand_bytes)); +get_atrandom_bytes (k_rand_bytes,sizeof (k_rand_bytes)); u_rand_bytes = (elf_addr_t __user *)STACK_ALLOC (p,sizeof (k_rand_bytes));if (__copy_to_user (u_rand_bytes, k_rand_bytes,sizeof (k_rand_bytes)))
Signature · 1 line
1.7.4.1
Quoted 66 lines · Jeff Liu
Hello,
The problems have been fixed in this version as per Kees's comments for v3.
Hi Kees,
Would you please ACK this patch if you think it is ok except the strength of these various RNGs you are concerned?Changes:
--------v4->v3:
- s/random_stack_user()/get_atrandom_bytes()/
- Move this function to ahead of its use to avoid the predeclaration.
v3->v2:
- Tweak code comments ofrandom_stack_user().
- Remove redundant bits mask and shift upon the random variable.
v2->v1:
- Fix random copy to check up buffer length that are not 4-byte multiples.v3 can be found at:
http://www.spinics.net/lists/linux-fsdevel/msg59597.html
v2 can be found at:
http://www.spinics.net/lists/linux-fsdevel/msg59418.html
v1 can be found at:
http://www.spinics.net/lists/linux-fsdevel/msg59128.htmlThanks,
-JeffEntropy is quickly depleted under normal operations like ls(1), cat(1), etc... between 2.6.30 to current mainline, for instance:
$ cat /proc/sys/kernel/random/entropy_avail 3428 $ cat /proc/sys/kernel/random/entropy_avail 2911 $cat /proc/sys/kernel/random/entropy_avail 2620We observed this problem has been occurring since 2.6.30 with
fs/binfmt_elf.c: create_elf_tables()->get_random_bytes(), introduced byf06295b44c296c8f("ELF: implementAT_RANDOMfor glibc PRNG seeding")./*
* Generate 16 random bytes for userspace PRNG seeding. */get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));The patch introduces a wrapper around
get_random_int()which has lower overhead than callingget_random_bytes()directly.With this patch applied:
$ cat /proc/sys/kernel/random/entropy_avail 2731 $ cat /proc/sys/kernel/random/entropy_avail 2802 $ cat /proc/sys/kernel/random/entropy_avail 2878Analyzed by John Sobecki.
Quoted 13 lines
Signed-off-by Jie Liu <[email protected]>Cc Andrew Morton <[email protected]>Cc Al Viro <[email protected]>Cc Andreas Dilger <[email protected]>Cc Alan Cox <[email protected]>Cc Arnd Bergmann <[email protected]>Cc John Sobecki <[email protected]>Cc James Morris <[email protected]>Cc Jakub Jelinek <[email protected]>Cc Ted Ts'o <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Kees Cook <[email protected]>Cc Ulrich Drepper <[email protected]>
-Kees
Signature · 2 lines
Kees Cook Chrome OS Security
Hi Stephan,
As per your previous comments for this fix, you have promised another approach which is promising to avoid entropy starvation, I got this info from the following thread:
[PATCH] avoid entropy starvation due to stack protection
https://lkml.org/lkml/2012/12/14/267
My current fix has been merged into Andrew's tree(marked in "stuck" state) for a long time, and it also works well in our internal specific kernel, I'd like to know if there is any update from you, so that we can move it along for mainline. :)
Thanks,
-Jeff
Andrew Morton
Acked-by Kees CookQuoted 10 lines · On Thu, 07 Nov 2013 12:14:17 +0800 Jeff Liu <[email protected]> wrote:
Hi Stephan,
As per your previous comments for this fix, you have promised another approach which is promising to avoid entropy starvation, I got this info from the following thread:
[PATCH] avoid entropy starvation due to stack protection
https://lkml.org/lkml/2012/12/14/267My current fix has been merged into Andrew's
tree(marked in "stuck" state)for a long time, and it also works well in our internal specific kernel, I'd like to know if there is any update from you, so that we can move it along for mainline. :)
This:
From: Jeff Liu <[email protected]>
Subject: binfmt_elf.c: use get_random_int() to fix entropy depleting
Entropy is quickly depleted under normal operations like ls(1), cat(1), etc... between 2.6.30 to current mainline, for instance:
$ cat /proc/sys/kernel/random/entropy_avail 3428 $ cat /proc/sys/kernel/random/entropy_avail 2911 $cat /proc/sys/kernel/random/entropy_avail 2620
We observed this problem has been occurring since 2.6.30 with fs/binfmt_elf.c: create_elf_tables()->get_random_bytes(), introduced by f06295b44c296c8f ("ELF: implement AT_RANDOM for glibc PRNG seeding").
/*
* Generate 16 random bytes for userspace PRNG seeding. */get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));
The patch introduces a wrapper around get_random_int() which has lower overhead than calling get_random_bytes() directly.
With this patch applied:
$ cat /proc/sys/kernel/random/entropy_avail 2731 $ cat /proc/sys/kernel/random/entropy_avail 2802 $ cat /proc/sys/kernel/random/entropy_avail 2878
Analyzed by John Sobecki.
This has been applied on a specific Oracle kernel and has been running on the customer's production environment (the original bug reporter) for several months; it has worked fine until now.
---
fs/binfmt_elf.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
fs/binfmt_elf.c renamed from fs/binfmt_elf.c~binfmt_elfc-use-get_random_int-to-fix-entropy-depleting +20 -1
@@ -140,6 +140,25 @@ static int padzero(unsigned long elf_bss#define ELF_BASE_PLATFORM NULL#endif +/* +* Use get_random_int() to implement AT_RANDOM while avoiding depletion +* of the entropy pool. +*/ +static void get_atrandom_bytes (unsigned char *buf,size_t nbytes) +{ +unsigned char *p = buf; + +while (nbytes) { +unsigned int random_variable; +size_t chunk =min (nbytes,sizeof (random_variable)); + + random_variable =get_random_int (); +memcpy (p, &random_variable, chunk); + p += chunk; + nbytes -= chunk; + } +} +static int create_elf_tables (struct linux_binprm *bprm,struct elfhdr *exec,unsigned long load_addr,unsigned long interp_load_addr)
@@ -201,7 +220,7 @@ create_elf_tables(struct linux_binprm *b/* * Generate 16 random bytes for userspace PRNG seeding. */ -get_random_bytes (k_rand_bytes,sizeof (k_rand_bytes)); +get_atrandom_bytes (k_rand_bytes,sizeof (k_rand_bytes)); u_rand_bytes = (elf_addr_t __user *)STACK_ALLOC (p,sizeof (k_rand_bytes));if (__copy_to_user (u_rand_bytes, k_rand_bytes,sizeof (k_rand_bytes)))
_
Am Donnerstag, 7. November 2013, 12:14:17 schrieb Jeff Liu:
Hi Jeff,
Quoted 7 lines
Hi Stephan,
As per your previous comments for this fix, you have promised another approach which is promising to avoid entropy starvation, I got this info from the following thread: [PATCH] avoid entropy starvation due to stack protection
https://lkml.org/lkml/2012/12/14/267
There are several solutions:
- Ted is trying to prevent a constant reseeding of the nonblocking_pool from the input_pool with a set of patches. I am unsure whether these patches find their way into the kernel. With those patches, we can happily keep get_random_bytes without too much strain on the input_pool entropy -- i.e. drop the conversion to get_random_int.
- The begin of the email thread contains a patch that adds a new pool which I called the kernel_pool that is just just for kernel internal purposes. With Teds proposed changes to nonblocking_pool, nonblocking_pool would behave almost like my kernel_pool and thus my kernel_pool patch would not be needed.
- Lastly I am trying to add a new seed source to random.c and kernel crypto API which could also be used as a stand-alone noise source. That proposed noise source would effectively alleviate a lot of entropy problems. The discussion for inclusion is raging at
http://lkml.org/lkml/2013/10/11/582. Ted is having concerns and we are in a discussion to address those.
Quoted 8 lines
My current fix has been merged into Andrew's
tree(marked in "stuck" state)for a long time, and it also works well in our internal specific kernel, I'd like to know if there is any update from you, so that we can move it along for mainline. :)Thanks,
-Jeff
Ciao
Stephan
Thanks for your prompt response!
Quoted 12 lines · Stephan Mueller
Am Donnerstag, 7. November 2013, 12:14:17 schrieb Jeff Liu:
Hi Jeff,
Quoted 7 lines
Hi Stephan,
As per your previous comments for this fix, you have promised another approach which is promising to avoid entropy starvation, I got this info from the following thread: [PATCH] avoid entropy starvation due to stack protection
https://lkml.org/lkml/2012/12/14/267
Quoted 7 lines
There are several solutions:
- Ted is trying to prevent a constant reseeding of the
nonblocking_poolfrom theinput_poolwith a set of patches. I am unsure whether these patches find their way into the kernel. With those patches, we can happily keepget_random_byteswithout too much strain on theinput_poolentropy -- i.e. drop the conversion toget_random_int.
Yup, that's would be great if we can solve this problem without that.
Quoted 7 lines
- The begin of the email thread contains a patch that adds a new pool which I called the
kernel_poolthat is just just for kernel internal purposes. With Teds proposed changes tononblocking_pool,nonblocking_poolwould behave almost like mykernel_pooland thus mykernel_poolpatch would not be needed.
Quoted 6 lines
- Lastly I am trying to add a new seed source to
random.cand kernel crypto API which could also be used as a stand-alone noise source. That proposed noise source would effectively alleviate a lot of entropy problems. The discussion for inclusion is raging at
http://lkml.org/lkml/2013/10/11/582. Ted is having concerns and we are in a discussion to address those.
I spent a few hours reading through the thread through some contents are beyond my understanding. Looks the proposed approach has already been widely tested on various platforms, but there still have concerns like the random numbers which are generated via the jitter "entropy collector" probably can not be convinced to be more secure.
Thanks for your efforts and I'll keep a close eye on it's progress.
Regards,
-Jeff