linux-kernel - Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

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:   Thu, 6 Jul 2017 21:48:49 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Michal Hocko <mhocko@...nel.org>,
        Ben Hutchings <ben@...adent.org.uk>, Willy Tarreau <w@....eu>,
        Hugh Dickins <hughd@...gle.com>,
        Oleg Nesterov <oleg@...hat.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Rik van Riel <riel@...hat.com>,
        Larry Woodman <lwoodman@...hat.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Tony Luck <tony.luck@...el.com>,
        "James E.J. Bottomley" <jejb@...isc-linux.org>,
        Helge Diller <deller@....de>,
        James Hogan <james.hogan@...tec.com>,
        Laura Abbott <labbott@...hat.com>, Greg KH <greg@...ah.com>,
        "security@...nel.org" <security@...nel.org>,
        Qualys Security Advisory <qsa@...lys.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Ximin Luo <infinity0@...ian.org>
Subject: Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

On Thu, Jul 6, 2017 at 12:12 PM, Kees Cook <keescook@...omium.org> wrote:
> On Thu, Jul 6, 2017 at 10:52 AM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>> On Thu, Jul 6, 2017 at 10:29 AM, Kees Cook <keescook@...omium.org> wrote:
>>>>
>>>>  (a) minimal: just use our existing default stack (and stack _only_)
>>>> limit value for suid binaries that actually get extra permissions: {
>>>> _STK_LIM, RLIM_INFINITY }.
>>>
>>> This would look a lot like the existing patch; it'd just not copy the
>>> init process rlimits.
>>
>> Can't we just do the final rlimit setting so late in execve that we
>> don't need that whole "saved_rlimit" thing?
>
> The stack rlimit defines the mmap layout too:
>
> do_execveat_common() ->
> exec_binprm() ->
> search_binary_handler() ->
> fmt->load_binary (load_elf_binary()) ->
> setup_new_exec() ->
> arch_pick_mmap_layout() ->
> mmap_is_legacy() ->
> rlimit(RLIMIT_STACK) == RLIM_INFINITY
>
> exec_binprm() happens after the other stack setup (copy_strings()), so
> if we wanted to avoid saved_rlimit, we'd have to replumb how
> arch_pick_mmap_layout() works and how copy_strings() performs its
> calculations (neither looks too terrible).

How about a much simpler solution: don't read rlimit at all in
copy_strings(), let alone try to enforce it.  Instead, just before the
point of no return, check how much stack space is already used and, if
it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
abort.  Sure, this adds overhead if we're going to abort, but does
that really matter?

I don't see why using rlimit for layout control makes any sense
whatsoever.  Is there some historical reason we need that?  As far as
I can see (on insufficient inspection) is that the kernel is trying to
guarantee that, if we have so much arg crap that our remaining stack
is less than 128k, then we don't exceed our limit by a little bit.