Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756463AbdDEX6J (ORCPT ); Wed, 5 Apr 2017 19:58:09 -0400 Received: from mail.kernel.org ([198.145.29.136]:44016 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756432AbdDEX6A (ORCPT ); Wed, 5 Apr 2017 19:58:00 -0400 MIME-Version: 1.0 In-Reply-To: References: <1490811363-93944-1-git-send-email-keescook@chromium.org> <1490811363-93944-5-git-send-email-keescook@chromium.org> From: Andy Lutomirski Date: Wed, 5 Apr 2017 16:57:35 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() To: Kees Cook Cc: "kernel-hardening@lists.openwall.com" , Mark Rutland , Andy Lutomirski , Hoeun Ryu , PaX Team , Emese Revfy , Russell King , X86 ML , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" 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: 1185 Lines: 33 On Wed, Mar 29, 2017 at 6:41 PM, Kees Cook wrote: > On Wed, Mar 29, 2017 at 3:38 PM, Andy Lutomirski wrote: >> On Wed, Mar 29, 2017 at 11:15 AM, Kees Cook wrote: >>> Based on PaX's x86 pax_{open,close}_kernel() implementation, this >>> allows HAVE_ARCH_RARE_WRITE to work on x86. >>> >> >>> + >>> +static __always_inline unsigned long __arch_rare_write_begin(void) >>> +{ >>> + unsigned long cr0; >>> + >>> + preempt_disable(); >> >> This looks wrong. DEBUG_LOCKS_WARN_ON(!irqs_disabled()) would work, >> as would local_irq_disable(). There's no way that just disabling >> preemption is enough. >> >> (Also, how does this interact with perf nmis?) > > Do you mean preempt_disable() isn't strong enough here? I'm open to > suggestions. The goal would be to make sure nothing between _begin and > _end would get executed without interruption... > Sorry for the very slow response. preempt_disable() isn't strong enough to prevent interrupts, and an interrupt here would run with WP off, causing unknown havoc. I tend to think that the caller should be responsible for turning off interrupts. --Andy