Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1682039 > unrolled thread

[RFC][PATCH] exec: Use init rlimits for setuid exec

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-06 06:40 +0200
    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Andy Lutomirski <luto@kernel.org> - 2017-07-06 07:00 +0200
      Re: [RFC][PATCH] exec: Use init rlimits for setuid exec ebiederm@xmission.com (Eric W. Biederman) - 2017-07-06 15:00 +0200
        Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Andy Lutomirski <luto@kernel.org> - 2017-07-06 17:30 +0200
    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Willy Tarreau <w@1wt.eu> - 2017-07-06 07:50 +0200
    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec ebiederm@xmission.com (Eric W. Biederman) - 2017-07-06 14:50 +0200
      Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Andy Lutomirski <luto@kernel.org> - 2017-07-06 17:40 +0200
    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-06 18:40 +0200
      Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-06 19:00 +0200
      Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-06 19:30 +0200
        Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-06 20:00 +0200
          Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-06 21:20 +0200
            Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Andy Lutomirski <luto@kernel.org> - 2017-07-07 06:50 +0200
              Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-07 07:10 +0200
              Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 07:20 +0200
                Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-07 07:40 +0200
                  Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 07:50 +0200
                    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 08:50 +0200
                      Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-07 18:30 +0200
                        Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 20:30 +0200
                Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Andy Lutomirski <luto@kernel.org> - 2017-07-07 07:40 +0200
                  Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 07:50 +0200
                    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-07 08:10 +0200
                      Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 08:20 +0200
                        Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-07 18:10 +0200
                          Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 20:30 +0200
                    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Andy Lutomirski <luto@kernel.org> - 2017-07-07 17:00 +0200
              Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-07 07:20 +0200
            Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Michal Hocko <mhocko@kernel.org> - 2017-07-10 10:50 +0200
              Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-10 18:20 +0200
                Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Willy Tarreau <w@1wt.eu> - 2017-07-10 19:00 +0200
              Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Kees Cook <keescook@chromium.org> - 2017-07-10 18:20 +0200
                Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Michal Hocko <mhocko@kernel.org> - 2017-07-10 18:30 +0200
                  Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Michal Hocko <mhocko@kernel.org> - 2017-07-10 20:20 +0200
                    Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Rik van Riel <riel@redhat.com> - 2017-07-10 20:40 +0200
      Re: [RFC][PATCH] exec: Use init rlimits for setuid exec Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-07-13 02:00 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1682926

On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>> 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?
>>
>> We should avoid using up tons of memory and then failing. Better to
>> cap it as we use it. Plumbing a sane value into this shouldn't be hard
>> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).

Aren't there real use cases that use many megs of arguments?

We could probably get away with saying max(rlimit(RLIMIT_STACK), 2MB)
as long as we make sure later on that we don't screw up if we've
overallocated?

>>
>>> 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.
>>
>> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
>> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
>> this, but I thought it was relied on by userspace.
>
> I always say this backwards. :P Default is top-down (allocate at high
> addresses and work down toward low). With unlimited stack, allocations
> start at low addresses and work up. Here's the results (shown with
> randomize_va_space sysctl set to 0):

Uhh, crikey!  Where's the code that does that?

[toc] | [prev] | [next] | [standalone]

#1682933

On Thu, Jul 6, 2017 at 10:36 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>>> 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?
>>>
>>> We should avoid using up tons of memory and then failing. Better to
>>> cap it as we use it. Plumbing a sane value into this shouldn't be hard
>>> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).
>
> Aren't there real use cases that use many megs of arguments?

They'd be relatively new since the args were pretty limited before.
I'd be curious to see them.

> We could probably get away with saying max(rlimit(RLIMIT_STACK), 2MB)
> as long as we make sure later on that we don't screw up if we've
> overallocated?

min, not max, but yeah. Here's part of what I have for get_arg_page():

                rlim = current->signal->rlim;
-               if (size > READ_ONCE(rlim[RLIMIT_STACK].rlim_cur) / 4)
+               arg_stack = READ_ONCE(rlim[RLIMIT_STACK].rlim_cur);
+               arg_stack = min_t(unsigned long, arg_stack, _STK_LIM) / 4;
+               if (size > arg_stack)
                        goto fail;

