[PATCH] arm64: ptrace: Add function argument access API

[PATCH] arm64: ptrace: Add function argument access API

[Masami Hiramatsu]

Add regs_get_argument() which returns N th argument of the function
call. On arm64, it supports up to 8th argument.
Note that this chooses most probably assignment, in some case
it can be incorrect (e.g. passing data structure or floating
point etc.)

This enables ftrace kprobe events to access kernel function
arguments via $argN syntax.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/Kconfig              |    1 +
 arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 117b2541ef3d..6ba0da4be73c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -148,6 +148,7 @@ config ARM64
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_REGS_AND_STACK_ACCESS_API
+	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_RCU_TABLE_FREE
 	select HAVE_RCU_TABLE_INVALIDATE
 	select HAVE_RSEQ
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index ec60174c8c18..cfa1bc9b8b70 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
 	return regs->regs[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.
+ */
+static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
+						     unsigned int n)
+{
+#define NR_REG_ARGUMENTS 8
+	if (n < NR_REG_ARGUMENTS)
+		return regs_get_register(regs, n << 3);
+	return 0;
+}
+
 /* We must avoid circular header include via sched.h */
 struct task_struct;
 int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task);

[PATCH] arm64: ptrace: Add function argument access API

[Masami Hiramatsu]

Add regs_get_argument() which returns N th argument of the function
call. On arm64, it supports up to 8th argument.
Note that this chooses most probably assignment, in some case
it can be incorrect (e.g. passing data structure or floating
point etc.)

This enables ftrace kprobe events to access kernel function
arguments via $argN syntax.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/Kconfig              |    1 +
 arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 117b2541ef3d..6ba0da4be73c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -148,6 +148,7 @@ config ARM64
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_REGS_AND_STACK_ACCESS_API
+	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_RCU_TABLE_FREE
 	select HAVE_RCU_TABLE_INVALIDATE
 	select HAVE_RSEQ
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index ec60174c8c18..cfa1bc9b8b70 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
 	return regs->regs[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.
+ */
+static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
+						     unsigned int n)
+{
+#define NR_REG_ARGUMENTS 8
+	if (n < NR_REG_ARGUMENTS)
+		return regs_get_register(regs, n << 3);
+	return 0;
+}
+
 /* We must avoid circular header include via sched.h */
 struct task_struct;
 int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: ptrace: Add function argument access API

[Will Deacon]

