From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1422809AbdEXOdr (ORCPT ); Wed, 24 May 2017 10:33:47 -0400 Received: from mail.kernel.org ([198.145.29.99]:43412 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422775AbdEXOdp (ORCPT ); Wed, 24 May 2017 10:33:45 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7031423990 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mhiramat@kernel.org Date: Wed, 24 May 2017 23:33:36 +0900 From: Masami Hiramatsu To: Thomas Gleixner Cc: Steven Rostedt , Kees Cook , LKML , x86@kernel.org, Masami Hiramatsu , "Luis R. Rodriguez" , Peter Zijlstra Subject: Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX Message-Id: <20170524233336.6a5d5c6d0a4d9a0af6036c2e@kernel.org> In-Reply-To: References: X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 24 May 2017 15:47:17 +0200 (CEST) Thomas Gleixner wrote: > ftrace uses 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. > > This problem exists since ftrace uses trampolines on x86, but it went > unnoticed because the W=X sanity check only triggers when the tracer > builtin selftests are enabled. Though the mappings are also created W+X w/o > the self tests when the tracer is used after booting. > > Add proper set_memory_rw/ro() calls to [un]protect the trampolines before > and after modification. > This looks good to me. (same as I did for kprobes trampoline pages) Reviewed-by: Masami Hiramatsu Thank you, > Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for ftrace_ops") > Signed-off-by: Thomas Gleixner > --- > arch/x86/kernel/ftrace.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc > unsigned long offset; > unsigned long ip; > unsigned int size; > - int ret; > + int ret, npages; > > if (ops->trampoline) { > /* > @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc > */ > if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) > return; > + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT; > + set_memory_rw(ops->trampoline, npages); > } else { > ops->trampoline = create_trampoline(ops, &size); > if (!ops->trampoline) > return; > ops->trampoline_size = size; > + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > } > > offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS); > @@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc > /* Do a safe modify in case the trampoline is executing */ > new = ftrace_call_replace(ip, (unsigned long)func); > ret = update_ftrace_func(ip, new); > + set_memory_ro(ops->trampoline, npages); > > /* The update should never fail */ > WARN_ON(ret); -- Masami Hiramatsu