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

Groups > linux.kernel > #1440886 > unrolled thread

[PATCH v5 00/32] virtually mapped stacks and thread_info cleanup

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 00/32] virtually mapped stacks and thread_info cleanup Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 18/32] signal: Consolidate {TS,TLF}_RESTORE_SIGMASK code Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
      Re: [PATCH v5 18/32] signal: Consolidate {TS,TLF}_RESTORE_SIGMASK code Brian Gerst <brgerst@gmail.com> - 2016-07-12 14:00 +0200
        Re: [PATCH v5 18/32] signal: Consolidate {TS,TLF}_RESTORE_SIGMASK code Andy Lutomirski <luto@amacapital.net> - 2016-07-13 01:10 +0200
    [PATCH v5 08/32] dma-api: Teach the "DMA-from-stack" check about vmapped stacks Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 32/32] fork: Cache two thread stacks per cpu if CONFIG_VMAP_STACK is set Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 27/32] x86: Move thread_info into task_struct Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 31/32] sched: Free the stack early if CONFIG_THREAD_INFO_IN_TASK Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 23/32] printk: When dumping regs, show the stack, not thread_info Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 19/32] x86/smp: Remove stack_smp_processor_id() Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 20/32] x86/smp: Remove unnecessary initialization of thread_info::cpu Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 16/32] x86: Move uaccess_err and sig_on_uaccess_err to thread_struct Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:00 +0200
    [PATCH v5 04/32] x86/mm: Remove kernel_unmap_pages_in_pgd() and efi_cleanup_page_tables() Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    [PATCH v5 01/32] bluetooth: Switch SMP to crypto_cipher_encrypt_one() Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
      Re: [PATCH v5 01/32] bluetooth: Switch SMP to crypto_cipher_encrypt_one() Andy Lutomirski <luto@amacapital.net> - 2016-07-14 21:20 +0200
        Re: [PATCH v5 01/32] bluetooth: Switch SMP to crypto_cipher_encrypt_one() Marcel Holtmann <marcel@holtmann.org> - 2016-07-14 22:40 +0200
        Re: [PATCH v5 01/32] bluetooth: Switch SMP to  crypto_cipher_encrypt_one() David Miller <davem@davemloft.net> - 2016-07-14 22:50 +0200
    [PATCH v5 05/32] mm: Track NR_KERNEL_STACK in KiB instead of number of stacks Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    [PATCH v5 12/32] x86/dumpstack/64: Handle faults when printing the "Stack:" part of an OOPS Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    [PATCH v5 06/32] mm: Fix memcg stack accounting for sub-page stacks Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
      Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated Valdis.Kletnieks@vt.edu - 2016-07-22 06:50 +0200
        Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd,  don't set the pgd entry until it's populated Andy Lutomirski <luto@kernel.org> - 2016-07-22 07:40 +0200
          Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd,  don't set the pgd entry until it's populated Ingo Molnar <mingo@kernel.org> - 2016-07-22 12:30 +0200
            Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd,  don't set the pgd entry until it's populated Andy Lutomirski <luto@amacapital.net> - 2016-07-22 20:30 +0200
              Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd,  don't set the pgd entry until it's populated Andy Lutomirski <luto@amacapital.net> - 2016-07-22 20:40 +0200
              Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd,  don't set the pgd entry until it's populated Ingo Molnar <mingo@kernel.org> - 2016-07-22 22:20 +0200
          Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated Nicolai Stange <nicstange@gmail.com> - 2016-07-23 17:00 +0200
    [PATCH v5 10/32] x86/dumpstack: Honor supplied @regs arg Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    [PATCH v5 02/32] x86/mm/hotplug: Don't remove PGD entries in remove_pagetable() Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    [PATCH v5 15/32] x86/mm: Improve stack-overflow #PF handling Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    [PATCH v5 11/32] x86/dumpstack: Try harder to get a call trace on stack overflow Andy Lutomirski <luto@kernel.org> - 2016-07-11 23:10 +0200
    Re: [PATCH v5 00/32] virtually mapped stacks and thread_info cleanup Herbert Xu <herbert@gondor.apana.org.au> - 2016-07-12 11:00 +0200
    Re: [PATCH v5 00/32] virtually mapped stacks and thread_info cleanup Christian Borntraeger <borntraeger@de.ibm.com> - 2016-07-13 11:00 +0200
      Re: [PATCH v5 00/32] virtually mapped stacks and thread_info cleanup Andy Lutomirski <luto@amacapital.net> - 2016-07-13 20:40 +0200
        Re: [PATCH v5 00/32] virtually mapped stacks and thread_info cleanup Christian Borntraeger <borntraeger@de.ibm.com> - 2016-07-13 21:00 +0200

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

#1440902 — [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

This avoids pointless races in which another CPU or task might see a
partially populated global pgd entry.  These races should normally
be harmless, but, if another CPU propagates the entry via
vmalloc_fault and then populate_pgd fails (due to memory allocation
failure, for example), this prevents a use-after-free of the pgd
entry.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/mm/pageattr.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 7a1f7bbf4105..6088aa03de63 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -1104,8 +1104,6 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
 		pud = (pud_t *)get_zeroed_page(GFP_KERNEL | __GFP_NOTRACK);
 		if (!pud)
 			return -1;
-
-		set_pgd(pgd_entry, __pgd(__pa(pud) | _KERNPG_TABLE));
 	}
 
 	pgprot_val(pgprot) &= ~pgprot_val(cpa->mask_clr);
@@ -1113,11 +1111,16 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
 
 	ret = populate_pud(cpa, addr, pgd_entry, pgprot);
 	if (ret < 0) {
-		unmap_pgd_range(cpa->pgd, addr,
+		if (pud)
+			free_page((unsigned long)pud);
+		unmap_pud_range(pgd_entry, addr,
 				addr + (cpa->numpages << PAGE_SHIFT));
 		return ret;
 	}
 
+	if (pud)
+		set_pgd(pgd_entry, __pgd(__pa(pud) | _KERNPG_TABLE));
+
 	cpa->numpages = ret;
 	return 0;
 }
