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

Groups > linux.kernel > #1427131 > unrolled thread

[PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    [PATCH v3 02/13] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    [PATCH v3 03/13] x86/mm: Remove kernel_unmap_pages_in_pgd() and efi_cleanup_page_tables() Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
      Re: [PATCH v3 03/13] x86/mm: Remove kernel_unmap_pages_in_pgd() and  efi_cleanup_page_tables() Matt Fleming <matt@codeblueprint.co.uk> - 2016-06-21 12:20 +0200
    [PATCH v3 11/13] x86/dumpstack/64: Handle faults when printing the "Stack:" part of an OOPS Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    [PATCH v3 06/13] fork: Add generic vmalloced stack support Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
      Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support Jann Horn <jannh@google.com> - 2016-06-21 12:10 +0200
        Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support Andy Lutomirski <luto@amacapital.net> - 2016-06-21 19:10 +0200
          Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support Kees Cook <keescook@chromium.org> - 2016-06-21 19:20 +0200
            Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support Andy Lutomirski <luto@amacapital.net> - 2016-06-21 19:40 +0200
            Re: [kernel-hardening] Re: [PATCH v3 06/13] fork: Add generic  vmalloced stack support Rik van Riel <riel@redhat.com> - 2016-06-21 20:50 +0200
              Re: [kernel-hardening] Re: [PATCH v3 06/13] fork: Add generic  vmalloced stack support Andy Lutomirski <luto@amacapital.net> - 2016-06-21 21:50 +0200
              Re: [kernel-hardening] Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support Arnd Bergmann <arnd@arndb.de> - 2016-06-21 22:00 +0200
    [PATCH v3 04/13] mm: Track NR_KERNEL_STACK in KiB instead of number of stacks Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
      Re: [PATCH v3 04/13] mm: Track NR_KERNEL_STACK in KiB instead of  number of stacks Michal Hocko <mhocko@kernel.org> - 2016-06-22 10:50 +0200
    [PATCH v3 07/13] x86/die: Don't try to recover from an OOPS on a non-default stack Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    [PATCH v3 08/13] x86/dumpstack: When OOPSing, rewind the stack before do_exit Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    [PATCH v3 01/13] x86/mm/hotplug: Don't remove PGD entries in remove_pagetable() Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    [PATCH v3 10/13] x86/dumpstack: Try harder to get a call trace on stack overflow Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    [PATCH v3 09/13] x86/dumpstack: When dumping stack bytes due to OOPS, start with regs->sp Andy Lutomirski <luto@kernel.org> - 2016-06-21 01:50 +0200
    Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-21 06:10 +0200
      Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Andy Lutomirski <luto@amacapital.net> - 2016-06-21 18:50 +0200
        Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-21 19:20 +0200
          Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Andy Lutomirski <luto@amacapital.net> - 2016-06-21 19:40 +0200
            Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Kees Cook <keescook@chromium.org> - 2016-06-21 20:20 +0200
              Re: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks  with guard pages (x86, core) Rik van Riel <riel@redhat.com> - 2016-06-21 20:30 +0200
      Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Andy Lutomirski <luto@amacapital.net> - 2016-06-23 03:30 +0200
        Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-23 08:10 +0200
          Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86,  core) Oleg Nesterov <oleg@redhat.com> - 2016-06-23 16:40 +0200
            Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Linus Torvalds <torvalds@linux-foundation.org> - 2016-06-23 18:40 +0200
              Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Andy Lutomirski <luto@amacapital.net> - 2016-06-23 18:50 +0200
                Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86,  core) Oleg Nesterov <oleg@redhat.com> - 2016-06-23 19:20 +0200
              Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86,  core) Oleg Nesterov <oleg@redhat.com> - 2016-06-23 19:10 +0200
    Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Arnd Bergmann <arnd@arndb.de> - 2016-06-21 11:40 +0200
      Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Kees Cook <keescook@chromium.org> - 2016-06-21 19:30 +0200
        Re: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks  with guard pages (x86, core) Andy Lutomirski <luto@amacapital.net> - 2016-06-21 20:10 +0200
        Re: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks  with guard pages (x86, core) Rik van Riel <riel@redhat.com> - 2016-06-21 20:10 +0200
        Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Andy Lutomirski <luto@amacapital.net> - 2016-06-21 21:50 +0200
          Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Kees Cook <keescook@chromium.org> - 2016-06-21 22:20 +0200
        Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Arnd Bergmann <arnd@arndb.de> - 2016-06-21 21:50 +0200

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

