On 03/07/16 at 03:28pm, Kees Cook wrote: > On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <b...@redhat.com> wrote: > > From: Yinghai Lu <ying...@kernel.org> > > > > We are going to support kaslr with 64bit above 4G, and new random output > > could be anywhere. Array mem_avoid is used for kaslr to search new output > > address. Current code only track range that is after output+output_size. > > So we need to track all ranges instead of just after output+output_size. > > > > Current code has first entry which is extra bytes before input+input_size, > > and it is according to output_size. Other entries are for initrd, cmdline, > > and heap/stack for ZO running. > > > > At first, let's check the first entry that should be in the mem_avoid array. > > Now ZO sit end of the buffer always, we can find out where is text and > > data/bss etc of ZO. > > > > Since init_size >= run_size, and input+input_len >= output+output_len, > > here make several assumptions for better presentation by graph: > > - init_size > run_size > > - input+input_len > output+output_len > > - run_size > output_len > > I would like to see each of these assumptions justified. Why is > init_size > run_size, etc? > choose_kernel_location's "output_size" is calculated as max(run_size, > output_len), so run_size may not be > output_len...
Sure. I will add this case in next post. Thanks a lot. > > > > > 0 output input input+input_len > > output+init_size > > | | | | > > | > > |-----|-------------------|--------|------------------|----|------------|----------| > > | | | > > output+init_size-ZO_INIT_SIZE output+output_len > > output+run_size > > > > [output, output+init_size) is the for decompressing buffer for compressed > > kernel. > > > > [output, output+run_size) is for VO run size. > > [output, output+output_len) is (VO (vmlinux after objcopy) plus relocs) > > > > [output+init_size-ZO_INIT_SIZE, output+init_size) is copied ZO. > > [input, input+input_len) is copied compressed (VO (vmlinux after objcopy) > > plus relocs), not the ZO. > > > > [input+input_len, output+init_size) is [_text, _end) for ZO. that could be > > first range in mem_avoid. Now the new first entry already includes heap and > > stack for ZO running. So no need to put them separately into mem_avoid > > array. > > > > Also [input, input+input_size) need be put in mem_avoid array. It is > > adjacent > > to new first entry, so merge them. > > I wonder if this diagram and description should live in a comment with the > code. I think it would be very helpful for people interested in this process. Do you think it's ok to put it where init_size is calculated in boot/header.S? Or other suitable places? > > > > > > At last we need to put boot_params into the mem_avoid too. As with 64bit > > bootloader could put it anywhere. > > > > After these changes, we have all ranges which need be avoided in mem_avoid > > array. > > > > Cc: Kees Cook <keesc...@chromium.org> > > Signed-off-by: Yinghai Lu <ying...@kernel.org> > > --- > > v2->v3: > > Adjust the patch log. > > > > arch/x86/boot/compressed/aslr.c | 29 +++++++++++++---------------- > > 1 file changed, 13 insertions(+), 16 deletions(-) > > > > diff --git a/arch/x86/boot/compressed/aslr.c > > b/arch/x86/boot/compressed/aslr.c > > index 622aa88..b93be03 100644 > > --- a/arch/x86/boot/compressed/aslr.c > > +++ b/arch/x86/boot/compressed/aslr.c > > @@ -109,7 +109,7 @@ struct mem_vector { > > unsigned long size; > > }; > > > > -#define MEM_AVOID_MAX 5 > > +#define MEM_AVOID_MAX 4 > > static struct mem_vector mem_avoid[MEM_AVOID_MAX]; > > > > static bool mem_contains(struct mem_vector *region, struct mem_vector > > *item) > > @@ -135,21 +135,22 @@ static bool mem_overlaps(struct mem_vector *one, > > struct mem_vector *two) > > } > > > > static void mem_avoid_init(unsigned long input, unsigned long input_size, > > - unsigned long output, unsigned long output_size) > > + unsigned long output) > > { > > + unsigned long init_size = real_mode->hdr.init_size; > > u64 initrd_start, initrd_size; > > u64 cmd_line, cmd_line_size; > > - unsigned long unsafe, unsafe_len; > > char *ptr; > > > > /* > > * Avoid the region that is unsafe to overlap during > > - * decompression (see calculations at top of misc.c). > > + * decompression. > > + * As we already move ZO (arch/x86/boot/compressed/vmlinux) > > + * to the end of buffer, [input+input_size, output+init_size) > > + * has [_text, _end) for ZO. > > */ > > - unsafe_len = (output_size >> 12) + 32768 + 18; > > - unsafe = (unsigned long)input + input_size - unsafe_len; > > - mem_avoid[0].start = unsafe; > > - mem_avoid[0].size = unsafe_len; > > + mem_avoid[0].start = input; > > + mem_avoid[0].size = (output + init_size) - input; > > > > /* Avoid initrd. */ > > initrd_start = (u64)real_mode->ext_ramdisk_image << 32; > > @@ -169,13 +170,9 @@ static void mem_avoid_init(unsigned long input, > > unsigned long input_size, > > mem_avoid[2].start = cmd_line; > > mem_avoid[2].size = cmd_line_size; > > > > - /* Avoid heap memory. */ > > - mem_avoid[3].start = (unsigned long)free_mem_ptr; > > - mem_avoid[3].size = BOOT_HEAP_SIZE; > > - > > - /* Avoid stack memory. */ > > - mem_avoid[4].start = (unsigned long)free_mem_end_ptr; > > - mem_avoid[4].size = BOOT_STACK_SIZE; > > + /* Avoid params */ > > + mem_avoid[3].start = (unsigned long)real_mode; > > + mem_avoid[3].size = sizeof(*real_mode); > > } > > > > /* Does this memory vector overlap a known avoided area? */ > > @@ -319,7 +316,7 @@ unsigned char *choose_kernel_location(unsigned char > > *input, > > > > /* Record the various known unsafe memory ranges. */ > > mem_avoid_init((unsigned long)input, input_size, > > - (unsigned long)output, output_size); > > + (unsigned long)output); > > > > /* Walk e820 and find a random address. */ > > random = find_random_addr(choice, output_size); > > -- > > 2.5.0 > > > > > > -- > Kees Cook > Chrome OS & Brillo Security