Messages in this thread | | | From | Sven Schnelle <> | Subject | Re: BUG: KASAN: slab-out-of-bounds in print_synth_event+0xa68/0xa78 | Date | Tue, 08 Aug 2023 16:28:49 +0200 |
| |
Steven Rostedt <rostedt@goodmis.org> writes:
>> I think the problem is that the code assigns data_offset with: >> >> *(u32 *)&entry->fields[*n_u64] = data_offset; >> >> but reads it with: >> >> offset = (u32)entry->fields[n_u64]; >> >> which works on LE, but not BE. > > Ah, that makes sense. I didn't realize (or forgot) that s390 was BE. My > PowerPC box that was BE died years ago, and I have stopped testing BE ever > since :-(
Ok. If you want something for testing BE i could provide you with an s390 linux image + the commandline to run that within qemu. Linux on s390 is not much different than other platforms, but you would need an s390 cross-compiler.
>> >> I'm currently preparing the patch below, which also makes the code a bit >> easier to read. I'm still seeing no stack traces, but at least the >> random memory reads are gone and no KASAN warning anymore. I'll >> continue fixing and sent a full patch as soon as everything is fixed. >> >> >From 82fc673f0d3b6031b760b4217bebdb1047119041 Mon Sep 17 00:00:00 2001 >> From: Sven Schnelle <svens@linux.ibm.com> >> Date: Tue, 8 Aug 2023 11:35:12 +0200 >> Subject: [PATCH] tracing/synthetic: use union instead of casts >> >> The current code uses a lot of casts to access the fields >> member in struct synth_trace_events with different sizes. >> This makes the code hard to read, and had already introduced >> an endianess bug. Use a union and struct instead. >> >> Signed-off-by: Sven Schnelle <svens@linux.ibm.com> >> --- >> kernel/trace/trace_events_synth.c | 100 +++++++++++++++--------------- >> 1 file changed, 50 insertions(+), 50 deletions(-) >> >> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c >> index d6a70aff2410..1f8fe7f2b5b2 100644 >> --- a/kernel/trace/trace_events_synth.c >> +++ b/kernel/trace/trace_events_synth.c >> @@ -125,9 +125,22 @@ static bool synth_event_match(const char *system, const char *event, >> (!system || strcmp(system, SYNTH_SYSTEM) == 0); >> } >> >> +struct synth_trace_data { >> + u16 len; >> + u16 offset; >> +}; > > This is actually common throughout the tracing code (as all dynamic fields > have this). We should probably make this more generic than just for > synthetic events. Although, that would probably break BE user space. Hmm, > we could have it be:
I'm not familiar with the ftrace code, so I think i would need some more time to find all the other locations. Therefore i updated the patch to move the structure declaration to trace.h and sent that as a first step.
Thanks, Sven
| |