#1427260

On Mon, Jun 20, 2016 at 4:43 PM, Andy Lutomirski <luto@kernel.org> wrote:
>
> On my laptop, this adds about 1.5µs of overhead to task creation,
> which seems to be mainly caused by vmalloc inefficiently allocating
> individual pages even when a higher-order page is available on the
> freelist.

I really think that problem needs to be fixed before this should be merged.

The easy fix may be to just have a very limited re-use of these stacks
in generic code, rather than try to do anything fancy with multi-page
allocations. Just a few of these allocations held in reserve (perhaps
make the allocations percpu to avoid new locks).

It won't help for a thundering herd problem where you start tons of
new threads, but those don't tend to be short-lived ones anyway. In
contrast, I think one common case is the "run shell scripts" that runs
tons and tons of short-lived processes, and having a small "stack of
stacks" would probably catch that case very nicely. Even a
single-entry cache might be ok, but I see no reason to not make it be
perhaps three or four stacks per CPU.

Make the "thread create/exit" sequence go really fast by avoiding the
allocation/deallocation, and hopefully catching a hot cache and TLB
line too.

Performance is not something that we add later. If the first version
of the patch series doesn't perform well, it should not be considered
ready.

            Linus

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

#1427947

On Mon, Jun 20, 2016 at 9:01 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jun 20, 2016 at 4:43 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>
>> On my laptop, this adds about 1.5µs of overhead to task creation,
>> which seems to be mainly caused by vmalloc inefficiently allocating
>> individual pages even when a higher-order page is available on the
>> freelist.
>
> I really think that problem needs to be fixed before this should be merged.
>
> The easy fix may be to just have a very limited re-use of these stacks
> in generic code, rather than try to do anything fancy with multi-page
> allocations. Just a few of these allocations held in reserve (perhaps
> make the allocations percpu to avoid new locks).
>
> It won't help for a thundering herd problem where you start tons of
> new threads, but those don't tend to be short-lived ones anyway. In
> contrast, I think one common case is the "run shell scripts" that runs
> tons and tons of short-lived processes, and having a small "stack of
> stacks" would probably catch that case very nicely. Even a
> single-entry cache might be ok, but I see no reason to not make it be
> perhaps three or four stacks per CPU.
>
> Make the "thread create/exit" sequence go really fast by avoiding the
> allocation/deallocation, and hopefully catching a hot cache and TLB
> line too.

To put the numbers in perspective: we'll pay the 1.5µs every time we
do any kind of clone(), but I think that many of the interesting cases
may be so far dominated by other costs that this is lost in the noise.
For scripts, execve() and all the dynamic linking overhead is so much
larger that no one will ever notice this:

time for i in `seq 1000`; do /bin/true; done

real    0m2.641s
user    0m0.058s
sys    0m0.107s

That's over 2ms per /bin/true invocation, so we're talking about less
than a 0.1% slowdown.  For fork() (i.e. !CLONE_VM), we'll have the
full cost of copying the mm.  And for anything with a thundering herd,
there will be lots of context switches, and just the context switches
are likely to swamp the task creation time.

On the flip side, on workloads where higher-order page allocation
requires any sort of compation, using vmalloc should be much faster.

So I'm leaning toward fewer cache entries per cpu, maybe just one.
I'm all for making it a bit faster, but I think we should weigh that
against increasing memory usage too much and thus scaring away the
embedded folks.

--Andy

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

#1427985

On Tue, Jun 21, 2016 at 9:45 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> So I'm leaning toward fewer cache entries per cpu, maybe just one.
> I'm all for making it a bit faster, but I think we should weigh that
> against increasing memory usage too much and thus scaring away the
> embedded folks.

I don't think the embedded folks will be scared by a per-cpu cache, if
it's just one or two entries.  And I really do think that even just
one or two entries will indeed catch a lot of the cases.

And yes, fork+execve() is too damn expensive in page table build-up
and tear-down. I'm not sure why bash doesn't do vfork+exec for when it
has to wait for the process anyway, but it doesn't seem to do that.

                 Linus

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

#1427996

On Tue, Jun 21, 2016 at 10:16 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Jun 21, 2016 at 9:45 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> So I'm leaning toward fewer cache entries per cpu, maybe just one.
>> I'm all for making it a bit faster, but I think we should weigh that
>> against increasing memory usage too much and thus scaring away the
>> embedded folks.
>
> I don't think the embedded folks will be scared by a per-cpu cache, if
> it's just one or two entries.  And I really do think that even just
> one or two entries will indeed catch a lot of the cases.
>
> And yes, fork+execve() is too damn expensive in page table build-up
> and tear-down. I'm not sure why bash doesn't do vfork+exec for when it
> has to wait for the process anyway, but it doesn't seem to do that.
>

I don't know about bash, but glibc very recently fixed a long-standing
but in posix_spawn and started using clone() in a sensible manner for
this.

FWIW, it may be a while before this can be enabled in distro kernels.
There are some code paths (*cough* crypto users *cough*) that think
that calling sg_init_one with a stack address is a reasonable thing to
do, and it doesn't work with a vmalloced stack.  grsecurity works
around this by using a real lowmem higher-order stack, aliasing it
into vmalloc space, and arranging for virt_to_phys to backtrack the
alias, but eww.  I think I'd rather find and fix the bugs, assuming
they're straightforward.

--Andy

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

#1428043

On Tue, Jun 21, 2016 at 10:27 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Tue, Jun 21, 2016 at 10:16 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Tue, Jun 21, 2016 at 9:45 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>
>>> So I'm leaning toward fewer cache entries per cpu, maybe just one.
>>> I'm all for making it a bit faster, but I think we should weigh that
>>> against increasing memory usage too much and thus scaring away the
>>> embedded folks.
>>
>> I don't think the embedded folks will be scared by a per-cpu cache, if
>> it's just one or two entries.  And I really do think that even just
>> one or two entries will indeed catch a lot of the cases.
>>
>> And yes, fork+execve() is too damn expensive in page table build-up
>> and tear-down. I'm not sure why bash doesn't do vfork+exec for when it
>> has to wait for the process anyway, but it doesn't seem to do that.
>>
>
> I don't know about bash, but glibc very recently fixed a long-standing
> but in posix_spawn and started using clone() in a sensible manner for
> this.
>
> FWIW, it may be a while before this can be enabled in distro kernels.
> There are some code paths (*cough* crypto users *cough*) that think
> that calling sg_init_one with a stack address is a reasonable thing to
> do, and it doesn't work with a vmalloced stack.  grsecurity works

... O_o ...

Why does it not work on a vmalloced stack??

> around this by using a real lowmem higher-order stack, aliasing it
> into vmalloc space, and arranging for virt_to_phys to backtrack the
> alias, but eww.  I think I'd rather find and fix the bugs, assuming
> they're straightforward.

Yeah. That's ugly.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

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

#1428055 — Re: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

[Multipart message — attachments visible in raw view] — view raw

On Tue, 2016-06-21 at 11:12 -0700, Kees Cook wrote:
> On Tue, Jun 21, 2016 at 10:27 AM, Andy Lutomirski
> <luto@amacapital.net> wrote:
> > FWIW, it may be a while before this can be enabled in distro
> > kernels.
> > There are some code paths (*cough* crypto users *cough*) that think
> > that calling sg_init_one with a stack address is a reasonable thing
> > to
> > do, and it doesn't work with a vmalloced stack.  grsecurity works
> ... O_o ...
> 
> Why does it not work on a vmalloced stack??

Because virt_to_page() does not work on vmalloced
memory.

-- 
All Rights Reversed.

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

#1429383

On Mon, Jun 20, 2016 at 9:01 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jun 20, 2016 at 4:43 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>
>> On my laptop, this adds about 1.5µs of overhead to task creation,
>> which seems to be mainly caused by vmalloc inefficiently allocating
>> individual pages even when a higher-order page is available on the
>> freelist.
>
> I really think that problem needs to be fixed before this should be merged.
>
> The easy fix may be to just have a very limited re-use of these stacks
> in generic code, rather than try to do anything fancy with multi-page
> allocations. Just a few of these allocations held in reserve (perhaps
> make the allocations percpu to avoid new locks).

