linux-kernel - Re: [PATCHv4 3/5] x86/mm: fix 32-bit mmap() for 64-bit ELF

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: <397ddf67-92bb-f210-bb8b-09580db41385@virtuozzo.com>
Date:   Tue, 14 Feb 2017 18:24:23 +0300
From:   Dmitry Safonov <dsafonov@...tuozzo.com>
To:     Thomas Gleixner <tglx@...utronix.de>
CC:     <linux-kernel@...r.kernel.org>, <0x7f454c46@...il.com>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...e.de>, <x86@...nel.org>,
        <linux-mm@...ck.org>
Subject: Re: [PATCHv4 3/5] x86/mm: fix 32-bit mmap() for 64-bit ELF

On 02/11/2017 10:49 PM, Thomas Gleixner wrote:
> On Mon, 30 Jan 2017, Dmitry Safonov wrote:
>
>> Fix 32-bit compat_sys_mmap() mapping VMA over 4Gb in 64-bit binaries
>> and 64-bit sys_mmap() mapping VMA only under 4Gb in 32-bit binaries.
>> Introduced new bases for compat syscalls in mm_struct:
>> mmap_compat_base and mmap_compat_legacy_base for top-down and
>> bottom-up allocations accordingly.
>> Taught arch_get_unmapped_area{,_topdown}() to use the new mmap_bases
>> in compat syscalls for high/low limits in vm_unmapped_area().
>>
>> I discovered that bug on ZDTM tests for compat 32-bit C/R.
>> Working compat sys_mmap() in 64-bit binaries is really needed for that
>> purpose, as 32-bit applications are restored from 64-bit CRIU binary.
>
> Again that changelog sucks.
>
> Explain the problem/bug first. Then explain the way to fix it and do not
> tell fairy tales about what you did without explaing the bug in the first
> place.
>
> Documentation....SubittingPatches explains that very well.

Rewrote changelog.

>> +config HAVE_ARCH_COMPAT_MMAP_BASES
>> +	bool
>> +	help
>> +	  If this is set, one program can do native and compatible syscall
>> +	  mmap() on architecture. Thus kernel has different bases to
>> +	  compute high and low virtual address limits for allocation.
>
> Sigh. How is a user supposed to decode this?
>
> 	  This allows 64bit applications to invoke syscalls in 64bit and
> 	  32bit mode. Required for ....

Ok

>>
>> @@ -113,10 +114,19 @@ static void find_start_end(unsigned long flags, unsigned long *begin,
>>  		if (current->flags & PF_RANDOMIZE) {
>>  			*begin = randomize_page(*begin, 0x02000000);
>>  		}
>> -	} else {
>> -		*begin = current->mm->mmap_legacy_base;
>> -		*end = TASK_SIZE;
>> +		return;
>>  	}
>> +
>> +#ifdef CONFIG_COMPAT
>
> Can you please find a solution which does not create that ifdef horror in
> the code? Just a few accessors to those compat fields are required to do
> that.

I'll try

>> +
>> +#ifdef CONFIG_COMPAT
>> +	arch_pick_mmap_base(&mm->mmap_compat_base, &mm->mmap_compat_legacy_base,
>> +			arch_compat_rnd(), IA32_PAGE_OFFSET);
>> +#endif
>
> Ditto
>
> Thanks,
>
> 	tglx
>


-- 
              Dmitry