Re: [PATCH v3 06/19] x86, kaslr: Clean up useless code related to run_size.

From: Baoquan He
Date: Tue Mar 08 2016 - 00:01:00 EST


On 03/07/16 at 03:12pm, Kees Cook wrote:
> > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> > index 42e76c2..069120e 100644
> > --- a/arch/x86/boot/compressed/misc.c
> > +++ b/arch/x86/boot/compressed/misc.c
> > @@ -393,9 +393,9 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> > unsigned char *input_data,
> > unsigned long input_len,
> > unsigned char *output,
> > - unsigned long output_len,
> > - unsigned long run_size)
> > + unsigned long output_len)
> > {
> > + unsigned long run_size = VO__end - VO__text;
>
> I think this can be "const" now, yes?

Yes, agree. No one can change it.
Will update it.

Thanks

>
> > unsigned char *output_orig = output;
> >
> > real_mode = rmode;
> > @@ -416,8 +416,6 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> > lines = real_mode->screen_info.orig_video_lines;
> > cols = real_mode->screen_info.orig_video_cols;
> >
> > - run_size = VO__end - VO__text;
> > -
> > console_init();
> > debug_putstr("early console in decompress_kernel\n");
> >
> > diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
> > index a613c84..c5148642 100644
> > --- a/arch/x86/boot/compressed/mkpiggy.c
> > +++ b/arch/x86/boot/compressed/mkpiggy.c
> > @@ -34,13 +34,11 @@ int main(int argc, char *argv[])
> > {
> > uint32_t olen;
> > long ilen;
> > - unsigned long run_size;
> > FILE *f = NULL;
> > int retval = 1;
> >
> > - if (argc < 3) {
> > - fprintf(stderr, "Usage: %s compressed_file run_size\n",
> > - argv[0]);
> > + if (argc < 2) {
> > + fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
> > goto bail;
> > }
> >
> > @@ -65,15 +63,11 @@ int main(int argc, char *argv[])
> > ilen = ftell(f);
> > olen = get_unaligned_le32(&olen);
> >
> > - run_size = atoi(argv[2]);
> > -
> > printf(".section \".rodata..compressed\",\"a\",@progbits\n");
> > printf(".globl z_input_len\n");
> > printf("z_input_len = %lu\n", ilen);
> > printf(".globl z_output_len\n");
> > printf("z_output_len = %lu\n", (unsigned long)olen);
> > - printf(".globl z_run_size\n");
> > - printf("z_run_size = %lu\n", run_size);
> >
> > printf(".globl input_data, input_data_end\n");
> > printf("input_data:\n");
> > diff --git a/arch/x86/tools/calc_run_size.sh b/arch/x86/tools/calc_run_size.sh
> > deleted file mode 100644
> > index 1a4c17b..0000000
> > --- a/arch/x86/tools/calc_run_size.sh
> > +++ /dev/null
> > @@ -1,42 +0,0 @@
> > -#!/bin/sh
> > -#
> > -# Calculate the amount of space needed to run the kernel, including room for
> > -# the .bss and .brk sections.
> > -#
> > -# Usage:
> > -# objdump -h a.out | sh calc_run_size.sh
> > -
> > -NUM='\([0-9a-fA-F]*[ \t]*\)'
> > -OUT=$(sed -n 's/^[ \t0-9]*.b[sr][sk][ \t]*'"$NUM$NUM$NUM$NUM"'.*/\1\4/p')
> > -if [ -z "$OUT" ] ; then
> > - echo "Never found .bss or .brk file offset" >&2
> > - exit 1
> > -fi
> > -
> > -OUT=$(echo ${OUT# })
> > -sizeA=$(printf "%d" 0x${OUT%% *})
> > -OUT=${OUT#* }
> > -offsetA=$(printf "%d" 0x${OUT%% *})
> > -OUT=${OUT#* }
> > -sizeB=$(printf "%d" 0x${OUT%% *})
> > -OUT=${OUT#* }
> > -offsetB=$(printf "%d" 0x${OUT%% *})
> > -
> > -run_size=$(( $offsetA + $sizeA + $sizeB ))
> > -
> > -# BFD linker shows the same file offset in ELF.
> > -if [ "$offsetA" -ne "$offsetB" ] ; then
> > - # Gold linker shows them as consecutive.
> > - endB=$(( $offsetB + $sizeB ))
> > - if [ "$endB" != "$run_size" ] ; then
> > - printf "sizeA: 0x%x\n" $sizeA >&2
> > - printf "offsetA: 0x%x\n" $offsetA >&2
> > - printf "sizeB: 0x%x\n" $sizeB >&2
> > - printf "offsetB: 0x%x\n" $offsetB >&2
> > - echo ".bss and .brk are non-contiguous" >&2
> > - exit 1
> > - fi
> > -fi
> > -
> > -printf "%d\n" $run_size
> > -exit 0
> > --
> > 2.5.0
> >
>
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security