Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751634AbdGYSqU (ORCPT ); Tue, 25 Jul 2017 14:46:20 -0400 Received: from mail-it0-f54.google.com ([209.85.214.54]:38317 "EHLO mail-it0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751611AbdGYSqS (ORCPT ); Tue, 25 Jul 2017 14:46:18 -0400 MIME-Version: 1.0 In-Reply-To: <20170725175807.hflthwlmnecu4mtd@treble> References: <20170712082710.g2syanmhtwqeus4o@gmail.com> <20170712144254.tihj43mvdj2so74d@treble> <20170712192750.p4wwz6ptjrub7bav@gmail.com> <20170714171745.xq257arzxnypq4mt@treble> <20170725090944.enku4cxxnrh5eszi@gmail.com> <20170725175807.hflthwlmnecu4mtd@treble> From: Kees Cook Date: Tue, 25 Jul 2017 11:46:16 -0700 X-Google-Sender-Auth: sHBt5qJfaxLJI00aT_mjeRa5J9U Message-ID: Subject: Re: [PATCH v3 00/10] x86: ORC unwinder (previously undwarf) To: Josh Poimboeuf Cc: Ingo Molnar , "x86@kernel.org" , LKML , live-patching@vger.kernel.org, Linus Torvalds , Andy Lutomirski , Jiri Slaby , "H. Peter Anvin" , Peter Zijlstra , Mike Galbraith Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2198 Lines: 48 On Tue, Jul 25, 2017 at 10:58 AM, Josh Poimboeuf wrote: > [ Adding Kees to CC for the hardened usercopy discussion. ] > > Kees, FYI: frame pointers may be disabled by default on x86 relatively > soon (presumably weeks or months) in favor of the ORC unwinder. So the > hardened usercopy stack walk will no longer work as advertised. > > Using the ORC unwinder for hardened usercopy would probably be pretty > bad performance-wise. I'm not sure what else could be done. Ingo did > have a few ideas for sanity checks: > > On Tue, Jul 25, 2017 at 11:09:44AM +0200, Ingo Molnar wrote: >> > > > Well, on x86, hardened usercopy relies on frame pointers, but not the >> > > > unwinder. It does the frame pointer walk manually to avoid the full >> > > > unwinder overhead. See arch_within_stack_frames(). >> >> BTW., I think this aspect of the hardened user-copy is crazy stuff - there can be >> many stack frames, and this adds a serious amount of overhead even with frame >> pointers... >> >> I think the current behavior is fine: if frame pointers are disabled then >> arch_within_stack_frames() returns NOT_STACK. Maybe it could do a few sanity >> checks: we do know the kernel stack range and we could check alignment as well. > > I believe it checks the kernel stack range already in > check_stack_object() before deciding whether to call > arch_within_stack_frames(). It also has an overlapping stack check. Right, pointers starting in the stack are already checked to not go beyond the stack. As far as dropping inter-frame overflow checking, while I'd prefer to keep it, but its benefit in my mind is already pretty minimal since it already doesn't protect/exclude the stack canary. And since this is a check for a linear overflow (i.e. a contiguous access) we're mostly protected by the existing stack canary for writes. For reads, we do risk allowing return addresses to get exposed, though without the frame pointer, we've got even less to expose in the first place. So, mainly, I'm fine with this. I'm slightly sad, but it's not a huge loss. The main benefit of usercopy hardening is the slab cache object size protections... -Kees -- Kees Cook Pixel Security