-- 
2.7.4

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

#1448396 — Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

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

On Mon, 11 Jul 2016 13:53:36 -0700, Andy Lutomirski said:
> This avoids pointless races in which another CPU or task might see a
> partially populated global pgd entry.  These races should normally
> be harmless, but, if another CPU propagates the entry via
> vmalloc_fault and then populate_pgd fails (due to memory allocation
> failure, for example), this prevents a use-after-free of the pgd
> entry.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/mm/pageattr.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

I just bisected a failure to boot down to this patch.  On my Dell Latitude
laptop, it results in the kernel being loaded and then just basically sitting
there dead in the water - as far as I can tell, it dies before the kernel
ever gets going far enough to do any console I/O (even with ignore_loglevel).
Nothing in /sys/fs/pstore either.  I admit not understanding the VM code
at all, so I don't have a clue *why* this causes indigestion...

CPU is an Intel Core i5-3340M in case that matters....

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

#1448401 — Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

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

On 07/21/2016 09:43 PM, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 11 Jul 2016 13:53:36 -0700, Andy Lutomirski said:
>> This avoids pointless races in which another CPU or task might see a
>> partially populated global pgd entry.  These races should normally
>> be harmless, but, if another CPU propagates the entry via
>> vmalloc_fault and then populate_pgd fails (due to memory allocation
>> failure, for example), this prevents a use-after-free of the pgd
>> entry.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/mm/pageattr.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> I just bisected a failure to boot down to this patch.  On my Dell Latitude
> laptop, it results in the kernel being loaded and then just basically sitting
> there dead in the water - as far as I can tell, it dies before the kernel
> ever gets going far enough to do any console I/O (even with ignore_loglevel).
> Nothing in /sys/fs/pstore either.  I admit not understanding the VM code
> at all, so I don't have a clue *why* this causes indigestion...
>
> CPU is an Intel Core i5-3340M in case that matters....
>

How much memory do you have and what's your config?  My code is 
obviously buggy, but I'm wondering why neither I nor the 0day bot caught 
this.

The attached patch is compile-tested only.  (Even Thunderbird doesn't 
want to send non-flowed text right now, sigh.)

--Andy

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

#1448523 — Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

* Andy Lutomirski <luto@kernel.org> wrote:

> On 07/21/2016 09:43 PM, Valdis.Kletnieks@vt.edu wrote:
> >On Mon, 11 Jul 2016 13:53:36 -0700, Andy Lutomirski said:
> >>This avoids pointless races in which another CPU or task might see a
> >>partially populated global pgd entry.  These races should normally
> >>be harmless, but, if another CPU propagates the entry via
> >>vmalloc_fault and then populate_pgd fails (due to memory allocation
> >>failure, for example), this prevents a use-after-free of the pgd
> >>entry.
> >>
> >>Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >>---
> >> arch/x86/mm/pageattr.c | 9 ++++++---
> >> 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> >I just bisected a failure to boot down to this patch.  On my Dell Latitude
> >laptop, it results in the kernel being loaded and then just basically sitting
> >there dead in the water - as far as I can tell, it dies before the kernel
> >ever gets going far enough to do any console I/O (even with ignore_loglevel).
> >Nothing in /sys/fs/pstore either.  I admit not understanding the VM code
> >at all, so I don't have a clue *why* this causes indigestion...
> >
> >CPU is an Intel Core i5-3340M in case that matters....
> >
> 
> How much memory do you have and what's your config?  My code is obviously
> buggy, but I'm wondering why neither I nor the 0day bot caught this.
> 
> The attached patch is compile-tested only.  (Even Thunderbird doesn't want
> to send non-flowed text right now, sigh.)
> 
> --Andy

> From 6589ddf69a1369e1ecb95f0af489d90b980e256e Mon Sep 17 00:00:00 2001
> Message-Id: <6589ddf69a1369e1ecb95f0af489d90b980e256e.1469165371.git.luto@kernel.org>
> From: Andy Lutomirski <luto@kernel.org>
> Date: Thu, 21 Jul 2016 22:22:02 -0700
> Subject: [PATCH] x86/mm: Fix populate_pgd()
> 
> I make an obvious error in populate_pgd() -- it would fail to correctly
> populate the page tables when it allocated a new pud page.

JFYI, on allnoconfig it gives:

  arch/x86/mm/pageattr.c:1016:20: error: implicit declaration of function ‘pud_index’ [-Werror=implicit-function-declaration]

Thanks,

	Ingo

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

#1448716 — Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