I implemented a percpu cache, and it's useless.

When a task goes away, one reference is held until the next RCU grace
period so that task_struct can be used under RCU (look for
delayed_put_task_struct).  This means that free_task gets called in
giant batches under heavy clone() load, which is the only time that
any of this matters, which means that only get to refill the cache
once per RCU batch, which means that there's very little benefit.

Once thread_info stops living in the stack, we could, in principle,
exempt the stack itself from RCU protection, thus saving a bit of
memory under load and making the cache work.  I've started working on
(optionally, per-arch) getting rid of on-stack thread_info, but that's
not ready yet.

FWIW, the same issue quite possibly hurts non-vmap-stack performance
as well, as it makes it much less likely that a cache-hot stack gets
immediately reused under heavy fork load.

So may I skip this for now?  I think that the performance hit is
unlikely to matter on most workloads, and I also expect the speedup
from not using higher-order allocations to be a decent win on some
workloads.

--Andy

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

#1429492

On Wed, Jun 22, 2016 at 6:22 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> I implemented a percpu cache, and it's useless.
>
> When a task goes away, one reference is held until the next RCU grace
> period so that task_struct can be used under RCU (look for
> delayed_put_task_struct).

Yeah, that RCU batching will screw the cache idea.

But isn't it only the "task_struct" that needs that? That's a separate
allocation from the stack, which contains the "thread_info".

I think that what we *could* do is re-use the tread-info within the
RCU grace period, as long as we delay freeing the task_struct.

Yes, yes, we currently tie the task_struct and thread_info lifetimes
together very tightly, but that's a historical thing rather than a
requirement. We do the

        account_kernel_stack(tsk->stack, -1);
        arch_release_thread_info(tsk->stack);
        free_thread_info(tsk->stack);

in free_task(), but I could imagine doing it earlier, and
independently of the RCU-delayed free.

In fact, I think we just do that at exit() time synchronously. The
reference counting of the task_struct() is because a lot of other
threads can have references to the exiting thread (and we have the
tasklist and thread lists that are RCU-traversed), but none of those
other references should ever look at the stack. Or even the
thread-info.

Hmm. I bet it would show some problems, but not be technically
impossible. Especially if we make the thread-info rules be like the
SLAB_DESTROY_BY_RCU semantics - the allocation may be re-used during
the RCU grace period, but it is going to still exists and be of the
same type.

This sounds very much like something for Oleg Nesterov.

Oleg, what do you think? Would it be reasonable to free the stack and
thread_info synchronously at exit time, clear the pointer (to catch
any odd use), and only RCU-delay the task_struct itself?

That is, after all, what we already do with the VM, semaphores, files,
fs info etc. There's no real reason I see to keep the stack around.

(Obviously, we can't release it in do_exit() itself like we do some of
the other state - it would need to be released after we've scheduled
away to another process' stack, but we already have that TASK_DEAD
handling in finish_task_switch for this exact reason).

                 Linus

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

#1429906 — Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

On 06/22, Linus Torvalds wrote:
>
> Oleg, what do you think? Would it be reasonable to free the stack and
> thread_info synchronously at exit time, clear the pointer (to catch
> any odd use), and only RCU-delay the task_struct itself?

I didn't see the patches yet, quite possibly I misunderstood... But no,
I don't this we can do this (if we are not going to move ti->flags to
task_struct at least).

> (Obviously, we can't release it in do_exit() itself like we do some of
> the other state - it would need to be released after we've scheduled
> away to another process' stack, but we already have that TASK_DEAD
> handling in finish_task_switch for this exact reason).

Yes, but the problem is that a zombie thread can do its last schedule
before it is reaped.

Just for example, syscall_regfunc() does

		read_lock(&tasklist_lock);
		for_each_process_thread(p, t) {
			set_tsk_thread_flag(t, TIF_SYSCALL_TRACEPOINT);
		}
		read_unlock(&tasklist_lock);

and this can easily hit a TASK_DEAD thread with ->stack == NULL.

And we can't free/nullify it when the parent/debuger reaps a zombie,
say, mark_oom_victim() expects that get_task_struct() protects
thread_info as well.

Oleg.

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

#1429971

