On Mon, 2 Oct 2023 15:52:42 +0200
Artem Savkov <asavkov@xxxxxxxxxx> wrote:
> linux-rt-devel tree contains a patch that adds an extra member to struct
> trace_entry. This causes the offset of args field in struct
> trace_event_raw_sys_enter be different from the one in struct
> syscall_trace_enter:
This patch looks like it's fixing the symptom and not the issue. No code
should rely on the two event structures to be related. That's an unwanted
coupling, that will likely cause issues down the road (like the RT patch
you mentioned).
>
> struct trace_event_raw_sys_enter {
> struct trace_entry ent; /* 0 12 */
>
> /* XXX last struct has 3 bytes of padding */
> /* XXX 4 bytes hole, try to pack */
>
> long int id; /* 16 8 */
> long unsigned int args[6]; /* 24 48 */
> /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
> char __data[]; /* 72 0 */
>
> /* size: 72, cachelines: 2, members: 4 */
> /* sum members: 68, holes: 1, sum holes: 4 */
> /* paddings: 1, sum paddings: 3 */
> /* last cacheline: 8 bytes */
> };
>
> struct syscall_trace_enter {
> struct trace_entry ent; /* 0 12 */
>
> /* XXX last struct has 3 bytes of padding */
>
> int nr; /* 12 4 */
> long unsigned int args[]; /* 16 0 */
>
> /* size: 16, cachelines: 1, members: 3 */
> /* paddings: 1, sum paddings: 3 */
> /* last cacheline: 16 bytes */
> };
>
> This, in turn, causes perf_event_set_bpf_prog() fail while running bpf
> test_profiler testcase because max_ctx_offset is calculated based on the
> former struct, while off on the latter:
The above appears to be pointing to the real bug. The "is calculated based
on the former struct while off on the latter" Why are the two being used
together? They are supposed to be *unrelated*!
>
> 10488 if (is_tracepoint || is_syscall_tp) {
> 10489 int off = trace_event_get_offsets(event->tp_event);
So basically this is clumping together the raw_syscalls with the syscalls
events as if they are the same. But the are not. They are created
differently. It's basically like using one structure to get the offsets of
another structure. That would be a bug anyplace else in the kernel. Sounds
like it's a bug here too.
I think the issue is with this code, not the tracing code.
We could expose the struct syscall_trace_enter and syscall_trace_exit if
the offsets to those are needed.
-- Steve
> 10490
> 10491 if (prog->aux->max_ctx_offset > off)
> 10492 return -EACCES;
> 10493 }
>
> This patch changes the type of nr member in syscall_trace_* structs to
> be long so that "args" offset is equal to that in struct
> trace_event_raw_sys_enter.
>
