From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935897AbdGTTKn (ORCPT ); Thu, 20 Jul 2017 15:10:43 -0400 Received: from mail-qk0-f169.google.com ([209.85.220.169]:34703 "EHLO mail-qk0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934668AbdGTTKl (ORCPT ); Thu, 20 Jul 2017 15:10:41 -0400 Subject: Re: [kernel-hardening] Re: [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP To: Ard Biesheuvel , James Morse , Mark Rutland , Laura Abbott Cc: Kernel Hardening , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Takahiro Akashi , Catalin Marinas , Dave Martin , Will Deacon , Kees Cook References: <20170713161050.GG26194@leverpostej> <20170713175543.GA32528@leverpostej> <20170714103258.GA16128@leverpostej> <20170714140605.GB16687@leverpostej> <20170714212717.GB1086@leverpostej> <39a5ad84-4124-5b33-146a-cd4e48f3762f@redhat.com> <59706B82.1030601@arm.com> From: Laura Abbott Message-ID: <6fd9b78c-a807-1652-975c-53cb97ce7ca8@redhat.com> Date: Thu, 20 Jul 2017 12:10:34 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/20/2017 10:30 AM, Ard Biesheuvel wrote: > On 20 July 2017 at 09:56, Ard Biesheuvel wrote: >> On 20 July 2017 at 09:36, James Morse wrote: >>> Hi Ard, >>> >>> On 20/07/17 06:35, Ard Biesheuvel wrote: >>>> On 20 July 2017 at 00:32, Laura Abbott wrote: >>>>> I didn't notice any performance impact but I also wasn't trying that >>>>> hard. I did try this with a different configuration and ran into >>>>> stackspace errors almost immediately: >>>>> >>>>> [ 0.358026] smp: Brought up 1 node, 8 CPUs >>>>> [ 0.359359] SMP: Total of 8 processors activated. >>>>> [ 0.359542] CPU features: detected feature: 32-bit EL0 Support >>>>> [ 0.361781] Insufficient stack space to handle exception! >>> >>> [...] >>> >>>>> [ 0.367382] Task stack: [0xffffff8008e80000..0xffffff8008e84000] >>>>> [ 0.367519] IRQ stack: [0xffffffc03bf62000..0xffffffc03bf66000] >>>> >>>> The IRQ stack is not 16K aligned ... >>> >>>>> [ 0.367687] ESR: 0x00000000 -- Unknown/Uncategorized >>>>> [ 0.367868] FAR: 0x0000000000000000 >>>>> [ 0.368059] Kernel panic - not syncing: kernel stack overflow >>>>> [ 0.368252] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.12.0-00018-ge9cf49d604ef-dirty #23 >>>>> [ 0.368427] Hardware name: linux,dummy-virt (DT) >>>>> [ 0.368612] Call trace: >>>>> [ 0.368774] [] dump_backtrace+0x0/0x228 >>>>> [ 0.368979] [] show_stack+0x10/0x20 >>>>> [ 0.369270] [] dump_stack+0x88/0xac >>>>> [ 0.369459] [] panic+0x120/0x278 >>>>> [ 0.369582] [] handle_bad_stack+0xd0/0xd8 >>>>> [ 0.369799] [] __do_softirq+0x74/0x210 >>>>> [ 0.370560] SMP: stopping secondary CPUs >>>>> [ 0.384269] Rebooting in 5 seconds.. >>>>> >>>>> The config is based on what I use for booting my Hikey android >>>>> board. I haven't been able to narrow down exactly which >>>>> set of configs set this off. >>>>> >>>> >>>> ... so for some reason, the percpu atom size change fails to take effect here. >>> >>> I'm not completely up to speed with these series, so this may be noise: >>> >>> When we added the IRQ stack Jungseok Lee discovered that alignment greater than >>> PAGE_SIZE only applies to CPU0. Secondary CPUs read the per-cpu init data into a >>> page-aligned area, but any greater alignment requirement is lost. >>> >>> Because of this the irqstack was only 16byte aligned, and struct thread_info had >>> to be be discovered without depending on stack alignment. >>> >> >> We [attempted to] address that by increasing the per-CPU atom size to >> THREAD_ALIGN if CONFIG_VMAP_STACK=y, but as I am typing this, I wonder >> if that percolates all the way down to the actual vmap() calls. I will >> investigate ... > > The issue is easily reproducible in QEMU as well, when building from > the same config. I tracked it down to CONFIG_NUMA=y, which sets > CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y, affecting the placement of > the static per-CPU data (including the IRQ stack). > > However, what I hadn't realised is that the first chunk is referenced > via the linear mapping, so we will need to [vm]allocate the per-CPU > IRQ stacks explicitly, and record the address in a per-CPU pointer > variable instead. > > I have updated my branch accordingly. > Yep, this version works, both in QEMU and booting Android. Thanks, Laura From mboxrd@z Thu Jan 1 00:00:00 1970 From: labbott@redhat.com (Laura Abbott) Date: Thu, 20 Jul 2017 12:10:34 -0700 Subject: [kernel-hardening] Re: [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP In-Reply-To: References: <20170713161050.GG26194@leverpostej> <20170713175543.GA32528@leverpostej> <20170714103258.GA16128@leverpostej> <20170714140605.GB16687@leverpostej> <20170714212717.GB1086@leverpostej> <39a5ad84-4124-5b33-146a-cd4e48f3762f@redhat.com> <59706B82.1030601@arm.com> Message-ID: <6fd9b78c-a807-1652-975c-53cb97ce7ca8@redhat.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 07/20/2017 10:30 AM, Ard Biesheuvel wrote: > On 20 July 2017 at 09:56, Ard Biesheuvel wrote: >> On 20 July 2017 at 09:36, James Morse wrote: >>> Hi Ard, >>> >>> On 20/07/17 06:35, Ard Biesheuvel wrote: >>>> On 20 July 2017 at 00:32, Laura Abbott wrote: >>>>> I didn't notice any performance impact but I also wasn't trying that >>>>> hard. I did try this with a different configuration and ran into >>>>> stackspace errors almost immediately: >>>>> >>>>> [ 0.358026] smp: Brought up 1 node, 8 CPUs >>>>> [ 0.359359] SMP: Total of 8 processors activated. >>>>> [ 0.359542] CPU features: detected feature: 32-bit EL0 Support >>>>> [ 0.361781] Insufficient stack space to handle exception! >>> >>> [...] >>> >>>>> [ 0.367382] Task stack: [0xffffff8008e80000..0xffffff8008e84000] >>>>> [ 0.367519] IRQ stack: [0xffffffc03bf62000..0xffffffc03bf66000] >>>> >>>> The IRQ stack is not 16K aligned ... >>> >>>>> [ 0.367687] ESR: 0x00000000 -- Unknown/Uncategorized >>>>> [ 0.367868] FAR: 0x0000000000000000 >>>>> [ 0.368059] Kernel panic - not syncing: kernel stack overflow >>>>> [ 0.368252] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.12.0-00018-ge9cf49d604ef-dirty #23 >>>>> [ 0.368427] Hardware name: linux,dummy-virt (DT) >>>>> [ 0.368612] Call trace: >>>>> [ 0.368774] [] dump_backtrace+0x0/0x228 >>>>> [ 0.368979] [] show_stack+0x10/0x20 >>>>> [ 0.369270] [] dump_stack+0x88/0xac >>>>> [ 0.369459] [] panic+0x120/0x278 >>>>> [ 0.369582] [] handle_bad_stack+0xd0/0xd8 >>>>> [ 0.369799] [] __do_softirq+0x74/0x210 >>>>> [ 0.370560] SMP: stopping secondary CPUs >>>>> [ 0.384269] Rebooting in 5 seconds.. >>>>> >>>>> The config is based on what I use for booting my Hikey android >>>>> board. I haven't been able to narrow down exactly which >>>>> set of configs set this off. >>>>> >>>> >>>> ... so for some reason, the percpu atom size change fails to take effect here. >>> >>> I'm not completely up to speed with these series, so this may be noise: >>> >>> When we added the IRQ stack Jungseok Lee discovered that alignment greater than >>> PAGE_SIZE only applies to CPU0. Secondary CPUs read the per-cpu init data into a >>> page-aligned area, but any greater alignment requirement is lost. >>> >>> Because of this the irqstack was only 16byte aligned, and struct thread_info had >>> to be be discovered without depending on stack alignment. >>> >> >> We [attempted to] address that by increasing the per-CPU atom size to >> THREAD_ALIGN if CONFIG_VMAP_STACK=y, but as I am typing this, I wonder >> if that percolates all the way down to the actual vmap() calls. I will >> investigate ... > > The issue is easily reproducible in QEMU as well, when building from > the same config. I tracked it down to CONFIG_NUMA=y, which sets > CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y, affecting the placement of > the static per-CPU data (including the IRQ stack). > > However, what I hadn't realised is that the first chunk is referenced > via the linear mapping, so we will need to [vm]allocate the per-CPU > IRQ stacks explicitly, and record the address in a per-CPU pointer > variable instead. > > I have updated my branch accordingly. > Yep, this version works, both in QEMU and booting Android. Thanks, Laura From mboxrd@z Thu Jan 1 00:00:00 1970 References: <20170713161050.GG26194@leverpostej> <20170713175543.GA32528@leverpostej> <20170714103258.GA16128@leverpostej> <20170714140605.GB16687@leverpostej> <20170714212717.GB1086@leverpostej> <39a5ad84-4124-5b33-146a-cd4e48f3762f@redhat.com> <59706B82.1030601@arm.com> From: Laura Abbott Message-ID: <6fd9b78c-a807-1652-975c-53cb97ce7ca8@redhat.com> Date: Thu, 20 Jul 2017 12:10:34 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [kernel-hardening] Re: [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP To: Ard Biesheuvel , James Morse , Mark Rutland , Laura Abbott Cc: Kernel Hardening , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Takahiro Akashi , Catalin Marinas , Dave Martin , Will Deacon , Kees Cook List-ID: On 07/20/2017 10:30 AM, Ard Biesheuvel wrote: > On 20 July 2017 at 09:56, Ard Biesheuvel wrote: >> On 20 July 2017 at 09:36, James Morse wrote: >>> Hi Ard, >>> >>> On 20/07/17 06:35, Ard Biesheuvel wrote: >>>> On 20 July 2017 at 00:32, Laura Abbott wrote: >>>>> I didn't notice any performance impact but I also wasn't trying that >>>>> hard. I did try this with a different configuration and ran into >>>>> stackspace errors almost immediately: >>>>> >>>>> [ 0.358026] smp: Brought up 1 node, 8 CPUs >>>>> [ 0.359359] SMP: Total of 8 processors activated. >>>>> [ 0.359542] CPU features: detected feature: 32-bit EL0 Support >>>>> [ 0.361781] Insufficient stack space to handle exception! >>> >>> [...] >>> >>>>> [ 0.367382] Task stack: [0xffffff8008e80000..0xffffff8008e84000] >>>>> [ 0.367519] IRQ stack: [0xffffffc03bf62000..0xffffffc03bf66000] >>>> >>>> The IRQ stack is not 16K aligned ... >>> >>>>> [ 0.367687] ESR: 0x00000000 -- Unknown/Uncategorized >>>>> [ 0.367868] FAR: 0x0000000000000000 >>>>> [ 0.368059] Kernel panic - not syncing: kernel stack overflow >>>>> [ 0.368252] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.12.0-00018-ge9cf49d604ef-dirty #23 >>>>> [ 0.368427] Hardware name: linux,dummy-virt (DT) >>>>> [ 0.368612] Call trace: >>>>> [ 0.368774] [] dump_backtrace+0x0/0x228 >>>>> [ 0.368979] [] show_stack+0x10/0x20 >>>>> [ 0.369270] [] dump_stack+0x88/0xac >>>>> [ 0.369459] [] panic+0x120/0x278 >>>>> [ 0.369582] [] handle_bad_stack+0xd0/0xd8 >>>>> [ 0.369799] [] __do_softirq+0x74/0x210 >>>>> [ 0.370560] SMP: stopping secondary CPUs >>>>> [ 0.384269] Rebooting in 5 seconds.. >>>>> >>>>> The config is based on what I use for booting my Hikey android >>>>> board. I haven't been able to narrow down exactly which >>>>> set of configs set this off. >>>>> >>>> >>>> ... so for some reason, the percpu atom size change fails to take effect here. >>> >>> I'm not completely up to speed with these series, so this may be noise: >>> >>> When we added the IRQ stack Jungseok Lee discovered that alignment greater than >>> PAGE_SIZE only applies to CPU0. Secondary CPUs read the per-cpu init data into a >>> page-aligned area, but any greater alignment requirement is lost. >>> >>> Because of this the irqstack was only 16byte aligned, and struct thread_info had >>> to be be discovered without depending on stack alignment. >>> >> >> We [attempted to] address that by increasing the per-CPU atom size to >> THREAD_ALIGN if CONFIG_VMAP_STACK=y, but as I am typing this, I wonder >> if that percolates all the way down to the actual vmap() calls. I will >> investigate ... > > The issue is easily reproducible in QEMU as well, when building from > the same config. I tracked it down to CONFIG_NUMA=y, which sets > CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y, affecting the placement of > the static per-CPU data (including the IRQ stack). > > However, what I hadn't realised is that the first chunk is referenced > via the linear mapping, so we will need to [vm]allocate the per-CPU > IRQ stacks explicitly, and record the address in a per-CPU pointer > variable instead. > > I have updated my branch accordingly. > Yep, this version works, both in QEMU and booting Android. Thanks, Laura