>>> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
>>> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
>>> this, but I thought it was relied on by userspace.
>>
>> I always say this backwards. :P Default is top-down (allocate at high
>> addresses and work down toward low). With unlimited stack, allocations
>> start at low addresses and work up. Here's the results (shown with
>> randomize_va_space sysctl set to 0):
>
> Uhh, crikey!  Where's the code that does that?

That was the call path I quoted earlier:

> 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

i.e. arch_pick_mmap_layout().

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1682940

On Thu, Jul 6, 2017 at 10:45 PM, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 10:36 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>
>> Aren't there real use cases that use many megs of arguments?
>
> They'd be relatively new since the args were pretty limited before.
> I'd be curious to see them.

"megs" yes. "many megs" no.

The traditional kernel limit was 32 pages (so 128kB on x86, explaining
our MAX_ARG value).

We moved to the much nider "two active VM's at the same time" model a
fairly long time ago, though - it was back in v2.6.23 or so. So about
10 years ago.

I would have expected lots of scripts to have been written since that
just end up going *far* over the old 128kB limit, because it's really
easy to do.

Things like big directories and the shell expanding "*" can easily be
a megabyte of arguments. I know I used to have scripts where I had to
use "xargs" in the past, and with the > 128kB change I just stopped,
because "a couple of megabytes" is enough for a lot of things where
128kB wasn't necessarily.

Oh, one example is actually the kernel source tree. I don't do it any
more (because "git grep" is much better), but I used to do things like

    grep something $(find . -name '*.[ch]')

all the time.

And that actually currently *just* overflows the 2MB argument size,
but used to work (easily) ten years ago. Oh, how the kernel has
grown..

Yes, yes, *portably* you should always have done

  find . -print0 -name '*.[ch]' | xargs -0 grep

but be honest now: that first thing is what you actually write when
you do some throw-away one-liner.

So 2+MB is still definitely something people can do (and probably *do* do).

             Linus

[toc] | [prev] | [next] | [standalone]

#1682943

On Thu, Jul 6, 2017 at 11:02 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> So 2+MB is still definitely something people can do (and probably *do* do).

With the default 8MB stack, most people are already limited to 2MB
here. I guess the question is, do people raise their stack rlimit to
gain more arguments? Should I pick a different value for the args?

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1683274

On Thu, Jul 6, 2017 at 11:10 PM, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 11:02 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> So 2+MB is still definitely something people can do (and probably *do* do).
>
> With the default 8MB stack, most people are already limited to 2MB
> here. I guess the question is, do people raise their stack rlimit to
> gain more arguments? Should I pick a different value for the args?

So I would not be at all surprised if people just made the stack limit
higher when they hit the E2BIG issue in some script.

So yes, I'd make the max args cutoff be higher than 2MB.

I'd suggest we make the code do:

 (a) keep the existing rlimit/4 check (so *most* people will see the
exact same behavior)

 (b) add a static max arg check for something that is closer to 8MB
but leaves a somewhat reasonable stack size even if the stack size get
reset to 8MB

I'd suggest that (b) case just be 6MB or something. Maybe make it
(_STK_LIM/4*3) or whatever, in case we ever end up changing that
value.

Hmm?

                 Linus

[toc] | [prev] | [next] | [standalone]

#1683361

On Fri, Jul 7, 2017 at 9:06 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jul 6, 2017 at 11:10 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Thu, Jul 6, 2017 at 11:02 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> So 2+MB is still definitely something people can do (and probably *do* do).
>>
>> With the default 8MB stack, most people are already limited to 2MB
>> here. I guess the question is, do people raise their stack rlimit to
>> gain more arguments? Should I pick a different value for the args?
>
> So I would not be at all surprised if people just made the stack limit
> higher when they hit the E2BIG issue in some script.
>
> So yes, I'd make the max args cutoff be higher than 2MB.
>
> I'd suggest we make the code do:
>
>  (a) keep the existing rlimit/4 check (so *most* people will see the
> exact same behavior)
>
>  (b) add a static max arg check for something that is closer to 8MB
> but leaves a somewhat reasonable stack size even if the stack size get
> reset to 8MB
>
> I'd suggest that (b) case just be 6MB or something. Maybe make it
> (_STK_LIM/4*3) or whatever, in case we ever end up changing that
> value.

Sounds good. I'll send a patch...

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1683239

