[PATCH bpf-next] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[PATCH bpf-next] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yauheni Kaliuta]

bpf tracepoint program uses struct trace_event_raw_sys_enter as
argument where trace_entry is the first field. Use the same instead
of unsigned long long since if it's amended (for example by RT
patch) it accesses data with wrong offset.

Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
---
 kernel/trace/trace_syscalls.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 942ddbdace4a..07f4fa395e99 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
 			       struct syscall_trace_enter *rec)
 {
 	struct syscall_tp_t {
-		unsigned long long regs;
+		struct trace_entry ent;
 		unsigned long syscall_nr;
 		unsigned long args[SYSCALL_DEFINE_MAXARGS];
 	} param;
 	int i;
 
+	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));
+
+	/* __bpf_prog_run() requires *regs as the first parameter */
 	*(struct pt_regs **)&param = regs;
 	param.syscall_nr = rec->nr;
 	for (i = 0; i < sys_data->nb_args; i++)
@@ -657,11 +660,14 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
 			      struct syscall_trace_exit *rec)
 {
 	struct syscall_tp_t {
-		unsigned long long regs;
+		struct trace_entry ent;
 		unsigned long syscall_nr;
 		unsigned long ret;
 	} param;
 
+	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));
+
+	/* __bpf_prog_run() requires *regs as the first parameter */
 	*(struct pt_regs **)&param = regs;
 	param.syscall_nr = rec->nr;
 	param.ret = rec->ret;
-- 
2.41.0

Re: [PATCH bpf-next] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yonghong Song]

On 7/27/23 8:06 AM, Yauheni Kaliuta wrote:
> bpf tracepoint program uses struct trace_event_raw_sys_enter as
> argument where trace_entry is the first field. Use the same instead
> of unsigned long long since if it's amended (for example by RT
> patch) it accesses data with wrong offset.

Is this 'amended by RT patch' a real thing?

> 
> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
> ---
>   kernel/trace/trace_syscalls.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 942ddbdace4a..07f4fa395e99 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
>   			       struct syscall_trace_enter *rec)
>   {
>   	struct syscall_tp_t {
> -		unsigned long long regs;
> +		struct trace_entry ent;
>   		unsigned long syscall_nr;
>   		unsigned long args[SYSCALL_DEFINE_MAXARGS];
>   	} param;

I suspect we may have issues for 32bit kernel.
In 32bit kernel, with the change, the alignment for
param could be 4. That means, the 'ctx' pointer
may have an alignment 4 for bpf program, if user
tries to do ctx->regs, which will be a mis-aligned
access and it may not work for all architectures.

>   	int i;
>   
> +	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));
> +
> +	/* __bpf_prog_run() requires *regs as the first parameter */
>   	*(struct pt_regs **)&param = regs;
>   	param.syscall_nr = rec->nr;
>   	for (i = 0; i < sys_data->nb_args; i++)
> @@ -657,11 +660,14 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
>   			      struct syscall_trace_exit *rec)
>   {
>   	struct syscall_tp_t {
> -		unsigned long long regs;
> +		struct trace_entry ent;
>   		unsigned long syscall_nr;
>   		unsigned long ret;
>   	} param;
>   
> +	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));

You already have BUILD_BUG_ON in perf_call_enter. There is no need
to have another one here.

> +
> +	/* __bpf_prog_run() requires *regs as the first parameter */
>   	*(struct pt_regs **)&param = regs;
>   	param.syscall_nr = rec->nr;
>   	param.ret = rec->ret;

Re: [PATCH bpf-next] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yauheni Kaliuta]

Hi, Yonghong!

>>>>> On Thu, 27 Jul 2023 10:37:10 -0700, Yonghong Song  wrote:

 > On 7/27/23 8:06 AM, Yauheni Kaliuta wrote:
 >> bpf tracepoint program uses struct trace_event_raw_sys_enter as
 >> argument where trace_entry is the first field. Use the same instead
 >> of unsigned long long since if it's amended (for example by RT
 >> patch) it accesses data with wrong offset.

 > Is this 'amended by RT patch' a real thing?

Yes for me.

>> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
 >> ---
 >> kernel/trace/trace_syscalls.c | 10 ++++++++--
 >> 1 file changed, 8 insertions(+), 2 deletions(-)
 >> diff --git a/kernel/trace/trace_syscalls.c
 >> b/kernel/trace/trace_syscalls.c
 >> index 942ddbdace4a..07f4fa395e99 100644
 >> --- a/kernel/trace/trace_syscalls.c
 >> +++ b/kernel/trace/trace_syscalls.c
 >> @@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
 >> struct syscall_trace_enter *rec)
 >> {
 >> struct syscall_tp_t {
 >> -		unsigned long long regs;
 >> +		struct trace_entry ent;
 >> unsigned long syscall_nr;
 >> unsigned long args[SYSCALL_DEFINE_MAXARGS];
 >> } param;

 > I suspect we may have issues for 32bit kernel.
 > In 32bit kernel, with the change, the alignment for
 > param could be 4. That means, the 'ctx' pointer
 > may have an alignment 4 for bpf program, if user
 > tries to do ctx->regs, which will be a mis-aligned
 > access and it may not work for all architectures.

well, will __aligned(8) save the world?

 >> int i;
 >> +	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));
 >> +
 >> +	/* __bpf_prog_run() requires *regs as the first parameter */
 >> *(struct pt_regs **)&param = regs;
 >> param.syscall_nr = rec->nr;
 >> for (i = 0; i < sys_data->nb_args; i++)
 >> @@ -657,11 +660,14 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
 >> struct syscall_trace_exit *rec)
 >> {
 >> struct syscall_tp_t {
 >> -		unsigned long long regs;
 >> +		struct trace_entry ent;
 >> unsigned long syscall_nr;
 >> unsigned long ret;
 >> } param;
 >> +	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));

 > You already have BUILD_BUG_ON in perf_call_enter. There is no need
 > to have another one here.

Oh yes, thanks  :)

 >> +
 >> +	/* __bpf_prog_run() requires *regs as the first parameter */
 >> *(struct pt_regs **)&param = regs;
 >> param.syscall_nr = rec->nr;
 >> param.ret = rec->ret;


-- 
WBR,
Yauheni Kaliuta

[PATCH bpf-next v2] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yauheni Kaliuta]

bpf tracepoint program uses struct trace_event_raw_sys_enter as
argument where trace_entry is the first field. Use the same instead
of unsigned long long since if it's amended (for example by RT
patch) it accesses data with wrong offset.

Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
---
v2:
- remove extra BUILD_BUG_ON
- add structure alignement

---
 kernel/trace/trace_syscalls.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 942ddbdace4a..b7139f8f4ce8 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
 			       struct syscall_trace_enter *rec)
 {
 	struct syscall_tp_t {
-		unsigned long long regs;
+		struct trace_entry ent;
 		unsigned long syscall_nr;
 		unsigned long args[SYSCALL_DEFINE_MAXARGS];
-	} param;
+	} __aligned(8) param;
 	int i;
 
+	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));
+
+	/* __bpf_prog_run() requires *regs as the first parameter */
 	*(struct pt_regs **)&param = regs;
 	param.syscall_nr = rec->nr;
 	for (i = 0; i < sys_data->nb_args; i++)
@@ -657,11 +660,12 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
 			      struct syscall_trace_exit *rec)
 {
 	struct syscall_tp_t {
-		unsigned long long regs;
+		struct trace_entry ent;
 		unsigned long syscall_nr;
 		unsigned long ret;
-	} param;
+	} __aligned(8) param;
 
+	/* __bpf_prog_run() requires *regs as the first parameter */
 	*(struct pt_regs **)&param = regs;
 	param.syscall_nr = rec->nr;
 	param.ret = rec->ret;
-- 
2.41.0

Re: [PATCH bpf-next v2] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yonghong Song]

On 7/28/23 7:27 AM, Yauheni Kaliuta wrote:
> bpf tracepoint program uses struct trace_event_raw_sys_enter as
> argument where trace_entry is the first field. Use the same instead
> of unsigned long long since if it's amended (for example by RT
> patch) it accesses data with wrong offset.
> 
> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
> ---
> v2:
> - remove extra BUILD_BUG_ON
> - add structure alignement
> 
> ---
>   kernel/trace/trace_syscalls.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 942ddbdace4a..b7139f8f4ce8 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
>   			       struct syscall_trace_enter *rec)
>   {
>   	struct syscall_tp_t {
> -		unsigned long long regs;
> +		struct trace_entry ent;
>   		unsigned long syscall_nr;
>   		unsigned long args[SYSCALL_DEFINE_MAXARGS];
> -	} param;
> +	} __aligned(8) param;
>   	int i;
>   
> +	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));