On Fri, Jul 22, 2016 at 3:21 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Andy Lutomirski <luto@kernel.org> wrote:
>
>> On 07/21/2016 09:43 PM, Valdis.Kletnieks@vt.edu wrote:
>> >On Mon, 11 Jul 2016 13:53:36 -0700, Andy Lutomirski said:
>> >>This avoids pointless races in which another CPU or task might see a
>> >>partially populated global pgd entry.  These races should normally
>> >>be harmless, but, if another CPU propagates the entry via
>> >>vmalloc_fault and then populate_pgd fails (due to memory allocation
>> >>failure, for example), this prevents a use-after-free of the pgd
>> >>entry.
>> >>
>> >>Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> >>---
>> >> arch/x86/mm/pageattr.c | 9 ++++++---
>> >> 1 file changed, 6 insertions(+), 3 deletions(-)
>> >
>> >I just bisected a failure to boot down to this patch.  On my Dell Latitude
>> >laptop, it results in the kernel being loaded and then just basically sitting
>> >there dead in the water - as far as I can tell, it dies before the kernel
>> >ever gets going far enough to do any console I/O (even with ignore_loglevel).
>> >Nothing in /sys/fs/pstore either.  I admit not understanding the VM code
>> >at all, so I don't have a clue *why* this causes indigestion...
>> >
>> >CPU is an Intel Core i5-3340M in case that matters....
>> >
>>
>> How much memory do you have and what's your config?  My code is obviously
>> buggy, but I'm wondering why neither I nor the 0day bot caught this.
>>
>> The attached patch is compile-tested only.  (Even Thunderbird doesn't want
>> to send non-flowed text right now, sigh.)
>>
>> --Andy
>
>> From 6589ddf69a1369e1ecb95f0af489d90b980e256e Mon Sep 17 00:00:00 2001
>> Message-Id: <6589ddf69a1369e1ecb95f0af489d90b980e256e.1469165371.git.luto@kernel.org>
>> From: Andy Lutomirski <luto@kernel.org>
>> Date: Thu, 21 Jul 2016 22:22:02 -0700
>> Subject: [PATCH] x86/mm: Fix populate_pgd()
>>
>> I make an obvious error in populate_pgd() -- it would fail to correctly
>> populate the page tables when it allocated a new pud page.
>
> JFYI, on allnoconfig it gives:
>
>   arch/x86/mm/pageattr.c:1016:20: error: implicit declaration of function ‘pud_index’ [-Werror=implicit-function-declaration]

As it happens, my fix interacts badly with the steaming pile of crap
that is Linux's support for <4 page table levels.  Can you just revert
the offending patch and I'll redo it differently?

<rant>
Holy crap the pagetable structures and helpers suck.  We have "pgd_t
*" that could point to a top-level entry, a top-level table, or to
something else entirely if we have fewer than four levels.  And we
have pud_t * that ambiguously points to a table or to an entry or to a
pmd if the build is feeling daft.  We have a helper called
"pud_offset" that doesn't compute any sort of offset -- it *traverses*
one level of the table and only works if you pass it the kind of pgt_t
* that points to an entry (not a table).

This garbage (as evidenced by my bug and my failed attempt to fix it)
only works if you never have a low-level page table that isn't linked
into a higher-level page table, and it mostly requires you to do
everything exactly the way it was originally done so all the horrible
inline helpers don't get confused.

And AFAICT all of this was done to manually unroll a loop, and I bet
it never sped anything up measurably even on 386 or PPro.

Whenever some vendor releases a 5 level page table CPU, can we
*please* clean this up first?  We should have a type that points to a
table, a different type that points to an entry (or maybe not have
pointers to entries at all), and the levels should be referred to by
*number*.  When you need to traverse all the way down, you write a
*loop* instead of four bloody helper functions, some of which are
incomprehensibly no-ops on some kernels.  And if this means that, on
Intel, we have a silly branch in the inner loop because the bottom
level entry format is special, who cares?
</rant>

--Andy

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

#1448723 — Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

On Fri, Jul 22, 2016 at 11:21 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Jul 22, 2016 at 3:21 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>
>> * Andy Lutomirski <luto@kernel.org> wrote:
>>
>>> On 07/21/2016 09:43 PM, Valdis.Kletnieks@vt.edu wrote:
>>> >On Mon, 11 Jul 2016 13:53:36 -0700, Andy Lutomirski said:
>>> >>This avoids pointless races in which another CPU or task might see a
>>> >>partially populated global pgd entry.  These races should normally
>>> >>be harmless, but, if another CPU propagates the entry via
>>> >>vmalloc_fault and then populate_pgd fails (due to memory allocation
>>> >>failure, for example), this prevents a use-after-free of the pgd
>>> >>entry.
>>> >>
>>> >>Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>> >>---
>>> >> arch/x86/mm/pageattr.c | 9 ++++++---
>>> >> 1 file changed, 6 insertions(+), 3 deletions(-)
>>> >
>>> >I just bisected a failure to boot down to this patch.  On my Dell Latitude
>>> >laptop, it results in the kernel being loaded and then just basically sitting
>>> >there dead in the water - as far as I can tell, it dies before the kernel
>>> >ever gets going far enough to do any console I/O (even with ignore_loglevel).
>>> >Nothing in /sys/fs/pstore either.  I admit not understanding the VM code
>>> >at all, so I don't have a clue *why* this causes indigestion...
>>> >
>>> >CPU is an Intel Core i5-3340M in case that matters....
>>> >
>>>
>>> How much memory do you have and what's your config?  My code is obviously
>>> buggy, but I'm wondering why neither I nor the 0day bot caught this.
>>>
>>> The attached patch is compile-tested only.  (Even Thunderbird doesn't want
>>> to send non-flowed text right now, sigh.)
>>>
>>> --Andy
>>
>>> From 6589ddf69a1369e1ecb95f0af489d90b980e256e Mon Sep 17 00:00:00 2001
>>> Message-Id: <6589ddf69a1369e1ecb95f0af489d90b980e256e.1469165371.git.luto@kernel.org>
>>> From: Andy Lutomirski <luto@kernel.org>
>>> Date: Thu, 21 Jul 2016 22:22:02 -0700
>>> Subject: [PATCH] x86/mm: Fix populate_pgd()
>>>
>>> I make an obvious error in populate_pgd() -- it would fail to correctly
>>> populate the page tables when it allocated a new pud page.
>>
>> JFYI, on allnoconfig it gives:
>>
>>   arch/x86/mm/pageattr.c:1016:20: error: implicit declaration of function ‘pud_index’ [-Werror=implicit-function-declaration]
>
> As it happens, my fix interacts badly with the steaming pile of crap
> that is Linux's support for <4 page table levels.  Can you just revert
> the offending patch and I'll redo it differently?

No, don't revert it.  The result doesn't work.  I'll send something.

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

#1448757 — Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

* Andy Lutomirski <luto@amacapital.net> wrote:

