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 S1165214AbdEYGZs (ORCPT <rfc822;w@1wt.eu>);
        Thu, 25 May 2017 02:25:48 -0400
Received: from Galois.linutronix.de ([146.0.238.70]:35551 "EHLO
        Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1165130AbdEYGZr (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 25 May 2017 02:25:47 -0400
Date: Thu, 25 May 2017 08:25:32 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
To: Steven Rostedt <rostedt@goodmis.org>
cc: 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] x86/ftrace: Make sure that ftrace trampolines are not
 RWX
In-Reply-To: <20170524182547.5c085dc7@vmware.local.home>
Message-ID: <alpine.DEB.2.20.1705250813190.2329@nanos>
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>
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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, 24 May 2017, Steven Rostedt wrote:
> The trampolines uses the module allocation, and it appears, that needs
> to become rw before freeing again.

Indeed. I realized that when enabling more debug options, which led to a
reliable triple fault.

How intuitive.

> I applied this patch, and it appears to fix the bug for me.

It fixes the bug, but ...

> -static inline void tramp_free(void *tramp)
> +static inline void tramp_free(void *tramp, int size)
>  {
> +	int npages;
> +
> +	npages = PAGE_ALIGN(size) >> PAGE_SHIFT;

For correctness sake this wants

    	set_memory_nx(...);

as well.

> +	set_memory_rw((unsigned long)tramp, npages);
>  	module_memfree(tramp);
>  }

I'll clean that up and post a V2.

Thanks,

	tglx