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

[Jeff Liu <jeff.liu@oracle.com>]

Hi Andreas,

On 10/27/2012 02:52 AM, Andreas Dilger wrote:
> 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.
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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html