> This garbage (as evidenced by my bug and my failed attempt to fix it)
> only works if you never have a low-level page table that isn't linked
> into a higher-level page table, and it mostly requires you to do
> everything exactly the way it was originally done so all the horrible
> inline helpers don't get confused.
> 
> And AFAICT all of this was done to manually unroll a loop, and I bet
> it never sped anything up measurably even on 386 or PPro.
> 
> Whenever some vendor releases a 5 level page table CPU, can we
> *please* clean this up first?  We should have a type that points to a
> table, a different type that points to an entry (or maybe not have
> pointers to entries at all), and the levels should be referred to by
> *number*.  When you need to traverse all the way down, you write a
> *loop* instead of four bloody helper functions, some of which are
> incomprehensibly no-ops on some kernels.  And if this means that, on
> Intel, we have a silly branch in the inner loop because the bottom
> level entry format is special, who cares?
> </rant>

I'd welcome (and help out!) any effort to clean it up gradually.

Thanks,

	Ingo

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

#1448955 — Re: [kernel-hardening] [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated

Valdis.Kletnieks@vt.edu writes:

> On Thu, 21 Jul 2016 22:34:33 -0700, Andy Lutomirski said:
>
>> How much memory do you have and what's your config?  My code is
>> obviously buggy, but I'm wondering why neither I nor the 0day bot caught
>> this.
>
> Probably because your devel box and the 0day bot both have 4-level page
> tables and the dual-core i5 in my laptop has (presumably) 3?
>
> In any case, your patch didn't fix things, nor did (as you noted in a mail
> to Ingo) does reverting the problem commit (and then the following one that
> deletes now-dead code so it will compile cleanly).


