linux-kernel - Re: How should we handle variable address space sizes (Re: [RFC 3/4] x86/mm: define TASK_SIZE as current->mm->task

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: <CALCETrU+BJ2JDRocBy-6SD_G6FbD+Ez_yb1QJ6Z1KXBjVzKOHg@mail.gmail.com>
Date:   Mon, 2 Jan 2017 08:52:41 -0800
From:   Andy Lutomirski <luto@...capital.net>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
Cc:     Dmitry Safonov <dsafonov@...tuozzo.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        linux-arch <linux-arch@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        Will Deacon <will.deacon@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "Carlos O'Donell" <carlos@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>
Subject: Re: How should we handle variable address space sizes (Re: [RFC 3/4]
 x86/mm: define TASK_SIZE as current->mm->task_size)

On Mon, Jan 2, 2017 at 1:49 AM, Kirill A. Shutemov <kirill@...temov.name> wrote:
> On Fri, Dec 30, 2016 at 06:11:05PM -0800, Andy Lutomirski wrote:
>> On Fri, Dec 30, 2016 at 7:56 AM, Dmitry Safonov <dsafonov@...tuozzo.com> wrote:
>> > Keep task's virtual address space size as mm_struct field which
>> > exists for a long time - it's initialized in setup_new_exec()
>> > depending on the new task's personality.
>> > This way TASK_SIZE will always be the same as current->mm->task_size.
>> > Previously, there could be an issue about different values of
>> > TASK_SIZE and current->mm->task_size: e.g, a 32-bit process can unset
>> > ADDR_LIMIT_3GB personality (with personality syscall) and
>> > so TASK_SIZE will be 4Gb, which is larger than mm->task_size = 3Gb.
>> > As TASK_SIZE *and* current->mm->task_size are used both in code
>> > frequently, this difference creates a subtle situations, for example:
>> > one can mmap addresses > 3Gb, but they will be hidden in
>> > /proc/pid/pagemap as it checks mm->task_size.
>> > I've moved initialization of mm->task_size earlier in setup_new_exec()
>> > as arch_pick_mmap_layout() initializes mmap_legacy_base with
>> > TASK_UNMAPPED_BASE, which depends on TASK_SIZE.
>>
>> I don't like this patch so much because I think that we should figure
>> out how this will all work in the long run first.  I've added some
>> more people to the thread because other arches have similar issues and
>> because x86 is about to get considerably more complicated (choices
>> include 3GB, 4GB, 47-bit, and 56-bit (the latter IIRC)).
>>
>> Here are a few of my thoughts on the matter.  This isn't all that well
>> thought out:
>>
>> The address space limit, especially if CRIU is in play, isn't really a
>> hard limit.  For example, you could allocate high memory then lower
>> the limit.  Similarly, I see no reason that an x32 program should be
>> forbidden from mapping some high addresses or, similarly, that an i386
>> program can't (if it really wanted to) do a 64-bit mmap() and get a
>> high address.
>>
>> On that note, can we just *delete* the task_size check from pagemap?
>> It's been there since the very beginning:
>>
>> commit 85863e475e59afb027b0113290e3796ee6020b7d
>> Author: Matt Mackall <mpm@...enic.com>
>> Date:   Mon Feb 4 22:29:04 2008 -0800
>>
>>     maps4: add /proc/pid/pagemap interface
>>
>> and there's no explanation for why it's needed.
>>
>> So maybe we should have a *number* (not a bit) that indicates the
>> maximum address that mmap() will return unless an override is in use.
>> Since common practice seems to be to stick this in the personality
>> field, we may need some fancy encoding.  Executing a setuid binary
>> needs to reset to the default, and personality handles that.
>
> If we want to be able to specify arbitrary address as maximum, a fancy
> encoding would need to claim 51 bits (63 VA - 12 in-page address) on x86
> from the persona flag.
> To me, it's stretching personality interface too far.
>
> Maybe it's easier to reset the rlimit for suid binaries?

I guess I don't see why rlimit makes any sense, though.  It's not a
resource utilization control, hard vs soft limits make very little
sense, requiring capabilities to exceed the hard limit doesn't help
anything, and it's only useful to preserve it across execve() to work
around bugs.

So if it's going to be a number, let's just make it be a new number
with a new API to control it.

--Andy