Considering we used 'unsigned long long regs' before, should
the above BUILD_BUG_ON should be
	BUILD_BUG_ON(sizeof(param.ent) < sizeof(long long));
?

> +
> +	/* __bpf_prog_run() requires *regs as the first parameter */

This comment is not correct.

static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog,
                                           const void *ctx,
                                           bpf_dispatcher_fn dfunc)
{
	...
}

The first parameter is 'prog'.

Also there is no __bpf_prog_run() referenced in this function
so this comment may confuse readers. So I suggest removing
this comment. The same for perf_call_bpf_exit() below.

>   	*(struct pt_regs **)&param = regs;
>   	param.syscall_nr = rec->nr;
>   	for (i = 0; i < sys_data->nb_args; i++)
> @@ -657,11 +660,12 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
>   			      struct syscall_trace_exit *rec)
>   {
>   	struct syscall_tp_t {
> -		unsigned long long regs;
> +		struct trace_entry ent;
>   		unsigned long syscall_nr;
>   		unsigned long ret;
> -	} param;
> +	} __aligned(8) param;
>   
> +	/* __bpf_prog_run() requires *regs as the first parameter */
>   	*(struct pt_regs **)&param = regs;
>   	param.syscall_nr = rec->nr;
>   	param.ret = rec->ret;

Re: [PATCH bpf-next v2] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yauheni Kaliuta]

Hi, Yonghong!

>>>>> On Fri, 28 Jul 2023 09:44:20 -0700, Yonghong Song  wrote:

 > On 7/28/23 7:27 AM, Yauheni Kaliuta wrote:
 >> bpf tracepoint program uses struct trace_event_raw_sys_enter as
 >> argument where trace_entry is the first field. Use the same instead
 >> of unsigned long long since if it's amended (for example by RT
 >> patch) it accesses data with wrong offset.
 >> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
 >> ---
 >> v2:
 >> - remove extra BUILD_BUG_ON
 >> - add structure alignement
 >> ---
 >> kernel/trace/trace_syscalls.c | 12 ++++++++----
 >> 1 file changed, 8 insertions(+), 4 deletions(-)
 >> diff --git a/kernel/trace/trace_syscalls.c
 >> b/kernel/trace/trace_syscalls.c
 >> index 942ddbdace4a..b7139f8f4ce8 100644
 >> --- a/kernel/trace/trace_syscalls.c
 >> +++ b/kernel/trace/trace_syscalls.c
 >> @@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
 >> struct syscall_trace_enter *rec)
 >> {
 >> struct syscall_tp_t {
 >> -		unsigned long long regs;
 >> +		struct trace_entry ent;
 >> unsigned long syscall_nr;
 >> unsigned long args[SYSCALL_DEFINE_MAXARGS];
 >> -	} param;
 >> +	} __aligned(8) param;
 >> int i;
 >> +	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));

 > Considering we used 'unsigned long long regs' before, should
 > the above BUILD_BUG_ON should be
 > 	BUILD_BUG_ON(sizeof(param.ent) < sizeof(long long));
 > ?

Since the pointer's value is assigned I agree with Alexei (in the
thread [1]) to use void *.

 >> +
 >> +	/* __bpf_prog_run() requires *regs as the first parameter */

 > This comment is not correct.

 > static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog,
 >                                           const void *ctx,
 >                                           bpf_dispatcher_fn dfunc)
 > {
 > 	...
 > }

 > The first parameter is 'prog'.

 > Also there is no __bpf_prog_run() referenced in this function
 > so this comment may confuse readers. So I suggest removing
 > this comment. The same for perf_call_bpf_exit() below.

Again, in [1] we agreed that it's better to have the comment
since it's even more confusing.

Could you help to formulate it?

"__bpf_prog_run() requires *regs as the first argument for bpf
prog" or something?