On Thu, Jul 6, 2017 at 10:45 PM, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Jul 6, 2017 at 10:36 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> Aren't there real use cases that use many megs of arguments?
>
> They'd be relatively new since the args were pretty limited before.
> I'd be curious to see them.
>
>> We could probably get away with saying max(rlimit(RLIMIT_STACK), 2MB)
>> as long as we make sure later on that we don't screw up if we've
>> overallocated?
>
> min, not max, but yeah. Here's part of what I have for get_arg_page():
>
>                 rlim = current->signal->rlim;
> -               if (size > READ_ONCE(rlim[RLIMIT_STACK].rlim_cur) / 4)
> +               arg_stack = READ_ONCE(rlim[RLIMIT_STACK].rlim_cur);
> +               arg_stack = min_t(unsigned long, arg_stack, _STK_LIM) / 4;
> +               if (size > arg_stack)
>                         goto fail;

I really did mean max, the idea being that, if we're going to increase
rlim_cur, it's a bit odd to fail the exec if it would have worked
under the higher value.  That being said, I see no real exploit vector
here if just rlimit(RLIMIT_STACK) is used.

(Can you just use rlimit()?  The open-coding seems entirely useless.)

I thought of another approach, though: change the rlimit macros so
that a secureexec program always gets at least 8MB stack.  Might be
less regression-prone.

[toc] | [prev] | [next] | [standalone]

#1682921

On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski <luto@kernel.org> wrote:
> 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?

We should avoid using up tons of memory and then failing. Better to
cap it as we use it. Plumbing a sane value into this shouldn't be hard
at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).

> 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.

IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
mmap instead of bottom-up. I mean, I'd be delighted to get rid of
this, but I thought it was relied on by userspace.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1684060

On Thu 06-07-17 12:12:55, Kees Cook wrote:
> On Thu, Jul 6, 2017 at 10:52 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Thu, Jul 6, 2017 at 10:29 AM, Kees Cook <keescook@chromium.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

FWIW this is gone in tip tree. See
lkml.kernel.org/r/20170614082218.12450-1-mhocko@kernel.org

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684397

On Mon, Jul 10, 2017 at 9:12 AM, Kees Cook <keescook@chromium.org> wrote:
>
> Sounds good to me, but won't large-memory users in 32-bit get annoyed?

We'll see.

I suspect that all large-memory users have long since upgraded to
x86-64 (rule of thumb: if you are upgrading kernels today, you
probably upgraded hardware ten years ago), and that this may be a
non-issue today.

But only time will tell.

I certainly prefer "keep it simple" over theoretical concerns. It's
why I prefer that unconditional stack limit too - we may have to make
it conditional on suid'ness or something like the ELF PT_GNU_STACK
setting, but before over-designing things, let's see if anybody even
cares.

             Linus

[toc] | [prev] | [next] | [standalone]

#1684419

On Mon, Jul 10, 2017 at 09:18:09AM -0700, Linus Torvalds wrote:
> On Mon, Jul 10, 2017 at 9:12 AM, Kees Cook <keescook@chromium.org> wrote:
> >
> > Sounds good to me, but won't large-memory users in 32-bit get annoyed?
> 
> We'll see.
> 
> I suspect that all large-memory users have long since upgraded to
> x86-64 (rule of thumb: if you are upgrading kernels today, you
> probably upgraded hardware ten years ago), and that this may be a
> non-issue today.

I tend to agree. We've been using 32-bit machines with "a lot" (=2GB)
of RAM and haproxy using something like 1.3GB in the past, and it
started to become a bit complex due to ASLR puching large holes between
each and every shared object, forcing us to stop setting strict
overcommit limits for example. We've abandonned them after kernel 3.10,
when the new models had been migrated to 64 bits a few years ago already
and I think anyone doing anything serious with memory doesn't use 32-bit
at all.

Well I know of one exception :-)  My netbook has 3 GB and is 32-bit,
running on 4.9 :

willy@eeepc:~$ uname -a
Linux eeepc 4.9.36-eeepc #1 SMP Mon Jul 10 07:33:29 CEST 2017 i686 Intel(R) Atom(TM) CPU N2800   @ 1.86GHz GenuineIntel GNU/Linux
willy@eeepc:~$ free
             total       used       free     shared    buffers     cached
