From: Ard Biesheuvel <ard.biesheuvel@linaro.org> To: Mark Rutland <mark.rutland@arm.com> Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Takahiro Akashi <akashi.takahiro@linaro.org>, Catalin Marinas <catalin.marinas@arm.com>, Dave Martin <dave.martin@arm.com>, James Morse <james.morse@arm.com>, Laura Abbott <labbott@fedoraproject.org>, Will Deacon <will.deacon@arm.com>, Kees Cook <keescook@chromium.org> Subject: Re: [kernel-hardening] Re: [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP Date: Fri, 14 Jul 2017 15:14:20 +0100 [thread overview] Message-ID: <CAKv+Gu-C+qt5nZ6N61M4fJ0fF_7jM1DCT9wW2gaoCBq6PZGWZQ@mail.gmail.com> (raw) In-Reply-To: <20170714140605.GB16687@leverpostej> On 14 July 2017 at 15:06, Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Jul 14, 2017 at 01:27:14PM +0100, Ard Biesheuvel wrote: >> On 14 July 2017 at 11:48, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > On 14 July 2017 at 11:32, Mark Rutland <mark.rutland@arm.com> wrote: >> >> On Thu, Jul 13, 2017 at 07:28:48PM +0100, Ard Biesheuvel wrote: > >> >>> OK, so here's a crazy idea: what if we >> >>> a) carve out a dedicated range in the VMALLOC area for stacks >> >>> b) for each stack, allocate a naturally aligned window of 2x the stack >> >>> size, and map the stack inside it, leaving the remaining space >> >>> unmapped > >> >> The logical ops (TST) and conditional branches (TB(N)Z, CB(N)Z) operate >> >> on XZR rather than SP, so to do this we need to get the SP value into a >> >> GPR. >> >> >> >> Previously, I assumed this meant we needed to corrupt a GPR (and hence >> >> stash that GPR in a sysreg), so I started writing code to free sysregs. >> >> >> >> However, I now realise I was being thick, since we can stash the GPR >> >> in the SP: >> >> >> >> sub sp, sp, x0 // sp = orig_sp - x0 >> >> add x0, sp, x0 // x0 = x0 - (orig_sp - x0) == orig_sp > > That comment is off, and should say x0 = x0 + (orig_sp - x0) == orig_sp > >> >> sub x0, x0, #S_FRAME_SIZE >> >> tb(nz) x0, #THREAD_SHIFT, overflow >> >> add x0, x0, #S_FRAME_SIZE >> >> sub x0, sp, x0 >> >> You need a neg x0, x0 here I think > > Oh, whoops. I'd mis-simplified things. > > We can avoid that by storing orig_sp + orig_x0 in sp: > > add sp, sp, x0 // sp = orig_sp + orig_x0 > sub x0, sp, x0 // x0 = orig_sp > < check > > sub x0, sp, x0 // x0 = orig_x0 > sub sp, sp, x0 // sp = orig_sp > > ... which works in a locally-built kernel where I've aligned all the > stacks. > Yes, that looks correct to me now. >> ... only, this requires a dedicated stack region, and so we'd need to >> check whether sp is inside that window as well. >> >> The easieast way would be to use a window whose start address is base2 >> aligned, but that means the beginning of the kernel VA range (where >> KASAN currently lives, and cannot be moved afaik), or a window at the >> top of the linear region. Neither look very appealing >> >> So that means arbitrary low and high limits to compare against in this >> entry path. That means more GPRs I'm afraid. > > Could you elaborate on that? I'm not sure that I follow. > > My understanding was that the comprimise with this approach is that we > only catch overflow/underflow within THREAD_SIZE of the stack, and can > get false-negatives elsewhere. Otherwise, IIUC this is sufficient > > Are you after a more stringent check (like those from the two existing > proposals that caught all out-of-bounds accesses)? > > Or am I missing something else? > No, not at all. I managed to confuse myself into thinking that we need to validate the value of SP in some way, i.e., as we would when dealing with an arbitrary faulting address.
WARNING: multiple messages have this Message-ID (diff)
From: ard.biesheuvel@linaro.org (Ard Biesheuvel) To: linux-arm-kernel@lists.infradead.org Subject: [kernel-hardening] Re: [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP Date: Fri, 14 Jul 2017 15:14:20 +0100 [thread overview] Message-ID: <CAKv+Gu-C+qt5nZ6N61M4fJ0fF_7jM1DCT9wW2gaoCBq6PZGWZQ@mail.gmail.com> (raw) In-Reply-To: <20170714140605.GB16687@leverpostej> On 14 July 2017 at 15:06, Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Jul 14, 2017 at 01:27:14PM +0100, Ard Biesheuvel wrote: >> On 14 July 2017 at 11:48, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > On 14 July 2017 at 11:32, Mark Rutland <mark.rutland@arm.com> wrote: >> >> On Thu, Jul 13, 2017 at 07:28:48PM +0100, Ard Biesheuvel wrote: > >> >>> OK, so here's a crazy idea: what if we >> >>> a) carve out a dedicated range in the VMALLOC area for stacks >> >>> b) for each stack, allocate a naturally aligned window of 2x the stack >> >>> size, and map the stack inside it, leaving the remaining space >> >>> unmapped > >> >> The logical ops (TST) and conditional branches (TB(N)Z, CB(N)Z) operate >> >> on XZR rather than SP, so to do this we need to get the SP value into a >> >> GPR. >> >> >> >> Previously, I assumed this meant we needed to corrupt a GPR (and hence >> >> stash that GPR in a sysreg), so I started writing code to free sysregs. >> >> >> >> However, I now realise I was being thick, since we can stash the GPR >> >> in the SP: >> >> >> >> sub sp, sp, x0 // sp = orig_sp - x0 >> >> add x0, sp, x0 // x0 = x0 - (orig_sp - x0) == orig_sp > > That comment is off, and should say x0 = x0 + (orig_sp - x0) == orig_sp > >> >> sub x0, x0, #S_FRAME_SIZE >> >> tb(nz) x0, #THREAD_SHIFT, overflow >> >> add x0, x0, #S_FRAME_SIZE >> >> sub x0, sp, x0 >> >> You need a neg x0, x0 here I think > > Oh, whoops. I'd mis-simplified things. > > We can avoid that by storing orig_sp + orig_x0 in sp: > > add sp, sp, x0 // sp = orig_sp + orig_x0 > sub x0, sp, x0 // x0 = orig_sp > < check > > sub x0, sp, x0 // x0 = orig_x0 > sub sp, sp, x0 // sp = orig_sp > > ... which works in a locally-built kernel where I've aligned all the > stacks. > Yes, that looks correct to me now. >> ... only, this requires a dedicated stack region, and so we'd need to >> check whether sp is inside that window as well. >> >> The easieast way would be to use a window whose start address is base2 >> aligned, but that means the beginning of the kernel VA range (where >> KASAN currently lives, and cannot be moved afaik), or a window at the >> top of the linear region. Neither look very appealing >> >> So that means arbitrary low and high limits to compare against in this >> entry path. That means more GPRs I'm afraid. > > Could you elaborate on that? I'm not sure that I follow. > > My understanding was that the comprimise with this approach is that we > only catch overflow/underflow within THREAD_SIZE of the stack, and can > get false-negatives elsewhere. Otherwise, IIUC this is sufficient > > Are you after a more stringent check (like those from the two existing > proposals that caught all out-of-bounds accesses)? > > Or am I missing something else? > No, not at all. I managed to confuse myself into thinking that we need to validate the value of SP in some way, i.e., as we would when dealing with an arbitrary faulting address.
next prev parent reply other threads:[~2017-07-14 14:14 UTC|newest] Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-07-12 22:32 [RFC PATCH 0/6] arm64: alternative VMAP_STACK implementation Mark Rutland 2017-07-12 22:32 ` [kernel-hardening] " Mark Rutland 2017-07-12 22:32 ` Mark Rutland 2017-07-12 22:32 ` [RFC PATCH 1/6] arm64: use tpidr_el1 for current, free sp_el0 Mark Rutland 2017-07-12 22:32 ` [kernel-hardening] " Mark Rutland 2017-07-12 22:32 ` Mark Rutland 2017-07-14 1:30 ` Will Deacon 2017-07-14 1:30 ` [kernel-hardening] " Will Deacon 2017-07-14 1:30 ` Will Deacon 2017-07-12 22:32 ` [RFC PATCH 2/6] arm64: avoid open-coding THREAD_SIZE{,_ORDER} Mark Rutland 2017-07-12 22:32 ` [kernel-hardening] " Mark Rutland 2017-07-12 22:32 ` Mark Rutland 2017-07-13 10:18 ` James Morse 2017-07-13 10:18 ` [kernel-hardening] " James Morse 2017-07-13 10:18 ` James Morse 2017-07-13 11:26 ` Mark Rutland 2017-07-13 11:26 ` [kernel-hardening] " Mark Rutland 2017-07-13 11:26 ` Mark Rutland 2017-07-12 22:33 ` [RFC PATCH 3/6] arm64: pad stacks to PAGE_SIZE for VMAP_STACK Mark Rutland 2017-07-12 22:33 ` [kernel-hardening] " Mark Rutland 2017-07-12 22:33 ` Mark Rutland 2017-07-12 22:33 ` [RFC PATCH 4/6] arm64: pass stack base to secondary_start_kernel Mark Rutland 2017-07-12 22:33 ` [kernel-hardening] " Mark Rutland 2017-07-12 22:33 ` Mark Rutland 2017-07-12 22:33 ` [RFC PATCH 5/6] arm64: keep track of current stack Mark Rutland 2017-07-12 22:33 ` [kernel-hardening] " Mark Rutland 2017-07-12 22:33 ` Mark Rutland 2017-07-12 22:33 ` [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP Mark Rutland 2017-07-12 22:33 ` [kernel-hardening] " Mark Rutland 2017-07-12 22:33 ` Mark Rutland 2017-07-13 6:58 ` Ard Biesheuvel 2017-07-13 6:58 ` [kernel-hardening] " Ard Biesheuvel 2017-07-13 6:58 ` Ard Biesheuvel 2017-07-13 10:49 ` Mark Rutland 2017-07-13 10:49 ` [kernel-hardening] " Mark Rutland 2017-07-13 10:49 ` Mark Rutland 2017-07-13 11:49 ` Ard Biesheuvel 2017-07-13 11:49 ` [kernel-hardening] " Ard Biesheuvel 2017-07-13 11:49 ` Ard Biesheuvel 2017-07-13 16:10 ` Mark Rutland 2017-07-13 16:10 ` [kernel-hardening] " Mark Rutland 2017-07-13 16:10 ` Mark Rutland 2017-07-13 17:55 ` [kernel-hardening] " Mark Rutland 2017-07-13 17:55 ` Mark Rutland 2017-07-13 17:55 ` Mark Rutland 2017-07-13 18:28 ` Ard Biesheuvel 2017-07-13 18:28 ` Ard Biesheuvel 2017-07-13 18:28 ` Ard Biesheuvel 2017-07-14 10:32 ` Mark Rutland 2017-07-14 10:32 ` Mark Rutland 2017-07-14 10:32 ` Mark Rutland 2017-07-14 10:48 ` Ard Biesheuvel 2017-07-14 10:48 ` Ard Biesheuvel 2017-07-14 10:48 ` Ard Biesheuvel 2017-07-14 12:27 ` Ard Biesheuvel 2017-07-14 12:27 ` Ard Biesheuvel 2017-07-14 12:27 ` Ard Biesheuvel 2017-07-14 14:06 ` Mark Rutland 2017-07-14 14:06 ` Mark Rutland 2017-07-14 14:06 ` Mark Rutland 2017-07-14 14:14 ` Ard Biesheuvel [this message] 2017-07-14 14:14 ` Ard Biesheuvel 2017-07-14 14:14 ` Ard Biesheuvel 2017-07-14 14:39 ` Robin Murphy 2017-07-14 14:39 ` Robin Murphy 2017-07-14 14:39 ` Robin Murphy 2017-07-14 15:03 ` Robin Murphy 2017-07-14 15:03 ` Robin Murphy 2017-07-14 15:03 ` Robin Murphy 2017-07-14 15:15 ` Ard Biesheuvel 2017-07-14 15:15 ` Ard Biesheuvel 2017-07-14 15:15 ` Ard Biesheuvel 2017-07-14 15:25 ` Mark Rutland 2017-07-14 15:25 ` Mark Rutland 2017-07-14 15:25 ` Mark Rutland 2017-07-14 21:27 ` Mark Rutland 2017-07-14 21:27 ` Mark Rutland 2017-07-14 21:27 ` Mark Rutland 2017-07-16 0:03 ` Ard Biesheuvel 2017-07-16 0:03 ` Ard Biesheuvel 2017-07-16 0:03 ` Ard Biesheuvel 2017-07-18 21:53 ` Laura Abbott 2017-07-18 21:53 ` Laura Abbott 2017-07-18 21:53 ` Laura Abbott 2017-07-19 8:08 ` Ard Biesheuvel 2017-07-19 8:08 ` Ard Biesheuvel 2017-07-19 8:08 ` Ard Biesheuvel 2017-07-19 23:32 ` Laura Abbott 2017-07-19 23:32 ` Laura Abbott 2017-07-20 5:35 ` Ard Biesheuvel 2017-07-20 5:35 ` Ard Biesheuvel 2017-07-20 5:35 ` Ard Biesheuvel 2017-07-20 8:36 ` James Morse 2017-07-20 8:36 ` James Morse 2017-07-20 8:36 ` James Morse 2017-07-20 8:56 ` Ard Biesheuvel 2017-07-20 8:56 ` Ard Biesheuvel 2017-07-20 8:56 ` Ard Biesheuvel 2017-07-20 17:30 ` Ard Biesheuvel 2017-07-20 17:30 ` Ard Biesheuvel 2017-07-20 17:30 ` Ard Biesheuvel 2017-07-20 19:10 ` Laura Abbott 2017-07-20 19:10 ` Laura Abbott 2017-07-20 19:10 ` Laura Abbott 2017-07-14 12:52 ` Mark Rutland 2017-07-14 12:52 ` Mark Rutland 2017-07-14 12:52 ` Mark Rutland 2017-07-14 12:55 ` Ard Biesheuvel 2017-07-14 12:55 ` Ard Biesheuvel 2017-07-14 12:55 ` Ard Biesheuvel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAKv+Gu-C+qt5nZ6N61M4fJ0fF_7jM1DCT9wW2gaoCBq6PZGWZQ@mail.gmail.com \ --to=ard.biesheuvel@linaro.org \ --cc=akashi.takahiro@linaro.org \ --cc=catalin.marinas@arm.com \ --cc=dave.martin@arm.com \ --cc=james.morse@arm.com \ --cc=keescook@chromium.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=labbott@fedoraproject.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.