From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S943714AbdEZNvG (ORCPT ); Fri, 26 May 2017 09:51:06 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:41389 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1763104AbdEZNvB (ORCPT ); Fri, 26 May 2017 09:51:01 -0400 Date: Fri, 26 May 2017 15:50:38 +0200 (CEST) From: Thomas Gleixner To: Steven Rostedt cc: Kees Cook , LKML , x86@kernel.org, Masami Hiramatsu , "Luis R. Rodriguez" , Peter Zijlstra Subject: Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX In-Reply-To: <20170526093717.0be3b849@gandalf.local.home> Message-ID: References: <20170524134728.61a896c9@vmware.local.home> <20170524182547.5c085dc7@vmware.local.home> <20170526093717.0be3b849@gandalf.local.home> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 26 May 2017, Steven Rostedt wrote: > On Thu, 25 May 2017 10:57:51 +0200 (CEST) > Thomas Gleixner wrote: > > > ftrace use module_alloc() to allocate trampoline pages. The mapping of > > module_alloc() is RWX, which makes sense as the memory is written to right > > after allocation. But nothing makes these pages RO after writing to them. > > > > Add proper set_memory_rw/ro() calls to protect the trampolines after > > modification. > > > > Signed-off-by: Thomas Gleixner > > --- > > Unfortunately, this was triggered in my tests: > > ftrace: allocating 54840 entries in 215 pages > Starting tracer 'function' > ------------[ cut here ]------------ > kernel BUG at /work/autotest/nobackup/linux-test.git/arch/x86/mm/pageattr.c:189! > > It appears that if you enable function tracing at boot up, calling > set_memory_ro() with interrupts disabled can cause this. As pageattr.c > at line 189 has: > > BUG_ON(irqs_disabled()); That's very early boot, right? So interrupts have to be disabled. So this wants to be: BUG_ON(irqs_disabled() && !early_boot_irqs_disabled); Thanks, tglx