But yes, I can remove it of course.

 >> *(struct pt_regs **)&param = regs;
 >> param.syscall_nr = rec->nr;
 >> for (i = 0; i < sys_data->nb_args; i++)
 >> @@ -657,11 +660,12 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
 >> struct syscall_trace_exit *rec)
 >> {
 >> struct syscall_tp_t {
 >> -		unsigned long long regs;
 >> +		struct trace_entry ent;
 >> unsigned long syscall_nr;
 >> unsigned long ret;
 >> -	} param;
 >> +	} __aligned(8) param;
 >> +	/* __bpf_prog_run() requires *regs as the first parameter */
 >> *(struct pt_regs **)&param = regs;
 >> param.syscall_nr = rec->nr;
 >> param.ret = rec->ret;


[1] https://lore.kernel.org/bpf/xunyjzy64q9b.fsf@redhat.com/T/#u

-- 
WBR,
Yauheni Kaliuta

Re: [PATCH bpf-next v2] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yonghong Song]

On 7/31/23 1:07 AM, Yauheni Kaliuta wrote:
> Hi, Yonghong!
> 
>>>>>> On Fri, 28 Jul 2023 09:44:20 -0700, Yonghong Song  wrote:
> 
>   > On 7/28/23 7:27 AM, Yauheni Kaliuta wrote:
>   >> bpf tracepoint program uses struct trace_event_raw_sys_enter as
>   >> argument where trace_entry is the first field. Use the same instead
>   >> of unsigned long long since if it's amended (for example by RT
>   >> patch) it accesses data with wrong offset.
>   >> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
>   >> ---
>   >> v2:
>   >> - remove extra BUILD_BUG_ON
>   >> - add structure alignement
>   >> ---
>   >> kernel/trace/trace_syscalls.c | 12 ++++++++----
>   >> 1 file changed, 8 insertions(+), 4 deletions(-)
>   >> diff --git a/kernel/trace/trace_syscalls.c
>   >> b/kernel/trace/trace_syscalls.c
>   >> index 942ddbdace4a..b7139f8f4ce8 100644
>   >> --- a/kernel/trace/trace_syscalls.c
>   >> +++ b/kernel/trace/trace_syscalls.c
>   >> @@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
>   >> struct syscall_trace_enter *rec)
>   >> {
>   >> struct syscall_tp_t {
>   >> -		unsigned long long regs;
>   >> +		struct trace_entry ent;
>   >> unsigned long syscall_nr;
>   >> unsigned long args[SYSCALL_DEFINE_MAXARGS];
>   >> -	} param;
>   >> +	} __aligned(8) param;
>   >> int i;
>   >> +	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));
> 
>   > Considering we used 'unsigned long long regs' before, should
>   > the above BUILD_BUG_ON should be
>   > 	BUILD_BUG_ON(sizeof(param.ent) < sizeof(long long));
>   > ?
> 
> Since the pointer's value is assigned I agree with Alexei (in the
> thread [1]) to use void *.

Okay, let us compare to sizeof(void *) then.

> 
>   >> +
>   >> +	/* __bpf_prog_run() requires *regs as the first parameter */
> 
>   > This comment is not correct.
> 
>   > static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog,
>   >                                           const void *ctx,
>   >                                           bpf_dispatcher_fn dfunc)
>   > {
>   > 	...
>   > }
> 
>   > The first parameter is 'prog'.
> 
>   > Also there is no __bpf_prog_run() referenced in this function
>   > so this comment may confuse readers. So I suggest removing
>   > this comment. The same for perf_call_bpf_exit() below.
> 
> Again, in [1] we agreed that it's better to have the comment
> since it's even more confusing.
> 
> Could you help to formulate it?
> 
> "__bpf_prog_run() requires *regs as the first argument for bpf
> prog" or something?
> 
> But yes, I can remove it of course.

You could have a comment like below:
	/* bpf prog requires 'regs' to be the first member in the ctx (a.k.a. 
&param) */

> 
>   >> *(struct pt_regs **)&param = regs;
>   >> param.syscall_nr = rec->nr;
>   >> for (i = 0; i < sys_data->nb_args; i++)
>   >> @@ -657,11 +660,12 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
>   >> struct syscall_trace_exit *rec)
>   >> {
>   >> struct syscall_tp_t {
>   >> -		unsigned long long regs;
>   >> +		struct trace_entry ent;
>   >> unsigned long syscall_nr;
>   >> unsigned long ret;
>   >> -	} param;
>   >> +	} __aligned(8) param;
>   >> +	/* __bpf_prog_run() requires *regs as the first parameter */
>   >> *(struct pt_regs **)&param = regs;
>   >> param.syscall_nr = rec->nr;
>   >> param.ret = rec->ret;
> 
> 
> [1] https://lore.kernel.org/bpf/xunyjzy64q9b.fsf@redhat.com/T/#u
> 

