linux-kernel - Re: BUG: KASAN: slab-out-of-bounds in print_synth

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 8 Aug 2023 13:20:30 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Sven Schnelle <svens@...ux.ibm.com>
Cc:     linux-kernel@...r.kernel.org, Tom Zanussi <zanussi@...nel.org>
Subject: Re: BUG: KASAN: slab-out-of-bounds in print_synth_event+0xa68/0xa78

On Tue, 08 Aug 2023 16:28:49 +0200
Sven Schnelle <svens@...ux.ibm.com> wrote:

> Steven Rostedt <rostedt@...dmis.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.

That's fine. I have you to test the BE code ;-)

> >> 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.

It's been on my todo list for sometime. Maybe I should just go and do that
part.

-- Steve