linux-kernel - Re: [PATCH v3 08/19] x86, kaslr: Consolidate mem

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]

Date:	Wed, 9 Mar 2016 21:42:27 +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>
Subject: Re: [PATCH v3 08/19] x86, kaslr: Consolidate mem_avoid array filling

On 03/08/16 at 10:17am, Kees Cook wrote:
> On Mon, Mar 7, 2016 at 9:21 PM, Baoquan He <bhe@...hat.com> wrote:
> > On 03/07/16 at 03:28pm, Kees Cook wrote:
> >> On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@...hat.com> wrote:
> >> > From: Yinghai Lu <yinghai@...nel.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?
> 
> Let's put it in aslr.c since it's the biggest user of these
> calculations? I would tend to avoid putting large comments in a .S
> file, but that's just personal preference.

Yeah, agree. Will put it in aslr.c.

Thanks a lot for your good suggestions.

>