Re: [PATCH] binfmt_elf.c: Introduce a wrapper of get_random_int() to fix entropy depleting

[Andreas Dilger <aedilger@gmail.com>]

On 2012-10-25, at 1:53, Jeff Liu <jeff.liu@oracle.com> 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@oracle.com>
> Analyzed-by: John Sobecki <john.sobecki@oracle.com>
> CC: Al Viro <viro@zeniv.linux.org.uk>
> 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>
> ---
> 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. 

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html