On Thu, Jun 23, 2016 at 7:31 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>
> I didn't see the patches yet, quite possibly I misunderstood... But no,
> I don't this we can do this (if we are not going to move ti->flags to
> task_struct at least).

Argh. Yes, ti->flags is used by others. Everything else should be
thread-synchronous, but there's ti->flags.

(And if we get scheduled, the thread-synchronous things will matter, of course):

> Yes, but the problem is that a zombie thread can do its last schedule
> before it is reaped.

Worse, the wait sequence will definitely look at it.

But that does bring up another possibility: do it at wait() time, when
we do release_thread(). That's when we *used* to synchronously free
it, before we did the lockless RCU walks.

At that point, it has been removed from all the thread lists. So the
only way to find it is through the RCU walks. Do any of *those* touch
ti->flags? I'm not seeing it, and it sounds fixable if any do.

If we could release the thread stack in release_thread(), that would be good.

Andy - I bet you can at least test it.

            Linus

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

#1429982

On Thu, Jun 23, 2016 at 9:30 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jun 23, 2016 at 7:31 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>>
>> I didn't see the patches yet, quite possibly I misunderstood... But no,
>> I don't this we can do this (if we are not going to move ti->flags to
>> task_struct at least).
>
> Argh. Yes, ti->flags is used by others. Everything else should be
> thread-synchronous, but there's ti->flags.
>
> (And if we get scheduled, the thread-synchronous things will matter, of course):
>
>> Yes, but the problem is that a zombie thread can do its last schedule
>> before it is reaped.
>
> Worse, the wait sequence will definitely look at it.
>
> But that does bring up another possibility: do it at wait() time, when
> we do release_thread(). That's when we *used* to synchronously free
> it, before we did the lockless RCU walks.
>
> At that point, it has been removed from all the thread lists. So the
> only way to find it is through the RCU walks. Do any of *those* touch
> ti->flags? I'm not seeing it, and it sounds fixable if any do.
>
> If we could release the thread stack in release_thread(), that would be good.
>
> Andy - I bet you can at least test it.

That sounds a bit more fragile than I'm really comfortable with,
although it'll at least oops reliably if we get it wrong.

But I'm planning on moving ti->flags (and the rest of thread_info,
either piecemeal or as a unit) into task_struct on architectures that
opt in, which, as a practical matter, hopefully means everyone who
opts in to virtual stacks.  So I'm more inclined make all the changes
in a different order:

1. Virtually mapped stacks (off by default but merged for testing,
possibly with a warning that distros shouldn't enable it yet.)

2. thread_info cleanup (which I want to do *anyway* because it's
critical to get the full hardening benefit)

3. Free stacks immediately and cache them (really easy).

This has the benefit of being much less dependent on who access what
field when and it should perform well with no churn.  I'm hoping to
have the thread_info stuff done in time for 4.8, too.

--Andy

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

#1429998 — Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

On 06/23, Andy Lutomirski wrote:
>
> That sounds a bit more fragile than I'm really comfortable with,
> although it'll at least oops reliably if we get it wrong.
>
> But I'm planning on moving ti->flags (and the rest of thread_info,
> either piecemeal or as a unit) into task_struct on architectures that
> opt in,

I agree, this looks better. probably it should not be that hard to fix
GET_THREAD_INFO/etc, but you know this much better than me.

Oleg.

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

#1429991 — Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

On 06/23, Linus Torvalds wrote:
>
> But that does bring up another possibility: do it at wait() time, when
> we do release_thread(). That's when we *used* to synchronously free
> it, before we did the lockless RCU walks.

Let me quote my previous email ;)

	And we can't free/nullify it when the parent/debuger reaps a zombie,
	say, mark_oom_victim() expects that get_task_struct() protects
	thread_info as well.

probably we can fix all such users though...

> At that point, it has been removed from all the thread lists. So the
> only way to find it is through the RCU walks. Do any of *those* touch
> ti->flags? I'm not seeing it,

Neither me, although I didn't try to grep too much.

> and it sounds fixable if any do

probably yes, but this would mean that tasklist_lock protects task->stack,
doesn't look really nice...

Oleg.

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

#1427514

On Monday, June 20, 2016 4:43:30 PM CEST Andy Lutomirski wrote:
> 
> On my laptop, this adds about 1.5µs of overhead to task creation,
> which seems to be mainly caused by vmalloc inefficiently allocating
> individual pages even when a higher-order page is available on the
> freelist.

