linux-kernel - Re: [PATCH v3 06/19] x86, kaslr: Clean up useless code related to run

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160308050041.GC2481@x1.redhat.com>
Date:	Tue, 8 Mar 2016 13:00:41 +0800
From:	Baoquan He <bhe@...hat.com>
To:	Kees Cook <keescook@...omium.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Yinghai Lu <yinghai@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>, Vivek Goyal <vgoyal@...hat.com>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Andy Lutomirski <luto@...nel.org>, lasse.collin@...aani.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dave Young <dyoung@...hat.com>,
	Josh Triplett <josh@...htriplett.org>,
	Matt Fleming <matt.fleming@...el.com>,
	Ard Biesheuvel <ard.biesheuvel@...aro.org>,
	Junjie Mao <eternal.n08@...il.com>
Subject: Re: [PATCH v3 06/19] x86, kaslr: Clean up useless code related to
 run_size.

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