On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> Add regs_get_argument() which returns N th argument of the function
> call. On arm64, it supports up to 8th argument.
> Note that this chooses most probably assignment, in some case
> it can be incorrect (e.g. passing data structure or floating
> point etc.)
> 
> This enables ftrace kprobe events to access kernel function
> arguments via $argN syntax.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  arch/arm64/Kconfig              |    1 +
>  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 117b2541ef3d..6ba0da4be73c 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -148,6 +148,7 @@ config ARM64
>  	select HAVE_PERF_REGS
>  	select HAVE_PERF_USER_STACK_DUMP
>  	select HAVE_REGS_AND_STACK_ACCESS_API
> +	select HAVE_FUNCTION_ARG_ACCESS_API
>  	select HAVE_RCU_TABLE_FREE
>  	select HAVE_RCU_TABLE_INVALIDATE
>  	select HAVE_RSEQ
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index ec60174c8c18..cfa1bc9b8b70 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
>  	return regs->regs[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.

In which cases would it be incorrect? I can imagine varargs causing
problems, but are there others?

> +static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
> +						     unsigned int n)
> +{
> +#define NR_REG_ARGUMENTS 8
> +	if (n < NR_REG_ARGUMENTS)
> +		return regs_get_register(regs, n << 3);

You can use pt_regs_read_reg() to avoid the shift here.

Will

Re: [PATCH] arm64: ptrace: Add function argument access API

[Will Deacon]

On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> Add regs_get_argument() which returns N th argument of the function
> call. On arm64, it supports up to 8th argument.
> Note that this chooses most probably assignment, in some case
> it can be incorrect (e.g. passing data structure or floating
> point etc.)
> 
> This enables ftrace kprobe events to access kernel function
> arguments via $argN syntax.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  arch/arm64/Kconfig              |    1 +
>  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 117b2541ef3d..6ba0da4be73c 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -148,6 +148,7 @@ config ARM64
>  	select HAVE_PERF_REGS
>  	select HAVE_PERF_USER_STACK_DUMP
>  	select HAVE_REGS_AND_STACK_ACCESS_API
> +	select HAVE_FUNCTION_ARG_ACCESS_API
>  	select HAVE_RCU_TABLE_FREE
>  	select HAVE_RCU_TABLE_INVALIDATE
>  	select HAVE_RSEQ
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index ec60174c8c18..cfa1bc9b8b70 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
>  	return regs->regs[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.

In which cases would it be incorrect? I can imagine varargs causing
problems, but are there others?

> +static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
> +						     unsigned int n)
> +{
> +#define NR_REG_ARGUMENTS 8
> +	if (n < NR_REG_ARGUMENTS)
> +		return regs_get_register(regs, n << 3);

You can use pt_regs_read_reg() to avoid the shift here.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: ptrace: Add function argument access API

[Masami Hiramatsu]

Hi Will,

On Wed, 3 Apr 2019 13:06:49 +0100
Will Deacon <will.deacon@arm.com> wrote:

> On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> > Add regs_get_argument() which returns N th argument of the function
> > call. On arm64, it supports up to 8th argument.
> > Note that this chooses most probably assignment, in some case
> > it can be incorrect (e.g. passing data structure or floating
> > point etc.)
> > 
> > This enables ftrace kprobe events to access kernel function
> > arguments via $argN syntax.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  arch/arm64/Kconfig              |    1 +
> >  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 117b2541ef3d..6ba0da4be73c 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -148,6 +148,7 @@ config ARM64
> >  	select HAVE_PERF_REGS
> >  	select HAVE_PERF_USER_STACK_DUMP
> >  	select HAVE_REGS_AND_STACK_ACCESS_API
> > +	select HAVE_FUNCTION_ARG_ACCESS_API
> >  	select HAVE_RCU_TABLE_FREE
> >  	select HAVE_RCU_TABLE_INVALIDATE
> >  	select HAVE_RSEQ
> > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > index ec60174c8c18..cfa1bc9b8b70 100644
> > --- a/arch/arm64/include/asm/ptrace.h
> > +++ b/arch/arm64/include/asm/ptrace.h
> > @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> >  	return regs->regs[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.
> 
> In which cases would it be incorrect? I can imagine varargs causing
> problems, but are there others?

As far as I can read "Procedure Call Standard for the ARM 64-bit 
Architecture(AArch64) 5.4.2 Parameter Passing Rules", it may not return
correct data if the target function has a parameter which is 16bytes
or bigger size. Of course that is just a limitation of this interface.
But anyway, it can return wrong data for the parameter after such big
parameters. I think, for func(data-struct-128bits p1, u64 p2), p1
is stored into r0 and r1, and p2 is stored r2, is that correct?

(Of course any SIMD/float parameter passed to the function, it also can
not correctly get, but in the kernel we can ignore it.)

> > +static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
> > +						     unsigned int n)
> > +{
> > +#define NR_REG_ARGUMENTS 8
> > +	if (n < NR_REG_ARGUMENTS)
> > +		return regs_get_register(regs, n << 3);
> 
> You can use pt_regs_read_reg() to avoid the shift here.

Ah, right!

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] arm64: ptrace: Add function argument access API

[Masami Hiramatsu]

Hi Will,

On Wed, 3 Apr 2019 13:06:49 +0100
Will Deacon <will.deacon@arm.com> wrote:

> On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> > Add regs_get_argument() which returns N th argument of the function
> > call. On arm64, it supports up to 8th argument.
> > Note that this chooses most probably assignment, in some case
> > it can be incorrect (e.g. passing data structure or floating
> > point etc.)
> > 
> > This enables ftrace kprobe events to access kernel function
> > arguments via $argN syntax.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  arch/arm64/Kconfig              |    1 +
> >  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 117b2541ef3d..6ba0da4be73c 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -148,6 +148,7 @@ config ARM64
> >  	select HAVE_PERF_REGS
> >  	select HAVE_PERF_USER_STACK_DUMP
> >  	select HAVE_REGS_AND_STACK_ACCESS_API
> > +	select HAVE_FUNCTION_ARG_ACCESS_API
> >  	select HAVE_RCU_TABLE_FREE
> >  	select HAVE_RCU_TABLE_INVALIDATE
> >  	select HAVE_RSEQ
> > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > index ec60174c8c18..cfa1bc9b8b70 100644
> > --- a/arch/arm64/include/asm/ptrace.h
> > +++ b/arch/arm64/include/asm/ptrace.h
> > @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> >  	return regs->regs[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.
> 
> In which cases would it be incorrect? I can imagine varargs causing
> problems, but are there others?

As far as I can read "Procedure Call Standard for the ARM 64-bit 
Architecture(AArch64) 5.4.2 Parameter Passing Rules", it may not return
correct data if the target function has a parameter which is 16bytes
or bigger size. Of course that is just a limitation of this interface.
But anyway, it can return wrong data for the parameter after such big
parameters. I think, for func(data-struct-128bits p1, u64 p2), p1
is stored into r0 and r1, and p2 is stored r2, is that correct?

(Of course any SIMD/float parameter passed to the function, it also can
not correctly get, but in the kernel we can ignore it.)

> > +static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
> > +						     unsigned int n)
> > +{
> > +#define NR_REG_ARGUMENTS 8
> > +	if (n < NR_REG_ARGUMENTS)
> > +		return regs_get_register(regs, n << 3);
> 
> You can use pt_regs_read_reg() to avoid the shift here.

Ah, right!

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: ptrace: Add function argument access API

[Will Deacon]

On Mon, Apr 08, 2019 at 09:33:20AM +0900, Masami Hiramatsu wrote:
> On Wed, 3 Apr 2019 13:06:49 +0100
> Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> > > Add regs_get_argument() which returns N th argument of the function
> > > call. On arm64, it supports up to 8th argument.
> > > Note that this chooses most probably assignment, in some case
> > > it can be incorrect (e.g. passing data structure or floating
> > > point etc.)
> > > 
> > > This enables ftrace kprobe events to access kernel function
> > > arguments via $argN syntax.
> > > 
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > ---
> > >  arch/arm64/Kconfig              |    1 +
> > >  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
> > >  2 files changed, 19 insertions(+)
> > > 
> > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > index 117b2541ef3d..6ba0da4be73c 100644
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -148,6 +148,7 @@ config ARM64
> > >  	select HAVE_PERF_REGS
> > >  	select HAVE_PERF_USER_STACK_DUMP
> > >  	select HAVE_REGS_AND_STACK_ACCESS_API
> > > +	select HAVE_FUNCTION_ARG_ACCESS_API
> > >  	select HAVE_RCU_TABLE_FREE
> > >  	select HAVE_RCU_TABLE_INVALIDATE
> > >  	select HAVE_RSEQ
> > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > index ec60174c8c18..cfa1bc9b8b70 100644
> > > --- a/arch/arm64/include/asm/ptrace.h
> > > +++ b/arch/arm64/include/asm/ptrace.h
> > > @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> > >  	return regs->regs[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.
> > 
> > In which cases would it be incorrect? I can imagine varargs causing
> > problems, but are there others?
> 
> As far as I can read "Procedure Call Standard for the ARM 64-bit 
> Architecture(AArch64) 5.4.2 Parameter Passing Rules", it may not return
> correct data if the target function has a parameter which is 16bytes
> or bigger size. Of course that is just a limitation of this interface.
> But anyway, it can return wrong data for the parameter after such big
> parameters. I think, for func(data-struct-128bits p1, u64 p2), p1
> is stored into r0 and r1, and p2 is stored r2, is that correct?

Oh, yes, passing things by value that don't fit in the registers will
obviously go wrong. Thanks. Do we do that in the kernel?

Anyway, please resend with the comment expanded a bit and using
pt_regs_read_reg(), then I can queue this up for 5.2.

Cheers,

Will

Re: [PATCH] arm64: ptrace: Add function argument access API

[Will Deacon]

On Mon, Apr 08, 2019 at 09:33:20AM +0900, Masami Hiramatsu wrote:
> On Wed, 3 Apr 2019 13:06:49 +0100
> Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> > > Add regs_get_argument() which returns N th argument of the function
> > > call. On arm64, it supports up to 8th argument.
> > > Note that this chooses most probably assignment, in some case
> > > it can be incorrect (e.g. passing data structure or floating
> > > point etc.)
> > > 
> > > This enables ftrace kprobe events to access kernel function
> > > arguments via $argN syntax.
> > > 
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > ---
> > >  arch/arm64/Kconfig              |    1 +
> > >  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
> > >  2 files changed, 19 insertions(+)
> > > 
> > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > index 117b2541ef3d..6ba0da4be73c 100644
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -148,6 +148,7 @@ config ARM64
> > >  	select HAVE_PERF_REGS
> > >  	select HAVE_PERF_USER_STACK_DUMP
> > >  	select HAVE_REGS_AND_STACK_ACCESS_API
> > > +	select HAVE_FUNCTION_ARG_ACCESS_API
> > >  	select HAVE_RCU_TABLE_FREE
> > >  	select HAVE_RCU_TABLE_INVALIDATE
> > >  	select HAVE_RSEQ
> > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > index ec60174c8c18..cfa1bc9b8b70 100644
> > > --- a/arch/arm64/include/asm/ptrace.h
> > > +++ b/arch/arm64/include/asm/ptrace.h
> > > @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> > >  	return regs->regs[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.
> > 
> > In which cases would it be incorrect? I can imagine varargs causing
> > problems, but are there others?
> 
> As far as I can read "Procedure Call Standard for the ARM 64-bit 
> Architecture(AArch64) 5.4.2 Parameter Passing Rules", it may not return
> correct data if the target function has a parameter which is 16bytes
> or bigger size. Of course that is just a limitation of this interface.
> But anyway, it can return wrong data for the parameter after such big
> parameters. I think, for func(data-struct-128bits p1, u64 p2), p1
> is stored into r0 and r1, and p2 is stored r2, is that correct?

Oh, yes, passing things by value that don't fit in the registers will
obviously go wrong. Thanks. Do we do that in the kernel?

Anyway, please resend with the comment expanded a bit and using
pt_regs_read_reg(), then I can queue this up for 5.2.

Cheers,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: ptrace: Add function argument access API

[Masami Hiramatsu]

On Thu, 11 Apr 2019 18:22:53 +0100
Will Deacon <will.deacon@arm.com> wrote:

> On Mon, Apr 08, 2019 at 09:33:20AM +0900, Masami Hiramatsu wrote:
> > On Wed, 3 Apr 2019 13:06:49 +0100
> > Will Deacon <will.deacon@arm.com> wrote:
> > > On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> > > > Add regs_get_argument() which returns N th argument of the function
> > > > call. On arm64, it supports up to 8th argument.
> > > > Note that this chooses most probably assignment, in some case
> > > > it can be incorrect (e.g. passing data structure or floating
> > > > point etc.)
> > > > 
> > > > This enables ftrace kprobe events to access kernel function
> > > > arguments via $argN syntax.
> > > > 
> > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > > ---
> > > >  arch/arm64/Kconfig              |    1 +
> > > >  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
> > > >  2 files changed, 19 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index 117b2541ef3d..6ba0da4be73c 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -148,6 +148,7 @@ config ARM64
> > > >  	select HAVE_PERF_REGS
> > > >  	select HAVE_PERF_USER_STACK_DUMP
> > > >  	select HAVE_REGS_AND_STACK_ACCESS_API
> > > > +	select HAVE_FUNCTION_ARG_ACCESS_API
> > > >  	select HAVE_RCU_TABLE_FREE
> > > >  	select HAVE_RCU_TABLE_INVALIDATE
> > > >  	select HAVE_RSEQ
> > > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > > index ec60174c8c18..cfa1bc9b8b70 100644
> > > > --- a/arch/arm64/include/asm/ptrace.h
> > > > +++ b/arch/arm64/include/asm/ptrace.h
> > > > @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> > > >  	return regs->regs[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.
> > > 
> > > In which cases would it be incorrect? I can imagine varargs causing
> > > problems, but are there others?
> > 
> > As far as I can read "Procedure Call Standard for the ARM 64-bit 
> > Architecture(AArch64) 5.4.2 Parameter Passing Rules", it may not return
> > correct data if the target function has a parameter which is 16bytes
> > or bigger size. Of course that is just a limitation of this interface.
> > But anyway, it can return wrong data for the parameter after such big
> > parameters. I think, for func(data-struct-128bits p1, u64 p2), p1
> > is stored into r0 and r1, and p2 is stored r2, is that correct?
> 
> Oh, yes, passing things by value that don't fit in the registers will
> obviously go wrong. Thanks. Do we do that in the kernel?

No, as far as I know. But I can't say it never happens in the kernel.

> 
> Anyway, please resend with the comment expanded a bit and using
> pt_regs_read_reg(), then I can queue this up for 5.2.

OK, I'll do.

Thank you!

> 
> Cheers,
> 
> Will


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] arm64: ptrace: Add function argument access API

[Masami Hiramatsu]

On Thu, 11 Apr 2019 18:22:53 +0100
Will Deacon <will.deacon@arm.com> wrote:

> On Mon, Apr 08, 2019 at 09:33:20AM +0900, Masami Hiramatsu wrote:
> > On Wed, 3 Apr 2019 13:06:49 +0100
> > Will Deacon <will.deacon@arm.com> wrote:
> > > On Mon, Mar 18, 2019 at 04:59:02PM +0900, Masami Hiramatsu wrote:
> > > > Add regs_get_argument() which returns N th argument of the function
> > > > call. On arm64, it supports up to 8th argument.
> > > > Note that this chooses most probably assignment, in some case
> > > > it can be incorrect (e.g. passing data structure or floating
> > > > point etc.)
> > > > 
> > > > This enables ftrace kprobe events to access kernel function
> > > > arguments via $argN syntax.
> > > > 
> > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > > ---
> > > >  arch/arm64/Kconfig              |    1 +
> > > >  arch/arm64/include/asm/ptrace.h |   18 ++++++++++++++++++
> > > >  2 files changed, 19 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index 117b2541ef3d..6ba0da4be73c 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -148,6 +148,7 @@ config ARM64
> > > >  	select HAVE_PERF_REGS
> > > >  	select HAVE_PERF_USER_STACK_DUMP
> > > >  	select HAVE_REGS_AND_STACK_ACCESS_API
> > > > +	select HAVE_FUNCTION_ARG_ACCESS_API
> > > >  	select HAVE_RCU_TABLE_FREE
> > > >  	select HAVE_RCU_TABLE_INVALIDATE
> > > >  	select HAVE_RSEQ
> > > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > > index ec60174c8c18..cfa1bc9b8b70 100644
> > > > --- a/arch/arm64/include/asm/ptrace.h
> > > > +++ b/arch/arm64/include/asm/ptrace.h
> > > > @@ -305,6 +305,24 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> > > >  	return regs->regs[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.
> > > 
> > > In which cases would it be incorrect? I can imagine varargs causing
> > > problems, but are there others?
> > 
> > As far as I can read "Procedure Call Standard for the ARM 64-bit 
> > Architecture(AArch64) 5.4.2 Parameter Passing Rules", it may not return
> > correct data if the target function has a parameter which is 16bytes
> > or bigger size. Of course that is just a limitation of this interface.
> > But anyway, it can return wrong data for the parameter after such big
> > parameters. I think, for func(data-struct-128bits p1, u64 p2), p1
> > is stored into r0 and r1, and p2 is stored r2, is that correct?
> 
> Oh, yes, passing things by value that don't fit in the registers will
> obviously go wrong. Thanks. Do we do that in the kernel?

No, as far as I know. But I can't say it never happens in the kernel.

> 
> Anyway, please resend with the comment expanded a bit and using
> pt_regs_read_reg(), then I can queue this up for 5.2.

OK, I'll do.

Thank you!

> 
> Cheers,
> 
> Will


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel