Re: [PATCH v9 3/5] x86/KASLR: Randomize virtual address separately

From: Ingo Molnar
Date: Fri Jun 17 2016 - 04:20:41 EST



* Kees Cook <keescook@xxxxxxxxxxxx> wrote:

> -unsigned char *choose_random_location(unsigned long input,
> - unsigned long input_size,
> - unsigned long output,
> - unsigned long output_size)
> +void choose_random_location(unsigned long input,
> + unsigned long input_size,
> + unsigned long *output,
> + unsigned long output_size,
> + unsigned long *virt_addr)
> {
> - unsigned long choice = output;
> unsigned long random_addr;
>
> + /* By default, keep output position unchanged. */
> + *virt_addr = *output;

So I applied this, after fixing a conflict with a recent hibernation related
change, but it would be nice to further clean up the types in this file, in
particular could we please propagate 'const' for all input-only pointers?

For example in the above function it would be obvious at a glance if it said
something like:

void choose_random_location(unsigned long input,
unsigned long input_size,
const unsigned long *output,
unsigned long output_size,
unsigned long *virt_addr)

when reading such a function prototype I can immediately tell: 'yeah, while it's
named "output", it's in fact a read-only input parameter - the _real_ output of
the function is 'virt_addr'.)

In addition to that it would also be useful to eliminate the 'virt_addr' parameter
altogether, and use an 'unsigned long' return value to set virt_addr in misc.c.

Ok?

Thanks,

Ingo