From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751939AbcGNTYG (ORCPT ); Thu, 14 Jul 2016 15:24:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33774 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751163AbcGNTYC (ORCPT ); Thu, 14 Jul 2016 15:24:02 -0400 Date: Thu, 14 Jul 2016 14:23:51 -0500 From: Josh Poimboeuf To: Kees Cook Cc: Andy Lutomirski , "linux-kernel@vger.kernel.org" , Rik van Riel , Casey Schaufler , PaX Team , Brad Spengler , Russell King , Catalin Marinas , Will Deacon , Ard Biesheuvel , Benjamin Herrenschmidt , Michael Ellerman , Tony Luck , Fenghua Yu , "David S. Miller" , X86 ML , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Andy Lutomirski , Borislav Petkov , Mathias Krause , Jan Kara , Vitaly Wool , Andrea Arcangeli , Dmitry Vyukov , Laura Abbott , "linux-arm-kernel@lists.infradead.org" , "linux-ia64@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , sparclinux , linux-arch , "linux-mm@kvack.org" , "kernel-hardening@lists.openwall.com" Subject: Re: [PATCH v2 01/11] mm: Implement stack frame object validation Message-ID: <20160714192351.567fmaz2h4drrxrc@treble> References: <1468446964-22213-1-git-send-email-keescook@chromium.org> <1468446964-22213-2-git-send-email-keescook@chromium.org> <20160714054842.6zal5rqawpgew26r@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 14 Jul 2016 19:24:01 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 14, 2016 at 11:10:18AM -0700, Kees Cook wrote: > On Wed, Jul 13, 2016 at 10:48 PM, Josh Poimboeuf wrote: > > On Wed, Jul 13, 2016 at 03:04:26PM -0700, Kees Cook wrote: > >> On Wed, Jul 13, 2016 at 3:01 PM, Andy Lutomirski wrote: > >> > On Wed, Jul 13, 2016 at 2:55 PM, Kees Cook wrote: > >> >> This creates per-architecture function arch_within_stack_frames() that > >> >> should validate if a given object is contained by a kernel stack frame. > >> >> Initial implementation is on x86. > >> >> > >> >> This is based on code from PaX. > >> >> > >> > > >> > This, along with Josh's livepatch work, are two examples of unwinders > >> > that matter for correctness instead of just debugging. ISTM this > >> > should just use Josh's code directly once it's been written. > >> > >> Do you have URL for Josh's code? I'd love to see what happening there. > > > > The code is actually going to be 100% different next time around, but > > FWIW, here's the last attempt: > > > > https://lkml.kernel.org/r/4d34d452bf8f85c7d6d5f93db1d3eeb4cba335c7.1461875890.git.jpoimboe@redhat.com > > > > In the meantime I've realized the need to rewrite the x86 core stack > > walking code to something much more manageable so we don't need all > > these unwinders everywhere. I'll probably post the patches in the next > > week or so. I'll add you to the CC list. > > Awesome! > > > With the new interface I think you'll be able to do something like: > > > > struct unwind_state; > > > > unwind_start(&state, current, NULL, NULL); > > unwind_next_frame(&state); > > oldframe = unwind_get_stack_pointer(&state); > > > > unwind_next_frame(&state); > > frame = unwind_get_stack_pointer(&state); > > > > do { > > if (obj + len <= frame) > > return blah; > > oldframe = frame; > > frame = unwind_get_stack_pointer(&state); > > > > } while (unwind_next_frame(&state); > > > > And then at the end there'll be some (still TBD) way to query whether it > > reached the last syscall pt_regs frame, or if it instead encountered a > > bogus frame pointer along the way and had to bail early. > > Sounds good to me. Will there be any frame size information available? > Right now, the unwinder from PaX just drops 2 pointers (saved frame, > saved ip) from the delta of frame address to find the size of the > actual stack area used by the function. If I could shave things like > padding and possible stack canaries off the size too, that would be > great. For x86, stacks are aligned at long word boundaries, so there's no real stack padding. Also the CC_STACKPROTECTOR stack canaries are created by a gcc feature which only affects certain functions (and thus certain frames) and I don't know of any reliable way to find them. So with frame pointers, I think the best you can do is just assume that the frame data area is always two words smaller than the total frame size. > Since I'm aiming the hardened usercopy series for 4.8, I figure I'll > just leave this unwinder in for now, and once yours lands, I can rip > it out again. Sure, sounds fine to me. If your code lands before I post mine, I can convert it myself. -- Josh