From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S969997AbdEYRqI (ORCPT <rfc822;w@1wt.eu>);
        Thu, 25 May 2017 13:46:08 -0400
Received: from mx2.suse.de ([195.135.220.15]:47663 "EHLO mx1.suse.de"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1760162AbdEYRqG (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 25 May 2017 13:46:06 -0400
Date: Thu, 25 May 2017 19:46:04 +0200
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>,
        Kees Cook <keescook@chromium.org>, LKML <linux-kernel@vger.kernel.org>,
        x86@kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
        "Luis R. Rodriguez" <mcgrof@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not
 RWX
Message-ID: <20170525174604.GY8951@wotan.suse.de>
References: <alpine.DEB.2.20.1705241459480.2201@nanos>
 <20170524134728.61a896c9@vmware.local.home>
 <alpine.DEB.2.20.1705242112210.2283@nanos>
 <20170524182547.5c085dc7@vmware.local.home>
 <alpine.DEB.2.20.1705250813190.2329@nanos>
 <alpine.DEB.2.20.1705251056410.1862@nanos>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.20.1705251056410.1862@nanos>
User-Agent: Mutt/1.6.0 (2016-04-01)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, May 25, 2017 at 10:57:51AM +0200, 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 <tglx@linutronix.de>
> ---
>  arch/x86/kernel/ftrace.c |   20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned
>  {
>  	return module_alloc(size);
>  }
> -static inline void tramp_free(void *tramp)
> +static inline void tramp_free(void *tramp, int size)
>  {
> +	int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> +
> +	set_memory_nx((unsigned long)tramp, npages);
> +	set_memory_rw((unsigned long)tramp, npages);
>  	module_memfree(tramp);
>  }

Can/should module_memfree() just do this for users? With Masami's fix that'd
be 2 users already.

   Luis