Hi Andreas,
On 10/27/2012 02:52 AM, Andreas Dilger wrote:
On 2012-10-25, at 1:53, Jeff Liu <jeff.liu@xxxxxxxxxx> wrote:
We observed an issue regarding entropy quickly depleting under any normal I/O operations
like ls(1), cat(1),etc... for instance:
$ date; cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:24:37 CST 2012
3264
$ date; cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:24:40 CST 2012
2791
$ date; cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:24:42 CST 2012
2581
$ date; cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:24:43 CST 2012
2122
According to John's analysis, it started to happen with 2.6.30 with:
fs/binfmt_elf.c: create_elf_tables()->get_random_bytes() was introduced:
/*
* Generate 16 random bytes for userspace PRNG seeding.
*/
get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));
Here is proposal patch to replace get_random_bytes() with a wrapper function get_random_int()
which has low overhead to generate randoms, it looks stupid but works:
$ date;cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:31:32 CST 2012
2546
$ date;cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:31:33 CST 2012
2558
$ date;cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:31:34 CST 2012
2572
$ date;cat /proc/sys/kernel/random/entropy_avail
Thu Oct 25 15:31:36 CST 2012
2614
Also, I have a question about whether stack randomization tunable parameter could be considered in
this point or not, i.e,
If the user disabled the stack randomization via "kernel.randomize_va_space=0" or /proc/...
Does it sounds make sense if just copying the k_rand_bytes[] back to user space with current uninitialized
stack stuff rather than filling it with really strong random bytes, something like:
/*
* Generate 16 random bytes for userspace PRNG seeding if randomize is required.
*/
if (current->flags & PF_RANDOMIZE)
get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));
Above fix also works although Glibc->elf_loader need a random bytes array for stack guarding, which means that
the user want to take the risk by disabling stack randomize.
Any comments are appreciated!
-Jeff
Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
Analyzed-by: John Sobecki <john.sobecki@xxxxxxxxxx>
CC: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
CC: Arnd Bergmann <arnn@xxxxxxxx>
CC: James Morris <james.l.morris@xxxxxxxxxx>
CC: Ted Ts'o <tytso@xxxxxxx>
CC: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
fs/binfmt_elf.c | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fbd9f60..4fc92d5 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 *random_bytes, size_t nr);
/*
* 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));
This is passing the parameter in units of 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,19 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
#endif
}
+static void randomize_stack_user(unsigned char *random_bytes, size_t nr)
+{
+ unsigned int random_variable;
+ size_t i;
+
+ for (i = 0; i < nr; i += sizeof(random_variable)) {
+ random_variable = get_random_int() & STACK_RND_MASK;
+ random_variable <= PAGE_SHIFT;
+ memcpy(&random_bytes[i], &random_variable,
+ sizeof(random_variable));
This is filling in the the buffer in 4-byte increments. Are there any callers that will have buffers that are not 4-byte multiples? It would probably be safer to change the memcpy() to have a shorter length if the buffer is short.
Thanks for your comments!
I also thought to make randomize_stack_user() safer as your mentioned at
first, but I don't find other callers related to it in binfmt_elf.c.
Anyway, the current logic is not generic to deal with various array
length, I'll fix it.
Also, I even wonder if we can add above loops inside the
create_elf_table() directly if there has no other callers?
Thanks,
-Jeff
Cheers, Andreas
+ }
+}
+
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
