On 25.07.24 22:15, Steven Rostedt wrote:
> On Thu, 25 Jul 2024 21:42:41 +0200
> Mathias Krause <mini...@grsecurity.net> wrote:
>
>> Right. But the point is, that 'event_call' is really some '&user->call'.
>> With 'user' being free'd memory, what gives? Dereferencing 'event_call'
>> is UB, so this function is doomed to fail because it cannot know if its
>> only argument points to still valid memory or not. And that's the core
>> issue -- calling that function for an object that's long gone -- the
>> missing refcounting I hinted at in my first Email.
>
> Ah, I missed that the call was part of the user structure. But I think I
> found the real fix.
>
>> [...]
>
> I believe the issue is that f_start() needs to check if the event file has
> been freed.
>
> New patch:
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 6ef29eba90ce..5fbfa1c885de 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1627,12 +1627,14 @@ static int f_show(struct seq_file *m, void *v)
>
> static void *f_start(struct seq_file *m, loff_t *pos)
> {
> + struct trace_event_file *file;
> void *p = (void *)FORMAT_HEADER;
> loff_t l = 0;
>
> /* ->stop() is called even if ->start() fails */
> mutex_lock(&event_mutex);
> - if (!event_file_data(m->private))
> + file = event_file_data(m->private);
> + if (!file || (file->flags & EVENT_FILE_FL_FREED))
> return ERR_PTR(-ENODEV);
>
> while (l < *pos && p)
>
>
Nope, still the same splats.
Sorry,
Mathias
