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 S940042AbdEZJte (ORCPT <rfc822;w@1wt.eu>);
        Fri, 26 May 2017 05:49:34 -0400
Received: from mail.kernel.org ([198.145.29.99]:37520 "EHLO mail.kernel.org"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1759839AbdEZJtb (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 26 May 2017 05:49:31 -0400
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C4C0E23955
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: Fri, 26 May 2017 18:49:26 +0900
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Steven Rostedt <rostedt@goodmis.org>,
        LKML <linux-kernel@vger.kernel.org>, "x86@kernel.org" <x86@kernel.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are
 not RWX
Message-Id: <20170526184926.7cb902ab204e25552cc13071@kernel.org>
In-Reply-To: <CAGXu5j+=-3Po0SoO8N=LMzSTEA1y6sFnMQwq71Nwiw4dAXeABA@mail.gmail.com>
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>
        <20170525174604.GY8951@wotan.suse.de>
        <CAGXu5j+=-3Po0SoO8N=LMzSTEA1y6sFnMQwq71Nwiw4dAXeABA@mail.gmail.com>
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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, 25 May 2017 12:51:21 -0700
Kees Cook <keescook@chromium.org> wrote:

> On Thu, May 25, 2017 at 10:46 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > 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.
> 
> It seems like it really should. That would put it in a single place
> and avoid this mistake again in the future. Does module_memfree() have
> access to the allocation size, or does that need to get plumbed?

module_memfree() is just a wrapper of vfree, so find_vm_area()
will help us to get the size.

Thank you,


> 
> -Kees
> 
> -- 
> Kees Cook
> Pixel Security


-- 
Masami Hiramatsu <mhiramat@kernel.org>