Re: [PATCH bpf-next v2] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yauheni Kaliuta]

Hi, Yonghong!

>>>>> On Mon, 31 Jul 2023 11:20:55 -0700, Yonghong Song  wrote:


 >> >> +
 >> >> +	/* __bpf_prog_run() requires *regs as the first parameter */
 >> > This comment is not correct.
 >> > static __always_inline u32 __bpf_prog_run(const struct bpf_prog
 >> *prog,
 >> >                                           const void *ctx,
 >> >                                           bpf_dispatcher_fn dfunc)
 >> > {
 >> > 	...
 >> > }
 >> > The first parameter is 'prog'.
 >> > Also there is no __bpf_prog_run() referenced in this function
 >> > so this comment may confuse readers. So I suggest removing
 >> > this comment. The same for perf_call_bpf_exit() below.
 >> Again, in [1] we agreed that it's better to have the comment
 >> since it's even more confusing.
 >> Could you help to formulate it?
 >> "__bpf_prog_run() requires *regs as the first argument for bpf
 >> prog" or something?
 >> But yes, I can remove it of course.

 > You could have a comment like below:
 > 	/* bpf prog requires 'regs' to be the first member in the ctx
 > 	(a.k.a. &param) */


Thanks!


-- 
WBR,
Yauheni Kaliuta

[PATCH bpf-next v3] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yauheni Kaliuta]

bpf tracepoint program uses struct trace_event_raw_sys_enter as
argument where trace_entry is the first field. Use the same instead
of unsigned long long since if it's amended (for example by RT
patch) it accesses data with wrong offset.

Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>

---

v3:
- Fixed comment

v2:
- remove extra BUILD_BUG_ON
- add structure alignement

---
 kernel/trace/trace_syscalls.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 942ddbdace4a..de753403cdaf 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -555,12 +555,15 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re
 			       struct syscall_trace_enter *rec)
 {
 	struct syscall_tp_t {
-		unsigned long long regs;
+		struct trace_entry ent;
 		unsigned long syscall_nr;
 		unsigned long args[SYSCALL_DEFINE_MAXARGS];
-	} param;
+	} __aligned(8) param;
 	int i;
 
+	BUILD_BUG_ON(sizeof(param.ent) < sizeof(void *));
+
+	/* bpf prog requires 'regs' to be the first member in the ctx (a.k.a. &param) */
 	*(struct pt_regs **)&param = regs;
 	param.syscall_nr = rec->nr;
 	for (i = 0; i < sys_data->nb_args; i++)
@@ -657,11 +660,12 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg
 			      struct syscall_trace_exit *rec)
 {
 	struct syscall_tp_t {
-		unsigned long long regs;
+		struct trace_entry ent;
 		unsigned long syscall_nr;
 		unsigned long ret;
-	} param;
+	} __aligned(8) param;
 
+	/* bpf prog requires 'regs' to be the first member in the ctx (a.k.a. &param) */
 	*(struct pt_regs **)&param = regs;
 	param.syscall_nr = rec->nr;
 	param.ret = rec->ret;
-- 
2.41.0

Re: [PATCH bpf-next v3] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[Yonghong Song]

On 8/1/23 12:52 AM, Yauheni Kaliuta wrote:
> bpf tracepoint program uses struct trace_event_raw_sys_enter as
> argument where trace_entry is the first field. Use the same instead
> of unsigned long long since if it's amended (for example by RT
> patch) it accesses data with wrong offset.
> 
> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>

Acked-by: Yonghong Song <yonghong.song@linux.dev>

Re: [PATCH bpf-next v3] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Tue,  1 Aug 2023 10:52:22 +0300 you wrote:
> bpf tracepoint program uses struct trace_event_raw_sys_enter as
> argument where trace_entry is the first field. Use the same instead
> of unsigned long long since if it's amended (for example by RT
> patch) it accesses data with wrong offset.
> 
> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3] tracing: perf_call_bpf: use struct trace_entry in struct syscall_tp_t
    https://git.kernel.org/bpf/bpf-next/c/d3c4db86c711

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html