* [PATCH V3] binfmt_elf.c: Introduce a wrapper of get_random_int() to fix entropy depleting
@ 2012-11-07 5:27 Jeff Liu
2012-11-07 5:52 ` Jeff Liu
2012-11-07 6:11 ` [RESEND PATCH V3] binfmt_elf.c: use " Jeff Liu
0 siblings, 2 replies; 8+ messages in thread
From: Jeff Liu @ 2012-11-07 5:27 UTC (permalink / raw)
To: LKML
Cc: Andrew Morton, Kees Cook, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
Hello,
We have observed entropy quickly depleting under normal I/O between 2.6.30 to upstream, 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
It has been occurred with fs/binfmt_elf.c: create_elf_tables()->get_random_bytes()
was introduced began at 2.6.30.
/*
* Generate 16 random bytes for userspace PRNG seeding.
*/
get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));
This proposal patch is trying to introduce a wrapper of get_random_int() which has lower overhead
than calling get_random_bytes() directly.
Entropy increased 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
v3->v2:
-------
Remove redundant bits mask and shift upon the random variable according to Kees's review.
v2->v1:
-------
Fix random copy to check up buffer length that are not 4-byte multiples according to Andreas's comments.
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
Many thanks to Andreas, Andrew as well as Kees for reviewing the patch of past.
-Jeff
Signed-off-by: Jie Liu <jeff.liu@oracle.com>
Cc: John Sobecki <john.sobecki@oracle.com>
CC: Andrew Morton <akpm@linux-foundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andreas Dilger <aedilger@gmail.com>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Arnd Bergmann <arnn@arndb.de>
Cc: James Morris <james.l.morris@oracle.com>
Cc: Ted Ts'o <tytso@mit.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Ulrich Drepper <drepper@redhat.com>
---
fs/binfmt_elf.c | 24 +++++++++++++++++++++++-
1 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fbd9f60..9c36e50 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -48,6 +48,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
static int load_elf_library(struct file *);
static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
int, int, unsigned long);
+static void randomize_stack_user(unsigned char *buf, size_t nbytes);
/*
* If we don't support core dumping, then supply a NULL so we
@@ -200,7 +201,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));
+ randomize_stack_user(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)))
@@ -558,6 +559,27 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
#endif
}
+/*
+ * A wrapper of get_random_int() to generate random bytes which has lower
+ * overhead than calling get_random_bytes() directly.
+ * create_elf_tables() call this function to generate 16 random bytes for
+ * userspace PRNG seeding.
+ */
+static void randomize_stack_user(unsigned char *buf, size_t nbytes)
+{
+ unsigned char *p = buf;
+
+ while (nbytes) {
+ unsigned int random_variable;
+ size_t chunk = min(nbytes, sizeof(unsigned int));
+
+ random_variable = get_random_int();
+ memcpy(p, &random_variable, chunk);
+ p += chunk;
+ nbytes -= chunk;
+ }
+}
+
static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
struct file *interpreter = NULL; /* to shut gcc up */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH V3] binfmt_elf.c: Introduce a wrapper of get_random_int() to fix entropy depleting
2012-11-07 5:27 [PATCH V3] binfmt_elf.c: Introduce a wrapper of get_random_int() to fix entropy depleting Jeff Liu
@ 2012-11-07 5:52 ` Jeff Liu
2012-11-07 6:11 ` [RESEND PATCH V3] binfmt_elf.c: use " Jeff Liu
1 sibling, 0 replies; 8+ messages in thread
From: Jeff Liu @ 2012-11-07 5:52 UTC (permalink / raw)
To: LKML
Cc: Andrew Morton, Kees Cook, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
Please ignore this patch since I forgot revising the comments of this
new wrapper according to Andrew's, will re-send it a little while, sorry
for the noise!
-Jeff
On 11/07/2012 01:27 PM, Jeff Liu wrote:
> Hello,
>
> We have observed entropy quickly depleting under normal I/O between 2.6.30 to upstream, 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
>
> It has been occurred with fs/binfmt_elf.c: create_elf_tables()->get_random_bytes()
> was introduced began at 2.6.30.
> /*
> * Generate 16 random bytes for userspace PRNG seeding.
> */
> get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));
>
> This proposal patch is trying to introduce a wrapper of get_random_int() which has lower overhead
> than calling get_random_bytes() directly.
>
> Entropy increased 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
>
>
> v3->v2:
> -------
> Remove redundant bits mask and shift upon the random variable according to Kees's review.
>
> v2->v1:
> -------
> Fix random copy to check up buffer length that are not 4-byte multiples according to Andreas's comments.
>
> 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
>
> Many thanks to Andreas, Andrew as well as Kees for reviewing the patch of past.
>
> -Jeff
>
>
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> Cc: John Sobecki <john.sobecki@oracle.com>
> CC: Andrew Morton <akpm@linux-foundation.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andreas Dilger <aedilger@gmail.com>
> Cc: Alan Cox <alan@linux.intel.com>
> Cc: Arnd Bergmann <arnn@arndb.de>
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: Ted Ts'o <tytso@mit.edu>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Jakub Jelinek <jakub@redhat.com>
> Cc: Ulrich Drepper <drepper@redhat.com>
>
> ---
> fs/binfmt_elf.c | 24 +++++++++++++++++++++++-
> 1 files changed, 23 insertions(+), 1 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index fbd9f60..9c36e50 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -48,6 +48,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
> static int load_elf_library(struct file *);
> static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
> int, int, unsigned long);
> +static void randomize_stack_user(unsigned char *buf, size_t nbytes);
>
> /*
> * If we don't support core dumping, then supply a NULL so we
> @@ -200,7 +201,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));
> + randomize_stack_user(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)))
> @@ -558,6 +559,27 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
> #endif
> }
>
> +/*
> + * A wrapper of get_random_int() to generate random bytes which has lower
> + * overhead than calling get_random_bytes() directly.
> + * create_elf_tables() call this function to generate 16 random bytes for
> + * userspace PRNG seeding.
> + */
> +static void randomize_stack_user(unsigned char *buf, size_t nbytes)
> +{
> + unsigned char *p = buf;
> +
> + while (nbytes) {
> + unsigned int random_variable;
> + size_t chunk = min(nbytes, sizeof(unsigned int));
> +
> + random_variable = get_random_int();
> + memcpy(p, &random_variable, chunk);
> + p += chunk;
> + nbytes -= chunk;
> + }
> +}
> +
> static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> {
> struct file *interpreter = NULL; /* to shut gcc up */
>
^ permalink raw reply [flat|nested] 8+ messages in thread* [RESEND PATCH V3] binfmt_elf.c: use get_random_int() to fix entropy depleting
2012-11-07 5:27 [PATCH V3] binfmt_elf.c: Introduce a wrapper of get_random_int() to fix entropy depleting Jeff Liu
2012-11-07 5:52 ` Jeff Liu
@ 2012-11-07 6:11 ` Jeff Liu
2012-11-07 6:21 ` Kees Cook
1 sibling, 1 reply; 8+ messages in thread
From: Jeff Liu @ 2012-11-07 6:11 UTC (permalink / raw)
To: LKML
Cc: Andrew Morton, Kees Cook, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
Hello,
This is the revised patch for fix entropy depleting.
Changes:
--------
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.
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
Many thanks to Andreas, Andrew as well as Kees for reviewing the patch of past!
-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.
Signed-off-by: Jie Liu <jeff.liu@oracle.com>
Cc: John Sobecki <john.sobecki@oracle.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andreas Dilger <aedilger@gmail.com>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Arnd Bergmann <arnn@arndb.de>
Cc: James Morris <james.l.morris@oracle.com>
Cc: Ted Ts'o <tytso@mit.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Ulrich Drepper <drepper@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/binfmt_elf.c | 22 +++++++++++++++++++++-
1 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fbd9f60..b6c59f6 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -48,6 +48,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
static int load_elf_library(struct file *);
static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
int, int, unsigned long);
+static void randomize_stack_user(unsigned char *buf, size_t nbytes);
/*
* If we don't support core dumping, then supply a NULL so we
@@ -200,7 +201,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));
+ randomize_stack_user(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)))
@@ -558,6 +559,25 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
#endif
}
+/*
+ * Use get_random_int() to implement AT_RANDOM while avoiding depletion
+ * of the entropy pool.
+ */
+static void randomize_stack_user(unsigned char *buf, size_t nbytes)
+{
+ unsigned char *p = buf;
+
+ while (nbytes) {
+ unsigned int random_variable;
+ size_t chunk = min(nbytes, sizeof(unsigned int));
+
+ random_variable = get_random_int();
+ memcpy(p, &random_variable, chunk);
+ p += chunk;
+ nbytes -= chunk;
+ }
+}
+
static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
struct file *interpreter = NULL; /* to shut gcc up */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [RESEND PATCH V3] binfmt_elf.c: use get_random_int() to fix entropy depleting
2012-11-07 6:11 ` [RESEND PATCH V3] binfmt_elf.c: use " Jeff Liu
@ 2012-11-07 6:21 ` Kees Cook
2012-11-07 7:02 ` Jeff Liu
0 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2012-11-07 6:21 UTC (permalink / raw)
To: Jeff Liu
Cc: LKML, Andrew Morton, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
On Tue, Nov 6, 2012 at 10:11 PM, Jeff Liu <jeff.liu@oracle.com> wrote:
> Hello,
>
> This is the revised patch for fix entropy depleting.
>
> Changes:
> --------
> 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.
>
> 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
>
> Many thanks to Andreas, Andrew as well as Kees for reviewing the patch of past!
> -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.
>
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> Cc: John Sobecki <john.sobecki@oracle.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andreas Dilger <aedilger@gmail.com>
> Cc: Alan Cox <alan@linux.intel.com>
> Cc: Arnd Bergmann <arnn@arndb.de>
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: Ted Ts'o <tytso@mit.edu>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Jakub Jelinek <jakub@redhat.com>
> Cc: Ulrich Drepper <drepper@redhat.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>
> ---
> fs/binfmt_elf.c | 22 +++++++++++++++++++++-
> 1 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index fbd9f60..b6c59f6 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -48,6 +48,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
> static int load_elf_library(struct file *);
> static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
> int, int, unsigned long);
> +static void randomize_stack_user(unsigned char *buf, size_t nbytes);
I think it would be easier to just move the function ahead of its use
to avoid the predeclaration.
>
> /*
> * If we don't support core dumping, then supply a NULL so we
> @@ -200,7 +201,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));
> + randomize_stack_user(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)))
> @@ -558,6 +559,25 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
> #endif
> }
>
> +/*
> + * Use get_random_int() to implement AT_RANDOM while avoiding depletion
> + * of the entropy pool.
> + */
> +static void randomize_stack_user(unsigned char *buf, size_t nbytes)
I think this name needs changing -- it has nothing to do with the
stack except that that's where it ends up in userspace. Perhaps
"get_atrandom_bytes"?
> +{
> + unsigned char *p = buf;
> +
> + while (nbytes) {
> + unsigned int random_variable;
> + size_t chunk = min(nbytes, sizeof(unsigned int));
> +
> + random_variable = get_random_int();
I still want to hear at least from Ted about this changes -- we would
be potentially increasing the predictability of these bytes...
> + memcpy(p, &random_variable, chunk);
> + p += chunk;
> + nbytes -= chunk;
> + }
> +}
> +
> static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> {
> struct file *interpreter = NULL; /* to shut gcc up */
> --
> 1.7.4.1
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RESEND PATCH V3] binfmt_elf.c: use get_random_int() to fix entropy depleting
2012-11-07 6:21 ` Kees Cook
@ 2012-11-07 7:02 ` Jeff Liu
2012-11-07 7:13 ` Kees Cook
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Liu @ 2012-11-07 7:02 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Andrew Morton, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
On 11/07/2012 02:21 PM, Kees Cook wrote:
> On Tue, Nov 6, 2012 at 10:11 PM, Jeff Liu <jeff.liu@oracle.com> wrote:
>> Hello,
>>
>> This is the revised patch for fix entropy depleting.
>>
>> Changes:
>> --------
>> 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.
>>
>> 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
>>
>> Many thanks to Andreas, Andrew as well as Kees for reviewing the patch of past!
>> -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.
>>
>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>> Cc: John Sobecki <john.sobecki@oracle.com>
>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>> Cc: Andreas Dilger <aedilger@gmail.com>
>> Cc: Alan Cox <alan@linux.intel.com>
>> Cc: Arnd Bergmann <arnn@arndb.de>
>> Cc: James Morris <james.l.morris@oracle.com>
>> Cc: Ted Ts'o <tytso@mit.edu>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Jakub Jelinek <jakub@redhat.com>
>> Cc: Ulrich Drepper <drepper@redhat.com>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>
>> ---
>> fs/binfmt_elf.c | 22 +++++++++++++++++++++-
>> 1 files changed, 21 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
>> index fbd9f60..b6c59f6 100644
>> --- a/fs/binfmt_elf.c
>> +++ b/fs/binfmt_elf.c
>> @@ -48,6 +48,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
>> static int load_elf_library(struct file *);
>> static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
>> int, int, unsigned long);
>> +static void randomize_stack_user(unsigned char *buf, size_t nbytes);
>
> I think it would be easier to just move the function ahead of its use
> to avoid the predeclaration.
Yes, it's better.
>
>>
>> /*
>> * If we don't support core dumping, then supply a NULL so we
>> @@ -200,7 +201,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));
>> + randomize_stack_user(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)))
>> @@ -558,6 +559,25 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
>> #endif
>> }
>>
>> +/*
>> + * Use get_random_int() to implement AT_RANDOM while avoiding depletion
>> + * of the entropy pool.
>> + */
>> +static void randomize_stack_user(unsigned char *buf, size_t nbytes)
>
> I think this name needs changing -- it has nothing to do with the
> stack except that that's where it ends up in userspace. Perhaps
> "get_atrandom_bytes"?
I racked my brains but can not think out a better name than yours. :)
>
>> +{
>> + unsigned char *p = buf;
>> +
>> + while (nbytes) {
>> + unsigned int random_variable;
>> + size_t chunk = min(nbytes, sizeof(unsigned int));
>> +
>> + random_variable = get_random_int();
>
> I still want to hear at least from Ted about this changes -- we would
> be potentially increasing the predictability of these bytes...
We would not increasing that if this routine would be used for AT_RANDOM
only(and if the array keeping aligned to 4 bytes).
Otherwise, it would be, so let's waiting for further feedbacks.
Thanks,
-Jeff
>
>> + memcpy(p, &random_variable, chunk);
>> + p += chunk;
>> + nbytes -= chunk;
>> + }
>> +}
>> +
>> static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
>> {
>> struct file *interpreter = NULL; /* to shut gcc up */
>> --
>> 1.7.4.1
>
> -Kees
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RESEND PATCH V3] binfmt_elf.c: use get_random_int() to fix entropy depleting
2012-11-07 7:02 ` Jeff Liu
@ 2012-11-07 7:13 ` Kees Cook
2012-11-14 21:09 ` Andrew Morton
0 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2012-11-07 7:13 UTC (permalink / raw)
To: Jeff Liu
Cc: LKML, Andrew Morton, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
On Tue, Nov 6, 2012 at 11:02 PM, Jeff Liu <jeff.liu@oracle.com> wrote:
> On 11/07/2012 02:21 PM, Kees Cook wrote:
>> I still want to hear at least from Ted about this changes -- we would
>> be potentially increasing the predictability of these bytes...
>
> We would not increasing that if this routine would be used for AT_RANDOM
> only(and if the array keeping aligned to 4 bytes).
> Otherwise, it would be, so let's waiting for further feedbacks.
get_random_int() comes from a different pool than get_random_bytes(),
IIUC. I'd like to hear some convincing reasoning as to why this change
doesn't compromise predictability. :)
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH V3] binfmt_elf.c: use get_random_int() to fix entropy depleting
2012-11-07 7:13 ` Kees Cook
@ 2012-11-14 21:09 ` Andrew Morton
2012-11-14 21:14 ` Kees Cook
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2012-11-14 21:09 UTC (permalink / raw)
To: Kees Cook
Cc: Jeff Liu, LKML, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
On Tue, 6 Nov 2012 23:13:54 -0800
Kees Cook <keescook@chromium.org> wrote:
> On Tue, Nov 6, 2012 at 11:02 PM, Jeff Liu <jeff.liu@oracle.com> wrote:
> > On 11/07/2012 02:21 PM, Kees Cook wrote:
> >> I still want to hear at least from Ted about this changes -- we would
> >> be potentially increasing the predictability of these bytes...
> >
> > We would not increasing that if this routine would be used for AT_RANDOM
> > only(and if the array keeping aligned to 4 bytes).
> > Otherwise, it would be, so let's waiting for further feedbacks.
>
> get_random_int() comes from a different pool than get_random_bytes(),
> IIUC. I'd like to hear some convincing reasoning as to why this change
> doesn't compromise predictability. :)
But the original "ELF: implement AT_RANDOM for glibc PRNG seeding"
compromised predictability. That's the whole point of this patch.
What was so important about that patch that justified gobbling down so
much of the system's entropy accumulation?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RESEND PATCH V3] binfmt_elf.c: use get_random_int() to fix entropy depleting
2012-11-14 21:09 ` Andrew Morton
@ 2012-11-14 21:14 ` Kees Cook
0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2012-11-14 21:14 UTC (permalink / raw)
To: Andrew Morton
Cc: Jeff Liu, LKML, Andreas Dilger, John Sobecki,
viro@zeniv.linux.org.uk, Alan Cox, arnd@arndb.de, James Morris,
Ted Ts'o, gregkh@linuxfoundation.org, jakub, drepper,
linux-fsdevel@vger.kernel.org
On Wed, Nov 14, 2012 at 1:09 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Tue, 6 Nov 2012 23:13:54 -0800
> Kees Cook <keescook@chromium.org> wrote:
>
>> On Tue, Nov 6, 2012 at 11:02 PM, Jeff Liu <jeff.liu@oracle.com> wrote:
>> > On 11/07/2012 02:21 PM, Kees Cook wrote:
>> >> I still want to hear at least from Ted about this changes -- we would
>> >> be potentially increasing the predictability of these bytes...
>> >
>> > We would not increasing that if this routine would be used for AT_RANDOM
>> > only(and if the array keeping aligned to 4 bytes).
>> > Otherwise, it would be, so let's waiting for further feedbacks.
>>
>> get_random_int() comes from a different pool than get_random_bytes(),
>> IIUC. I'd like to hear some convincing reasoning as to why this change
>> doesn't compromise predictability. :)
>
> But the original "ELF: implement AT_RANDOM for glibc PRNG seeding"
> compromised predictability. That's the whole point of this patch.
It doesn't compromise predictability. It just used entropy. The idea
was that userspace had an actual need for it.
> What was so important about that patch that justified gobbling down so
> much of the system's entropy accumulation?
That does seem to be the core question here. If Ted doesn't think this
patch is a problem, then I don't object. Mostly I just had questions
about the strength of these various RNGs.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-11-14 21:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-07 5:27 [PATCH V3] binfmt_elf.c: Introduce a wrapper of get_random_int() to fix entropy depleting Jeff Liu
2012-11-07 5:52 ` Jeff Liu
2012-11-07 6:11 ` [RESEND PATCH V3] binfmt_elf.c: use " Jeff Liu
2012-11-07 6:21 ` Kees Cook
2012-11-07 7:02 ` Jeff Liu
2012-11-07 7:13 ` Kees Cook
2012-11-14 21:09 ` Andrew Morton
2012-11-14 21:14 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).