lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231003213844.1de0c138@gandalf.local.home> Date: Tue, 3 Oct 2023 21:38:44 -0400 From: Steven Rostedt <rostedt@...dmis.org> To: Artem Savkov <asavkov@...hat.com> Cc: Masami Hiramatsu <mhiramat@...nel.org>, linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org, netdev@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, linux-rt-users@...r.kernel.org, Jiri Olsa <jolsa@...nel.org> Subject: Re: [RFC PATCH] tracing: change syscall number type in struct syscall_trace_* On Mon, 2 Oct 2023 15:52:42 +0200 Artem Savkov <asavkov@...hat.com> 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. >
Powered by blists - more mailing lists