Would it help to have a fixed virtual address for the stack instead
and map the current stack to that during a task switch, similar to
how we handle fixmap pages?

That would of course trade the allocation overhead for a task switch
overhead, which may be better or worse. It would also give "current"
a constant address, which may give a small performance advantage
but may also introduce a new attack vector unless we randomize it
again.

	Arnd

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

#1427992

On Tue, Jun 21, 2016 at 2:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday, June 20, 2016 4:43:30 PM CEST Andy Lutomirski wrote:
>>
>> On my laptop, this adds about 1.5µs of overhead to task creation,
>> which seems to be mainly caused by vmalloc inefficiently allocating
>> individual pages even when a higher-order page is available on the
>> freelist.
>
> Would it help to have a fixed virtual address for the stack instead
> and map the current stack to that during a task switch, similar to
> how we handle fixmap pages?
>
> That would of course trade the allocation overhead for a task switch
> overhead, which may be better or worse. It would also give "current"
> a constant address, which may give a small performance advantage
> but may also introduce a new attack vector unless we randomize it
> again.

Right: we don't want a fixed address. That makes attacks WAY easier.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

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

#1428026 — Re: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

On Tue, Jun 21, 2016 at 11:02 AM, Rik van Riel <riel@redhat.com> wrote:
> On Tue, 2016-06-21 at 10:16 -0700, Kees Cook wrote:
>> On Tue, Jun 21, 2016 at 2:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >
>> > On Monday, June 20, 2016 4:43:30 PM CEST Andy Lutomirski wrote:
>> > >
>> > >
>> > > On my laptop, this adds about 1.5µs of overhead to task creation,
>> > > which seems to be mainly caused by vmalloc inefficiently
>> > > allocating
>> > > individual pages even when a higher-order page is available on
>> > > the
>> > > freelist.
>> > Would it help to have a fixed virtual address for the stack instead
>> > and map the current stack to that during a task switch, similar to
>> > how we handle fixmap pages?
>> >
>> > That would of course trade the allocation overhead for a task
>> > switch
>> > overhead, which may be better or worse. It would also give
>> > "current"
>> > a constant address, which may give a small performance advantage
>> > but may also introduce a new attack vector unless we randomize it
>> > again.
>> Right: we don't want a fixed address. That makes attacks WAY easier.
>
> Does that imply we might want the per-cpu cache of
> these stacks to be larger than one, in order to
> introduce some more randomness after an attacker
> crashed an ASLRed program looking for ROP gadgets,
> and the next one is spawned? :)

This is the kernel stack, so this only really matters if there's some
attack in which you OOPS but learn the kernel stack address in the
process and then reuse that stack.  So... maybe?

--Andy

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

#1428029 — Re: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

[Multipart message — attachments visible in raw view] — view raw

On Tue, 2016-06-21 at 10:16 -0700, Kees Cook wrote:
> On Tue, Jun 21, 2016 at 2:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > On Monday, June 20, 2016 4:43:30 PM CEST Andy Lutomirski wrote:
> > > 
> > > 
> > > On my laptop, this adds about 1.5µs of overhead to task creation,
> > > which seems to be mainly caused by vmalloc inefficiently
> > > allocating
> > > individual pages even when a higher-order page is available on
> > > the
> > > freelist.
> > Would it help to have a fixed virtual address for the stack instead
> > and map the current stack to that during a task switch, similar to
> > how we handle fixmap pages?
> > 
> > That would of course trade the allocation overhead for a task
> > switch
> > overhead, which may be better or worse. It would also give
> > "current"
> > a constant address, which may give a small performance advantage
> > but may also introduce a new attack vector unless we randomize it
> > again.
> Right: we don't want a fixed address. That makes attacks WAY easier.

Does that imply we might want the per-cpu cache of
these stacks to be larger than one, in order to
introduce some more randomness after an attacker
crashed an ASLRed program looking for ROP gadgets,
and the next one is spawned? :)

-- 
All Rights Reversed.

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

#1428111

