From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754935AbdDGUjq (ORCPT ); Fri, 7 Apr 2017 16:39:46 -0400 Received: from r00tworld.com ([212.85.137.150]:50621 "EHLO r00tworld.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751059AbdDGUji (ORCPT ); Fri, 7 Apr 2017 16:39:38 -0400 From: "PaX Team" To: Mathias Krause , Thomas Gleixner Date: Fri, 07 Apr 2017 21:52:36 +0200 MIME-Version: 1.0 Subject: Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Reply-to: pageexec@freemail.hu CC: Andy Lutomirski , Kees Cook , Andy Lutomirski , "kernel-hardening@lists.openwall.com" , Mark Rutland , Hoeun Ryu , Emese Revfy , Russell King , X86 ML , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Peter Zijlstra Message-ID: <58E7EE04.29218.6216C107@pageexec.freemail.hu> In-reply-to: References: <1490811363-93944-1-git-send-email-keescook@chromium.org>, , X-mailer: Pegasus Mail for Windows (4.72.572) Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7BIT Content-description: Mail message body X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.1.12 (r00tworld.com [212.85.137.150]); Fri, 07 Apr 2017 21:52:38 +0200 (CEST) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7 Apr 2017 at 11:46, Thomas Gleixner wrote: > On Fri, 7 Apr 2017, Mathias Krause wrote: > > Well, doesn't look good to me. NMIs will still be able to interrupt > > this code and will run with CR0.WP = 0. > > > > Shouldn't you instead question yourself why PaX can do it "just" with > > preempt_disable() instead?! > > That's silly. Just because PaX does it, doesn't mean it's correct. is that FUD or do you have actionable information to share? > To be honest, playing games with the CR0.WP bit is outright stupid to begin with. why is that? cr0.wp exists since the i486 and its behaviour fits my purposes quite well, it's the best security/performance i know of. > Whether protected by preempt_disable or local_irq_disable, to make that > work it needs CR0 handling in the exception entry/exit at the lowest > level. correct. > And that's just a nightmare maintainence wise as it's prone to be > broken over time. i've got 14 years of experience of maintaining it and i never saw it break. > Aside of that it's pointless overhead for the normal case. unless it's optional code as the whole feature already is. > The proper solution is: > > write_rare(ptr, val) > { > mp = map_shadow_rw(ptr); > *mp = val; > unmap_shadow_rw(mp); > } this is not *the* proper solution, but only a naive one that suffers from the exact same need that the cr0.wp approach does and has worse performance impact. not exactly a win... [continuing from your next mail in order to save round-trip time] > I really do not care whether PaX wants to chase and verify that over and > over. verifying it is no different than verifying, say, swapgs use. > I certainly don't want to take the chance to leak CR0.WP ever why and where would cr0.wp leak? > and I very much care about extra stuff to check in the entry/exit path. your 'proper' solution needs (a lot more) extra stuff too. > Why the heck should we care about rare writes being performant? because you've been misled by the NIH crowd here that the PaX feature they tried to (badly) extract from has anything to do with frequency of writes. it does not. what it does do is provide an environment for variables that are conceptually writable but for security reasons should be read-only most of the time by most of the code (ditto for the grossly misunderstood and thus misnamed ro-after-shit). now imagine locking down the page table hierarchy with it... > Making the world and some more writeable hardly qualifies as tightly > focused. you forgot to add 'for a window of a few insns' and that the map/unmap approach does the same under an attacker controlled ptr. > Making the mapping concept CPU local is not rocket science > either. The question is whether it's worth the trouble. it is for people who care about the integrity of the kernel, and all this read-onlyness stuff implies that some do. From mboxrd@z Thu Jan 1 00:00:00 1970 From: pageexec@freemail.hu (PaX Team) Date: Fri, 07 Apr 2017 21:52:36 +0200 Subject: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() In-Reply-To: References: <1490811363-93944-1-git-send-email-keescook@chromium.org>, , Message-ID: <58E7EE04.29218.6216C107@pageexec.freemail.hu> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 7 Apr 2017 at 11:46, Thomas Gleixner wrote: > On Fri, 7 Apr 2017, Mathias Krause wrote: > > Well, doesn't look good to me. NMIs will still be able to interrupt > > this code and will run with CR0.WP = 0. > > > > Shouldn't you instead question yourself why PaX can do it "just" with > > preempt_disable() instead?! > > That's silly. Just because PaX does it, doesn't mean it's correct. is that FUD or do you have actionable information to share? > To be honest, playing games with the CR0.WP bit is outright stupid to begin with. why is that? cr0.wp exists since the i486 and its behaviour fits my purposes quite well, it's the best security/performance i know of. > Whether protected by preempt_disable or local_irq_disable, to make that > work it needs CR0 handling in the exception entry/exit at the lowest > level. correct. > And that's just a nightmare maintainence wise as it's prone to be > broken over time. i've got 14 years of experience of maintaining it and i never saw it break. > Aside of that it's pointless overhead for the normal case. unless it's optional code as the whole feature already is. > The proper solution is: > > write_rare(ptr, val) > { > mp = map_shadow_rw(ptr); > *mp = val; > unmap_shadow_rw(mp); > } this is not *the* proper solution, but only a naive one that suffers from the exact same need that the cr0.wp approach does and has worse performance impact. not exactly a win... [continuing from your next mail in order to save round-trip time] > I really do not care whether PaX wants to chase and verify that over and > over. verifying it is no different than verifying, say, swapgs use. > I certainly don't want to take the chance to leak CR0.WP ever why and where would cr0.wp leak? > and I very much care about extra stuff to check in the entry/exit path. your 'proper' solution needs (a lot more) extra stuff too. > Why the heck should we care about rare writes being performant? because you've been misled by the NIH crowd here that the PaX feature they tried to (badly) extract from has anything to do with frequency of writes. it does not. what it does do is provide an environment for variables that are conceptually writable but for security reasons should be read-only most of the time by most of the code (ditto for the grossly misunderstood and thus misnamed ro-after-shit). now imagine locking down the page table hierarchy with it... > Making the world and some more writeable hardly qualifies as tightly > focused. you forgot to add 'for a window of a few insns' and that the map/unmap approach does the same under an attacker controlled ptr. > Making the mapping concept CPU local is not rocket science > either. The question is whether it's worth the trouble. it is for people who care about the integrity of the kernel, and all this read-onlyness stuff implies that some do. From mboxrd@z Thu Jan 1 00:00:00 1970 From: "PaX Team" Date: Fri, 07 Apr 2017 21:52:36 +0200 MIME-Version: 1.0 Message-ID: <58E7EE04.29218.6216C107@pageexec.freemail.hu> In-reply-to: References: <1490811363-93944-1-git-send-email-keescook@chromium.org>, , Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7BIT Content-description: Mail message body Subject: Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() To: Mathias Krause , Thomas Gleixner Cc: Andy Lutomirski , Kees Cook , Andy Lutomirski , "kernel-hardening@lists.openwall.com" , Mark Rutland , Hoeun Ryu , Emese Revfy , Russell King , X86 ML , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Peter Zijlstra List-ID: On 7 Apr 2017 at 11:46, Thomas Gleixner wrote: > On Fri, 7 Apr 2017, Mathias Krause wrote: > > Well, doesn't look good to me. NMIs will still be able to interrupt > > this code and will run with CR0.WP = 0. > > > > Shouldn't you instead question yourself why PaX can do it "just" with > > preempt_disable() instead?! > > That's silly. Just because PaX does it, doesn't mean it's correct. is that FUD or do you have actionable information to share? > To be honest, playing games with the CR0.WP bit is outright stupid to begin with. why is that? cr0.wp exists since the i486 and its behaviour fits my purposes quite well, it's the best security/performance i know of. > Whether protected by preempt_disable or local_irq_disable, to make that > work it needs CR0 handling in the exception entry/exit at the lowest > level. correct. > And that's just a nightmare maintainence wise as it's prone to be > broken over time. i've got 14 years of experience of maintaining it and i never saw it break. > Aside of that it's pointless overhead for the normal case. unless it's optional code as the whole feature already is. > The proper solution is: > > write_rare(ptr, val) > { > mp = map_shadow_rw(ptr); > *mp = val; > unmap_shadow_rw(mp); > } this is not *the* proper solution, but only a naive one that suffers from the exact same need that the cr0.wp approach does and has worse performance impact. not exactly a win... [continuing from your next mail in order to save round-trip time] > I really do not care whether PaX wants to chase and verify that over and > over. verifying it is no different than verifying, say, swapgs use. > I certainly don't want to take the chance to leak CR0.WP ever why and where would cr0.wp leak? > and I very much care about extra stuff to check in the entry/exit path. your 'proper' solution needs (a lot more) extra stuff too. > Why the heck should we care about rare writes being performant? because you've been misled by the NIH crowd here that the PaX feature they tried to (badly) extract from has anything to do with frequency of writes. it does not. what it does do is provide an environment for variables that are conceptually writable but for security reasons should be read-only most of the time by most of the code (ditto for the grossly misunderstood and thus misnamed ro-after-shit). now imagine locking down the page table hierarchy with it... > Making the world and some more writeable hardly qualifies as tightly > focused. you forgot to add 'for a window of a few insns' and that the map/unmap approach does the same under an attacker controlled ptr. > Making the mapping concept CPU local is not rocket science > either. The question is whether it's worth the trouble. it is for people who care about the integrity of the kernel, and all this read-onlyness stuff implies that some do.