Applying the patch directly on top of 360cb4d15567 ("x86/mm/cpa: In
populate_pgd(), don't set the PGD entry until it's populated") *does*
fix things for me.

Hardware: i7-4800MQ, 8GiB RAM, Dell Latitude E6540

FYI, the kernel panic grabbed via console=uart,io,0x3f8,... is

BUG: unable to handle kernel paging request at ffffb92ac0000fc0
IP: [<ffffffff8106b8d1>] native_set_pmd+0x1/0x10
PGD 0 
Oops: 0002 [#1] SMP
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc6+ #190
Hardware name: Dell Inc. Latitude E6540/0725FP, BIOS A10 06/26/2014
task: ffffffff81e0d580 ti: ffffffff81e00000 task.ti: ffffffff81e00000
RIP: 0010:[<ffffffff8106b8d1>]  [<ffffffff8106b8d1>] native_set_pmd+0x1/0x10
RSP: 0000:ffffffff81e03c38  EFLAGS: 00010206
RAX: 00000000ff0000f3 RBX: 00000000ff000000 RCX: ffff880000000000
RDX: ffffb92ac0000fc0 RSI: 00000000ff0000f3 RDI: ffffb92ac0000fc0
RBP: ffffffff81e03c90 R08: ffff880000000fc0 R09: 0000000000000073
R10: ffff88022ede5000 R11: 0000000000000001 R12: ffffffff81e03e48
R13: 0000000001000000 R14: 0000000000000073 R15: ffff880000000018
FS:  0000000000000000(0000) GS:ffff88022ea00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffb92ac0000fc0 CR3: 0000000001e06000 CR4: 00000000000406b0
Stack:
 ffffffff81e03c90 ffffffff8107217f 0000000000000073 0000000100000000
 0000000000000001 0000000000001000 ffff880000000018 0000000000001000
 ffffffff81e03e48 0000000100000000 ffffffffff2018a8 ffffffff81e03d08
Call Trace:
 [<ffffffff8107217f>] ? populate_pmd+0x11f/0x2c0
 [<ffffffff81072823>] __cpa_process_fault+0x503/0x5d0
 [<ffffffff81073223>] __change_page_attr_set_clr+0x563/0xe00
 [<ffffffff81074e6f>] kernel_map_pages_in_pgd+0x8f/0xd0
 [<ffffffff81fa5e2e>] __map_region+0x3c/0x58
 [<ffffffff81fa6064>] efi_map_region+0x31/0xca
 [<ffffffff81fa5af3>] efi_enter_virtual_mode+0x215/0x4bd
 [<ffffffff814c6289>] ? acpi_os_signal_semaphore+0x2c/0x38
 [<ffffffff814f5c4a>] ? acpi_ut_initialize_interfaces+0x62/0x67
 [<ffffffff81f84f78>] start_kernel+0x3cf/0x478
 [<ffffffff81f84120>] ? early_idt_handler_array+0x120/0x120
 [<ffffffff81f842db>] x86_64_start_reservations+0x2f/0x31
 [<ffffffff81f84429>] x86_64_start_kernel+0x14c/0x16f
Code: 89 e5 48 89 47 04 5d c3 66 90 55 48 89 e5 0f 01 f8 5d c3 0f 1f 80 00 00 00 00 55 48 89 37 48 89 e5 5d c3 0f 1f 80 00 00 00 00 55 <48> 89 37 48 89 e5 5d c3 0f 1f 80 00 00 00 00 55 48 89 37 48 89 
RIP  [<ffffffff8106b8d1>] native_set_pmd+0x1/0x10
 RSP <ffffffff81e03c38>
CR2: ffffb92ac0000fc0
---[ end trace 2f8154f277751049 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
---[ end Kernel panic - not syncing: Attempted to kill the idle task!


The reason the patch didn't work for Valdis might be that there is
another issue in next-20150722 with the same symptoms (provided you
don't watch the serial console). Valdis, did you apply the provided
patch on top of next?

The "other issue" is:

RDX: 0000000000000010 RSI: 00000000000306c3 RDI: ffff88003bdea2fc
RBP: ffffffffb6e03a70 R08: ffff88003bdea000 R09: 0000000000000000
R10: ffffffffb713d3a0 R11: 0000000000000008 R12: 0000000000000020
R13: ffff88003bdea2fc R14: ffffffffb6e03a80 R15: ffffffffb6e03ea0
FS:  0000000000000000(0000) GS:ffff9208aea00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff88003bdea300 CR3: 00000001dce06000 CR4: 00000000000406b0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Stack:
 ffffffffb6054cea 0000000000000000 0000000100000000 0000000000000001
 0000000000000000 0000000000000000 ffffffffb705c2e0 000000003fffc000
 ffffffffb6e03e90 ffffffffb6055487 ffff88003bdea2fc ffffffffb6e0d580
Call Trace:
 [<ffffffffb6054cea>] ? find_microcode_patch+0x4a/0xa0
 [<ffffffffb6055487>] load_microcode.isra.1.constprop.12+0x37/0xa0
 [<ffffffffb6036700>] ? dump_trace+0x120/0x320
 [<ffffffffb644fee8>] ? put_dec+0x18/0xa0
 [<ffffffffb645025d>] ? number+0x2ed/0x300
 [<ffffffffb6ff3ba1>] ? serial_putc+0x1e/0x2d
 [<ffffffffb6ff3b83>] ? serial8250_early_out+0x62/0x62
 [<ffffffffb654f127>] ? uart_console_write+0x57/0x70
 [<ffffffffb61094ad>] ? trace_hardirqs_off+0xd/0x10
 [<ffffffffb6152775>] ? __module_address+0x5/0xf0
 [<ffffffffb6152872>] ? __module_text_address+0x12/0x60
 [<ffffffffb61967e4>] ? is_ftrace_trampoline+0x44/0x70
 [<ffffffffb60d68a6>] ? __kernel_text_address+0x56/0x70
 [<ffffffffb60371bb>] ? print_context_stack+0x7b/0x100
 [<ffffffffb6109695>] ? __bfs+0x25/0x280
 [<ffffffffb61967e4>] ? is_ftrace_trampoline+0x44/0x70
 [<ffffffffb6152775>] ? __module_address+0x5/0xf0
 [<ffffffffb6152872>] ? __module_text_address+0x12/0x60
 [<ffffffffb61967e4>] ? is_ftrace_trampoline+0x44/0x70
 [<ffffffffb60d68a6>] ? __kernel_text_address+0x56/0x70
 [<ffffffffb60371bb>] ? print_context_stack+0x7b/0x100
 [<ffffffffb6036700>] ? dump_trace+0x120/0x320
 [<ffffffffb644fee8>] ? put_dec+0x18/0xa0
 [<ffffffffb645025d>] ? number+0x2ed/0x300
 [<ffffffffb6ff3ba1>] ? serial_putc+0x1e/0x2d
 [<ffffffffb6ff3b83>] ? serial8250_early_out+0x62/0x62
 [<ffffffffb654f127>] ? uart_console_write+0x57/0x70
 [<ffffffffb61094ad>] ? trace_hardirqs_off+0xd/0x10
 [<ffffffffb61094ad>] ? trace_hardirqs_off+0xd/0x10
 [<ffffffffb689de84>] ? _raw_spin_unlock_irqrestore+0x54/0x60
 [<ffffffffb611f16d>] ? console_unlock+0x33d/0x670
 [<ffffffffb611f7a1>] ? vprintk_emit+0x301/0x5e0
 [<ffffffffb605553f>] ? collect_cpu_info_early+0x4f/0x140
 [<ffffffffb61ea845>] ? __pr_info+0x5a/0x76
 [<ffffffffb60557cd>] load_ucode_intel_ap+0x5d/0x80
 [<ffffffffb6054924>] load_ucode_ap+0x94/0xa0
 [<ffffffffb60481a8>] cpu_init+0x58/0x3e0
 [<ffffffffb60709bc>] ? set_pte_vaddr+0x5c/0x90
 [<ffffffffb6fac06c>] trap_init+0x2b6/0x328
 [<ffffffffb6fa0dba>] start_kernel+0x224/0x47f
 [<ffffffffb6fa0120>] ? early_idt_handler_array+0x120/0x120
 [<ffffffffb6fa02cf>] x86_64_start_reservations+0x29/0x2b
 [<ffffffffb6fa041e>] x86_64_start_kernel+0x14d/0x170
Code: c1 74 04 85 c2 74 e4 b8 01 00 00 00 5d c3 41 89 ca b8 01 00 00 00 41 09 d2 74 f1 85 d1 74 98 5d c3 31 c0 5d c3 90 e8 eb b1 84 00 <39> 4f 04 77 03 31 c0 c3 55 48 89 e5 e8 6a ff ff ff 5d c3 0f 1f 
RIP  [<ffffffffb6055af5>] has_newer_microcode+0x5/0x20
 RSP <ffffffffb6e03a30>
CR2: ffff88003bdea300
---[ end trace b163fd3960fd46fb ]---
Kernel panic - not syncing: Attempted to kill the idle task!
---[ end Kernel panic - not syncing: Attempted to kill the idle task!

I bisected this one to 21ef9a5c3164 ("Merge branch 'x86/microcode'"). Both
of its parents do not exhibit that behaviour.  This merge's author is
Ingo Molnar, so I added him to the CC list.


Thanks,

Nicolai

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

#1440903 — [PATCH v5 10/32] x86/dumpstack: Honor supplied @regs arg

[cherry-picked from ef16dd0c2a523d2e3975bb1bea9f5727e3e7146f to
 avoid tangling the trees too much.]

The comment suggests that show_stack(NULL, NULL) should backtrace the
current context, but the code doesn't match the comment. If regs are
given, start the "Stack:" hexdump at regs->sp.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1467671487-10344-2-git-send-email-bp@alien8.de
Link: http://lkml.kernel.org/r/efcd79bf4106d61f1cd258c2caa87f3a0618eeac.1466036668.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/dumpstack_32.c | 4 +++-
 arch/x86/kernel/dumpstack_64.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index fef917e79b9d..948d77da3881 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -96,7 +96,9 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
 	int i;
 
 	if (sp == NULL) {
-		if (task)
+		if (regs)
+			sp = (unsigned long *)regs->sp;
+		else if (task)
 			sp = (unsigned long *)task->thread.sp;
 		else
 			sp = (unsigned long *)&sp;
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index d558a8a49016..a81e1ef73bf2 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -264,7 +264,9 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
 	 * back trace for this cpu:
 	 */
 	if (sp == NULL) {
-		if (task)
+		if (regs)
+			sp = (unsigned long *)regs->sp;
+		else if (task)
 			sp = (unsigned long *)task->thread.sp;
 		else
 			sp = (unsigned long *)&sp;
-- 
2.7.4

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

#1440905 — [PATCH v5 02/32] x86/mm/hotplug: Don't remove PGD entries in remove_pagetable()

From: Ingo Molnar <mingo@kernel.org>

So when memory hotplug removes a piece of physical memory from pagetable
mappings, it also frees the underlying PGD entry.

This complicates PGD management, so don't do this. We can keep the
PGD mapped and the PUD table all clear - it's only a single 4K page
per 512 GB of memory hotplugged.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <Waiman.Long@hp.com>
Cc: linux-mm@kvack.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Message-Id: <1442903021-3893-4-git-send-email-mingo@kernel.org>
---
 arch/x86/mm/init_64.c | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index bce2e5d9edd4..c7465453d64e 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -702,27 +702,6 @@ static void __meminit free_pmd_table(pmd_t *pmd_start, pud_t *pud)
 	spin_unlock(&init_mm.page_table_lock);
 }
 
-/* Return true if pgd is changed, otherwise return false. */
-static bool __meminit free_pud_table(pud_t *pud_start, pgd_t *pgd)
-{
-	pud_t *pud;
-	int i;
-
-	for (i = 0; i < PTRS_PER_PUD; i++) {
-		pud = pud_start + i;
-		if (pud_val(*pud))
-			return false;
-	}
-
-	/* free a pud table */
-	free_pagetable(pgd_page(*pgd), 0);
-	spin_lock(&init_mm.page_table_lock);
-	pgd_clear(pgd);
-	spin_unlock(&init_mm.page_table_lock);
-
-	return true;
-}
-
 static void __meminit
 remove_pte_table(pte_t *pte_start, unsigned long addr, unsigned long end,
 		 bool direct)
@@ -913,7 +892,6 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct)
 	unsigned long addr;
 	pgd_t *pgd;
 	pud_t *pud;
-	bool pgd_changed = false;
 
 	for (addr = start; addr < end; addr = next) {
 		next = pgd_addr_end(addr, end);
@@ -924,13 +902,8 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct)
 
 		pud = (pud_t *)pgd_page_vaddr(*pgd);
 		remove_pud_table(pud, addr, next, direct);
-		if (free_pud_table(pud, pgd))
-			pgd_changed = true;
 	}
 
-	if (pgd_changed)
-		sync_global_pgds(start, end - 1, 1);
-
 	flush_tlb_all();
 }
 
-- 
2.7.4

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

#1440906 — [PATCH v5 15/32] x86/mm: Improve stack-overflow #PF handling

If we get a page fault indicating kernel stack overflow, invoke
handle_stack_overflow().  To prevent us from overflowing the stack
again while handling the overflow (because we are likely to have
very little stack space left), call handle_stack_overflow() on the
double-fault stack

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/traps.h |  6 ++++++
 arch/x86/kernel/traps.c      |  6 +++---
 arch/x86/mm/fault.c          | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index c3496619740a..01fd0a7f48cd 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -117,6 +117,12 @@ extern void ist_exit(struct pt_regs *regs);
 extern void ist_begin_non_atomic(struct pt_regs *regs);
 extern void ist_end_non_atomic(void);
 
+#ifdef CONFIG_VMAP_STACK
+void __noreturn handle_stack_overflow(const char *message,
+				      struct pt_regs *regs,
+				      unsigned long fault_address);
+#endif
+
 /* Interrupts/Exceptions */
 enum {
 	X86_TRAP_DE = 0,	/*  0, Divide-by-zero */
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index f0345284c30b..33d4e7629a42 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -293,9 +293,9 @@ DO_ERROR(X86_TRAP_SS,     SIGBUS,  "stack segment",		stack_segment)
 DO_ERROR(X86_TRAP_AC,     SIGBUS,  "alignment check",		alignment_check)
 
 #ifdef CONFIG_VMAP_STACK
-static void __noreturn handle_stack_overflow(const char *message,
-					     struct pt_regs *regs,
-					     unsigned long fault_address)
+__visible void __noreturn handle_stack_overflow(const char *message,
+						struct pt_regs *regs,
+						unsigned long fault_address)
 {
 	printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",
 		 (void *)fault_address, current->stack,
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index ca44e2e7fd00..76c58fdcfb29 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -753,6 +753,40 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 		return;
 	}
 
+#ifdef CONFIG_VMAP_STACK
+	/*
+	 * Stack overflow?  During boot, we can fault near the initial
+	 * stack in the direct map, but that's not an overflow -- check
+	 * that we're in vmalloc space to avoid this.
+	 */
+	if (is_vmalloc_addr((void *)address) &&
+	    (((unsigned long)tsk->stack - 1 - address < PAGE_SIZE) ||
+	     address - ((unsigned long)tsk->stack + THREAD_SIZE) < PAGE_SIZE)) {
+		register void *__sp asm("rsp");
+		unsigned long stack =
+			this_cpu_read(orig_ist.ist[DOUBLEFAULT_STACK]) -
+			sizeof(void *);
+		/*
+		 * We're likely to be running with very little stack space
+		 * left.  It's plausible that we'd hit this condition but
+		 * double-fault even before we get this far, in which case
+		 * we're fine: the double-fault handler will deal with it.
+		 *
+		 * We don't want to make it all the way into the oops code
+		 * and then double-fault, though, because we're likely to
+		 * break the console driver and lose most of the stack dump.
+		 */
+		asm volatile ("movq %[stack], %%rsp\n\t"
+			      "call handle_stack_overflow\n\t"
+			      "1: jmp 1b"
+			      : "+r" (__sp)
+			      : "D" ("kernel stack overflow (page fault)"),
+				"S" (regs), "d" (address),
+				[stack] "rm" (stack));
+		unreachable();
+	}
+#endif
+
 	/*
 	 * 32-bit:
 	 *
-- 
2.7.4

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

#1440907 — [PATCH v5 11/32] x86/dumpstack: Try harder to get a call trace on stack overflow

If we overflow the stack, print_context_stack will abort.  Detect
this case and rewind back into the valid part of the stack so that
we can trace it.

Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/dumpstack.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 4eefee06f6fd..de8242d8bb61 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -87,7 +87,7 @@ static inline int valid_stack_ptr(struct task_struct *task,
 		else
 			return 0;
 	}
-	return p > t && p < t + THREAD_SIZE - size;
+	return p >= t && p < t + THREAD_SIZE - size;
 }
 
 unsigned long
@@ -98,6 +98,14 @@ print_context_stack(struct task_struct *task,
 {
 	struct stack_frame *frame = (struct stack_frame *)bp;
 
+	/*
+	 * If we overflowed the stack into a guard page, jump back to the
+	 * bottom of the usable stack.
+	 */
+	if ((unsigned long)task_stack_page(task) - (unsigned long)stack <
+	    PAGE_SIZE)
+		stack = (unsigned long *)task_stack_page(task);
+
 	while (valid_stack_ptr(task, stack, sizeof(*stack), end)) {
 		unsigned long addr;
 
-- 
2.7.4

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

#1441152

Andy Lutomirski <luto@kernel.org> wrote:
>
> Known issues: - tcp md5, rxkad, virtio_net, and virtio_console will
> have issues.  Eric Dumazet has a patch for tcp md5, and Michael
> Tsirkin says he'll fix virtio_net and virtio_console.  rxkad will be
> fixed via net-next.

There is one more stack SG user within crypto, cts.  But I have
already posted a patch for that and I am intending to push this
at the next merge window.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

#1442230

On 07/11/2016 10:53 PM, Andy Lutomirski wrote:
> Hi all-
> 
> Since the dawn of time, a kernel stack overflow has been a real PITA
> to debug, has caused nondeterministic crashes some time after the
> actual overflow, and has generally been easy to exploit for root.
> 
> With this series, arches can enable HAVE_ARCH_VMAP_STACK.  Arches
> that enable it (just x86 for now) get virtually mapped stacks with
> guard pages.  This causes reliable faults when the stack overflows.
> 
> If the arch implements it well, we get a nice OOPS on stack overflow
> (as opposed to panicing directly or otherwise exploding badly).  On
> x86, the OOPS is nice, has a usable call trace, and the overflowing
> task is killed cleanly.
> 
> This series (starting with v4) also extensively cleans up
> thread_info.  thread_info has been partially redundant with
> thread_struct for a long time -- both are places for arch code to
> add additional per-task variables.  thread_struct is much cleaner:
> it's always in task_struct, and there's nothing particularly magical
> about it.  So this series contains a bunch of cleanups on x86 to
> move almost everything from thread_info to thread_struct (which,
> even by itself, deletes more code than it adds) and to remove x86's
> dependence on thread_info's position on the stack.  Then it opts x86
> into a new config option THREAD_INFO_IN_TASK to get rid of
> arch-specific thread_info entirely and simply embed a defanged
> thread_info (containing only flags) and 'int cpu' into task_struct.
> 
> Once thread_info stops being magical, there's another benefit: we
> can free the thread stack as soon as the task is dead (without
> waiting for RCU) and then, if vmapped stacks are in use, cache the
> entire stack for reuse on the same cpu.
> 
> This seems to be an overall speedup of about 0.5-1 µs per
> pthread_create/join in a simple test -- a percpu cache of vmalloced
> stacks appears to be a bit faster than a high-order stack
> allocation, at least when the cache hits.  (I expect that workloads
> with a low cache hit rate are likely to be dominated by other
> effects anyway.)
> 
> This does not address interrupt stacks.
> 
> It's worth noting that s390 has an arch-specific gcc feature that
> detects stack overflows by adjusting function prologues.  Arches
> with features like that may wish to avoid using vmapped stacks to
> minimize the performance hit.

Yes, might not need this for stack overflow detection. What might 
be interesting is the thread_info/thread_struct change, if we can
strip down thread_info.(CONFIG_THREAD_INFO_IN_TASK). Would it actually
make sense to separate these two changes to see what performance
impact  CONFIG_THREAD_INFO_IN_TASK has on its own?

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

#1442751

On Wed, Jul 13, 2016 at 1:54 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> On 07/11/2016 10:53 PM, Andy Lutomirski wrote:
>> Hi all-
>>
>> Since the dawn of time, a kernel stack overflow has been a real PITA
>> to debug, has caused nondeterministic crashes some time after the
>> actual overflow, and has generally been easy to exploit for root.
>>
>> With this series, arches can enable HAVE_ARCH_VMAP_STACK.  Arches
>> that enable it (just x86 for now) get virtually mapped stacks with
>> guard pages.  This causes reliable faults when the stack overflows.
>>
>> If the arch implements it well, we get a nice OOPS on stack overflow
>> (as opposed to panicing directly or otherwise exploding badly).  On
>> x86, the OOPS is nice, has a usable call trace, and the overflowing
>> task is killed cleanly.
>>
>> This series (starting with v4) also extensively cleans up
>> thread_info.  thread_info has been partially redundant with
>> thread_struct for a long time -- both are places for arch code to
>> add additional per-task variables.  thread_struct is much cleaner:
>> it's always in task_struct, and there's nothing particularly magical
>> about it.  So this series contains a bunch of cleanups on x86 to
>> move almost everything from thread_info to thread_struct (which,
>> even by itself, deletes more code than it adds) and to remove x86's
>> dependence on thread_info's position on the stack.  Then it opts x86
>> into a new config option THREAD_INFO_IN_TASK to get rid of
>> arch-specific thread_info entirely and simply embed a defanged
>> thread_info (containing only flags) and 'int cpu' into task_struct.
>>
>> Once thread_info stops being magical, there's another benefit: we
>> can free the thread stack as soon as the task is dead (without
>> waiting for RCU) and then, if vmapped stacks are in use, cache the
>> entire stack for reuse on the same cpu.
>>
>> This seems to be an overall speedup of about 0.5-1 µs per
>> pthread_create/join in a simple test -- a percpu cache of vmalloced
>> stacks appears to be a bit faster than a high-order stack
>> allocation, at least when the cache hits.  (I expect that workloads
>> with a low cache hit rate are likely to be dominated by other
>> effects anyway.)
>>
>> This does not address interrupt stacks.
>>
>> It's worth noting that s390 has an arch-specific gcc feature that
>> detects stack overflows by adjusting function prologues.  Arches
>> with features like that may wish to avoid using vmapped stacks to
>> minimize the performance hit.
>
> Yes, might not need this for stack overflow detection. What might
> be interesting is the thread_info/thread_struct change, if we can
> strip down thread_info.(CONFIG_THREAD_INFO_IN_TASK). Would it actually
> make sense to separate these two changes to see what performance
> impact  CONFIG_THREAD_INFO_IN_TASK has on its own?
>

They're already separated.

CONFIG_THREAD_INFO_IN_TASK should have basically no performance impact
unless there are arch-dependent (percpu?) issues involved.  It does
enable immediate thread stack deallocation, though, and it would be
straightforward to make CONFIG_THREAD_INFO_IN_TASK cache stacks even
if CONFIG_VMAP_STACK=n.  That should be a moderate clone() speedup.


-- 
Andy Lutomirski
AMA Capital Management, LLC

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

#1442771

On 07/13/2016 08:36 PM, Andy Lutomirski wrote:
> On Wed, Jul 13, 2016 at 1:54 AM, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
>> On 07/11/2016 10:53 PM, Andy Lutomirski wrote:
>>> Hi all-
>>>
>>> Since the dawn of time, a kernel stack overflow has been a real PITA
>>> to debug, has caused nondeterministic crashes some time after the
>>> actual overflow, and has generally been easy to exploit for root.
>>>
>>> With this series, arches can enable HAVE_ARCH_VMAP_STACK.  Arches
>>> that enable it (just x86 for now) get virtually mapped stacks with
>>> guard pages.  This causes reliable faults when the stack overflows.
>>>
>>> If the arch implements it well, we get a nice OOPS on stack overflow
>>> (as opposed to panicing directly or otherwise exploding badly).  On
>>> x86, the OOPS is nice, has a usable call trace, and the overflowing
>>> task is killed cleanly.
>>>
>>> This series (starting with v4) also extensively cleans up
>>> thread_info.  thread_info has been partially redundant with
>>> thread_struct for a long time -- both are places for arch code to
>>> add additional per-task variables.  thread_struct is much cleaner:
>>> it's always in task_struct, and there's nothing particularly magical
>>> about it.  So this series contains a bunch of cleanups on x86 to
>>> move almost everything from thread_info to thread_struct (which,
>>> even by itself, deletes more code than it adds) and to remove x86's
>>> dependence on thread_info's position on the stack.  Then it opts x86
>>> into a new config option THREAD_INFO_IN_TASK to get rid of
>>> arch-specific thread_info entirely and simply embed a defanged
>>> thread_info (containing only flags) and 'int cpu' into task_struct.
>>>
>>> Once thread_info stops being magical, there's another benefit: we
>>> can free the thread stack as soon as the task is dead (without
>>> waiting for RCU) and then, if vmapped stacks are in use, cache the
>>> entire stack for reuse on the same cpu.
>>>
>>> This seems to be an overall speedup of about 0.5-1 µs per
>>> pthread_create/join in a simple test -- a percpu cache of vmalloced
>>> stacks appears to be a bit faster than a high-order stack
>>> allocation, at least when the cache hits.  (I expect that workloads
>>> with a low cache hit rate are likely to be dominated by other
>>> effects anyway.)
>>>
>>> This does not address interrupt stacks.
>>>
>>> It's worth noting that s390 has an arch-specific gcc feature that
>>> detects stack overflows by adjusting function prologues.  Arches
>>> with features like that may wish to avoid using vmapped stacks to
>>> minimize the performance hit.
>>
>> Yes, might not need this for stack overflow detection. What might
>> be interesting is the thread_info/thread_struct change, if we can
>> strip down thread_info.(CONFIG_THREAD_INFO_IN_TASK). Would it actually
>> make sense to separate these two changes to see what performance
>> impact  CONFIG_THREAD_INFO_IN_TASK has on its own?
>>
> 
> They're already separated.
> 
> CONFIG_THREAD_INFO_IN_TASK should have basically no performance impact
> unless there are arch-dependent (percpu?) issues involved.  It does
> enable immediate thread stack deallocation, though, and it would be
> straightforward to make CONFIG_THREAD_INFO_IN_TASK cache stacks even
> if CONFIG_VMAP_STACK=n.  That should be a moderate clone() speedup.

Yes. My point was more of having two patch series in case the discussion goes
on regarding virtual stack.

[toc] | [prev] | [standalone]

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

Back to top | Article view | linux.kernel

csiph-web