Mem:       3097840     649816    2448024          0      52476     507216
-/+ buffers/cache:      90124    3007716
Swap:      1025440          0    1025440

It only runs end-user stuff (firefox) so it cannot be considered anything
serious.

Willy

[toc] | [prev] | [next] | [standalone]

#1684398

On Mon, Jul 10, 2017 at 1:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Thu 06-07-17 12:12:55, Kees Cook wrote:
>> On Thu, Jul 6, 2017 at 10:52 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>> > On Thu, Jul 6, 2017 at 10:29 AM, Kees Cook <keescook@chromium.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
>
> FWIW this is gone in tip tree. See
> lkml.kernel.org/r/20170614082218.12450-1-mhocko@kernel.org

Sounds good to me, but won't large-memory users in 32-bit get annoyed?

A setuid/setgid exec will also get ADDR_COMPAT_LAYOUT cleared in
bprm_fill_uid() (but not for fs-caps), so that'll keep things from
getting layout-controlled. Thanks!

(ADDR_COMPAT_LAYOUT isn't cleared for capability elevations, though, I think.)

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1684402

On Mon 10-07-17 09:12:11, Kees Cook wrote:
> On Mon, Jul 10, 2017 at 1:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Thu 06-07-17 12:12:55, Kees Cook wrote:
> >> On Thu, Jul 6, 2017 at 10:52 AM, Linus Torvalds
> >> <torvalds@linux-foundation.org> wrote:
> >> > On Thu, Jul 6, 2017 at 10:29 AM, Kees Cook <keescook@chromium.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
> >
> > FWIW this is gone in tip tree. See
> > lkml.kernel.org/r/20170614082218.12450-1-mhocko@kernel.org
> 
> Sounds good to me, but won't large-memory users in 32-bit get annoyed?

Why would they? 32b do bottom up layouts by default.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684555

On Mon 10-07-17 18:27:51, Michal Hocko wrote:
> On Mon 10-07-17 09:12:11, Kees Cook wrote:
[...]
> > >> 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
> > >
> > > FWIW this is gone in tip tree. See
> > > lkml.kernel.org/r/20170614082218.12450-1-mhocko@kernel.org
> > 
> > Sounds good to me, but won't large-memory users in 32-bit get annoyed?
> 
> Why would they? 32b do bottom up layouts by default.

OK, I misread the code. 32b applications on 64b systems do top down by
default and only if they override this by ADDR_COMPAT_LAYOUT
personality. For some reason I thought that 32b userspace goes a
different path and makes sure that they are always doing bottom up.

Anyway even if somebody really needs to grow stack really large we have
the personality to give them the legacy layout.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1684568

On Mon, 2017-07-10 at 20:16 +0200, Michal Hocko wrote:

> OK, I misread the code. 32b applications on 64b systems do top down
> by
> default and only if they override this by ADDR_COMPAT_LAYOUT
> personality. For some reason I thought that 32b userspace goes a
> different path and makes sure that they are always doing bottom up.
> 
> Anyway even if somebody really needs to grow stack really large we
> have
> the personality to give them the legacy layout.

I think what will happen when rlimit_stack is RLIMIT_INFINITY
is that mmap_base will end up placing mm->mmap_base at 512MB
(task_size / 6 * 5 below the top of address space) for 32 bit
kernels, and we eventually fall back to a bottom-up search
if the space below mmap_base is exhausted (if it ever is).

[toc] | [prev] | [next] | [standalone]

#1686138

>  (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 }.

Even that is dangerous because a setuid binary can be transitioning
between two users (none privileged) yet be subject to an rlimit attack.
There's even less reason to believe that non root setuid binaries are
properly hardened than obvious targets. CPU limit attacks in particular
can be used to do some quite clever things.

Also consider a binary that is gaining some minor right (eg network
rights) being targetted because giving it extra permissions allows the
attacker to gain access to infinite resources when that clearly isn't the
intent.

>  (c) perhaps encourage people to annotate their suid binaries with
> initial resource requirements (and for stack, I mean the existing
> GNU_STACK ELF annotation in particular).

Making this for setuid binaries only makes no sense. If a user can
annotate required resources and the execve() fails if those resources are
over the rlimit then that is a useful feature full stop, and there's no
reason to even make it setuid dependent.

Alan

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web