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] [day] [month] [year] [list]

Message-ID: <1A0BF94E-2F42-4721-AEA1-27C048F7EAD6@zytor.com>
Date:   Mon, 02 Jan 2017 15:06:16 -0800
From:   hpa@...or.com
To:     Andy Lutomirski <luto@...capital.net>,
        "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>,
        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 January 2, 2017 8:52:41 AM PST, Andy Lutomirski <luto@...capital.net> wrote:
>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

It's an API that already exists, that's the plus. Hard and soft limits *do* make sense IMO.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.