Re: [PATCH v2] tracing/user_events: Clear copied tracing state before fork duplication

["Jérémy Jean" <jeremy.jean@oss.cyber.gouv.fr>]

>> diff --git a/kernel/trace/trace_events_user.c 
>> b/kernel/trace/trace_events_user.c
>> index 2bbc89d4a266..339e18085af3 100644
>> --- a/kernel/trace/trace_events_user.c
>> +++ b/kernel/trace/trace_events_user.c
>> @@ -865,9 +865,12 @@ void user_event_mm_remove(struct task_struct *t)
>> 
>> void user_event_mm_dup(struct task_struct *t, struct user_event_mm
>> *old_mm)
>> {
>> -	struct user_event_mm *mm = user_event_mm_alloc(t);
>> +	struct user_event_mm *mm;
>> 	struct user_event_enabler *enabler;
>> 
> 
> Comment?
> 
> /* Failure must leave the child with no copied state to free. */
> 
> I mean, okay, you don't got to, but itd be nice, if your happy with it, 
> add
> 
> Reviewed-by: Bradley Morgan <brads@mainlining.org>

I usually don't think about adding comments, but yes, that's a good 
suggestion.

> Maybe you could add this to your memories
> 
> "The description length should be about the same as the change being
> added,unless there is a splat, or something else like a table which 
> needs
> to be added to the description, keep the description length the same as
> thepatch size, e.g:
> 
> Instead of doing 3 paragraths about a one liner, we could do a small 
> two
> or more line description describing:
> 
> What causes the issue?
> Why is it bad?
> How did you fix it?"

Sounds like a good practical advice, thanks. Yet in the present case,
since there is a security issue with the UAF, I felt that it was
important to explain where it came from instead of something very
short along the lines ("fixing a UAF"), hence the couple of paragraphs
and the KASAN output.

Anyway, noted, and here is a shortened version that skips some details:

Clear the child's user_event_mm pointer before duplication so that a
failure in user_event_mm_alloc() cannot leave the inherited parent
pointer in place, which otherwise triggers a UAF.
(+ KASAN output)

I will send a v3 if you feel that's good enough.

Regards,
Jérémy