Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 12 Oct 2018 12:26:41 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     LKML <linux-kernel@vger.kernel.org>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Andy Lutomirski <luto@amacapital.net>,
        Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [RFC][PATCH] x86: ptrace: Add function argument access API
Message-ID: <20181012122641.3d87ed29@gandalf.local.home>
In-Reply-To: <20181011230021.0a7604fa@vmware.local.home>
References: <20181011230021.0a7604fa@vmware.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Anyone have any issues with this patch?

-- Steve


On Thu, 11 Oct 2018 23:00:21 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> [
>   A while ago I posted an RFC patchset for dynamic function based
>   events. But Masami pointed out that this could be done with kprobes
>   with minimal changes. He posted a patch set back in March
>   http://lkml.kernel.org/r/152049860385.7289.14079393589900496424.stgit@devbox
>   I've pulled this in locally, but haven't had the time until recently
>   to look at it seriously. I even plan to talk about these changes in
>   my talk at Open Source Summit in Edinburgh less than two weeks away
>   (talk about conference driven development!).
>   Anyway, the one patch that really needs external approval is the one
>   that creates a new architecture dependent API to retrieve function
>   arguments from pt_regs if the ip is at the start of the function call
>   (via a breakpoint or ftrace fentry). That's this patch.
> 
>   Anyone have any issues with it? If not, I'm going to start doing some
>   serious testing of this code and try to get it into the next merge
>   window.
> 
>   Thanks!
> 
>   -- Steve
> ]
> 
> From: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Add regs_get_argument() which returns N th argument of the
> function call.
> Note that this chooses most probably assignment, in some case
> it can be incorrect (e.g. passing data structure or floating
> point etc.)
> 
> This is expected to be called from kprobes or ftrace with regs
> where the top of stack is the return address.
> 
> Link: http://lkml.kernel.org/r/152465885737.26224.2822487520472783854.stgit@devbox
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  arch/Kconfig                  |  7 +++++++
>  arch/x86/Kconfig              |  1 +
>  arch/x86/include/asm/ptrace.h | 38 +++++++++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 6801123932a5..facace0c90fc 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -290,6 +290,13 @@ config HAVE_RSEQ
>  	  This symbol should be selected by an architecture if it
>  	  supports an implementation of restartable sequences.
>  
> +config HAVE_FUNCTION_ARG_ACCESS_API
> +	bool
> +	help
> +	  This symbol should be selected by an architecure if it supports
> +	  the API needed to access function arguments from pt_regs,
> +	  declared in asm/ptrace.h
> +
>  config HAVE_CLK
>  	bool
>  	help
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 1a0be022f91d..972973851779 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -184,6 +184,7 @@ config X86
>  	select HAVE_RCU_TABLE_INVALIDATE	if HAVE_RCU_TABLE_FREE
>  	select HAVE_REGS_AND_STACK_ACCESS_API
>  	select HAVE_RELIABLE_STACKTRACE		if X86_64 && (UNWINDER_FRAME_POINTER || UNWINDER_ORC) && STACK_VALIDATION
> +	select HAVE_FUNCTION_ARG_ACCESS_API
>  	select HAVE_STACKPROTECTOR		if CC_HAS_SANE_STACKPROTECTOR
>  	select HAVE_STACK_VALIDATION		if X86_64
>  	select HAVE_RSEQ
> diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> index 6de1fd3d0097..c2304b25e2fd 100644
> --- a/arch/x86/include/asm/ptrace.h
> +++ b/arch/x86/include/asm/ptrace.h
> @@ -256,6 +256,44 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
>  		return 0;
>  }
>  
> +/**
> + * regs_get_kernel_argument() - get Nth function argument in kernel
> + * @regs:	pt_regs of that context
> + * @n:		function argument number (start from 0)
> + *
> + * regs_get_argument() returns @n th argument of the function call.
> + * Note that this chooses most probably assignment, in some case
> + * it can be incorrect.
> + * This is expected to be called from kprobes or ftrace with regs
> + * where the top of stack is the return address.
> + */
> +static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
> +						     unsigned int n)
> +{
> +	static const unsigned int argument_offs[] = {
> +#ifdef __i386__
> +		offsetof(struct pt_regs, ax),
> +		offsetof(struct pt_regs, cx),
> +		offsetof(struct pt_regs, dx),
> +#define NR_REG_ARGUMENTS 3
> +#else
> +		offsetof(struct pt_regs, di),
> +		offsetof(struct pt_regs, si),
> +		offsetof(struct pt_regs, dx),
> +		offsetof(struct pt_regs, cx),
> +		offsetof(struct pt_regs, r8),
> +		offsetof(struct pt_regs, r9),
> +#define NR_REG_ARGUMENTS 6
> +#endif
> +	};
> +
> +	if (n >= NR_REG_ARGUMENTS) {
> +		n -= NR_REG_ARGUMENTS - 1;
> +		return regs_get_kernel_stack_nth(regs, n);
> +	} else
> +		return regs_get_register(regs, argument_offs[n]);
> +}
> +
>  #define arch_has_single_step()	(1)
>  #ifdef CONFIG_X86_DEBUGCTLMSR
>  #define arch_has_block_step()	(1)