On Tue, Jun 21, 2016 at 12:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, June 21, 2016 10:16:21 AM CEST Kees Cook wrote:
>> On Tue, Jun 21, 2016 at 2:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Monday, June 20, 2016 4:43:30 PM CEST Andy Lutomirski wrote:
>> >>
>> >> On my laptop, this adds about 1.5µs of overhead to task creation,
>> >> which seems to be mainly caused by vmalloc inefficiently allocating
>> >> individual pages even when a higher-order page is available on the
>> >> freelist.
>> >
>> > Would it help to have a fixed virtual address for the stack instead
>> > and map the current stack to that during a task switch, similar to
>> > how we handle fixmap pages?
>> >
>> > That would of course trade the allocation overhead for a task switch
>> > overhead, which may be better or worse. It would also give "current"
>> > a constant address, which may give a small performance advantage
>> > but may also introduce a new attack vector unless we randomize it
>> > again.
>>
>> Right: we don't want a fixed address. That makes attacks WAY easier.
>
> Do we care about making the address more random then? When I look
> at /proc/vmallocinfo, I see that allocations are all using
> consecutive addresses, so if you can figure out the virtual
> address of the stack for one process that would give you a good
> chance of guessing the address for the next pid.

Quite possibly.  We should seriously consider at least randomizing the
*start* of the vmalloc area, at least on 64-bit architectures.

--Andy

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

#1428130

On Tue, Jun 21, 2016 at 12:47 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Tue, Jun 21, 2016 at 12:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Tuesday, June 21, 2016 10:16:21 AM CEST Kees Cook wrote:
>>> On Tue, Jun 21, 2016 at 2:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> > On Monday, June 20, 2016 4:43:30 PM CEST Andy Lutomirski wrote:
>>> >>
>>> >> On my laptop, this adds about 1.5µs of overhead to task creation,
>>> >> which seems to be mainly caused by vmalloc inefficiently allocating
>>> >> individual pages even when a higher-order page is available on the
>>> >> freelist.
>>> >
>>> > Would it help to have a fixed virtual address for the stack instead
>>> > and map the current stack to that during a task switch, similar to
>>> > how we handle fixmap pages?
>>> >
>>> > That would of course trade the allocation overhead for a task switch
>>> > overhead, which may be better or worse. It would also give "current"
>>> > a constant address, which may give a small performance advantage
>>> > but may also introduce a new attack vector unless we randomize it
>>> > again.
>>>
>>> Right: we don't want a fixed address. That makes attacks WAY easier.
>>
>> Do we care about making the address more random then? When I look
>> at /proc/vmallocinfo, I see that allocations are all using
>> consecutive addresses, so if you can figure out the virtual
>> address of the stack for one process that would give you a good
>> chance of guessing the address for the next pid.
>
> Quite possibly.  We should seriously consider at least randomizing the
> *start* of the vmalloc area, at least on 64-bit architectures.

Yup, this is already under way for x86. Thomas Garnier has a series
that he's been working on:

http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=kaslr/memory

I'd love to see similar for other architectures too.

Thomas just sent me an updated series I'll be putting up for review later today.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

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

#1428112

On Tuesday, June 21, 2016 10:16:21 AM CEST Kees Cook wrote:
> On Tue, Jun 21, 2016 at 2:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday, June 20, 2016 4:43:30 PM CEST Andy Lutomirski wrote:
> >>
> >> On my laptop, this adds about 1.5µs of overhead to task creation,
> >> which seems to be mainly caused by vmalloc inefficiently allocating
> >> individual pages even when a higher-order page is available on the
> >> freelist.
> >
> > Would it help to have a fixed virtual address for the stack instead
> > and map the current stack to that during a task switch, similar to
> > how we handle fixmap pages?
> >
> > That would of course trade the allocation overhead for a task switch
> > overhead, which may be better or worse. It would also give "current"
> > a constant address, which may give a small performance advantage
> > but may also introduce a new attack vector unless we randomize it
> > again.
> 
> Right: we don't want a fixed address. That makes attacks WAY easier.

Do we care about making the address more random then? When I look
at /proc/vmallocinfo, I see that allocations are all using
consecutive addresses, so if you can figure out the virtual
address of the stack for one process that would give you a good
chance of guessing the address for the next pid.

	Arnd

[toc] | [prev] | [standalone]

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

Back to top | Article view | linux.kernel

csiph-web