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

[Bradley Morgan <brads@mainlining.org>]

On 27 August 2026 13:27:44 BST, "Jérémy Jean"
<jeremy.jean@oss.cyber.gouv.fr> wrote:
>On 2026-08-27 14:19, Steven Rostedt wrote:
>> On Thu, 27 Aug 2026 13:09:44 +0100
>> Bradley Morgan <brads@mainlining.org> wrote:
>> 
>>> >> @@ -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;
>>> >
>>> >Why this change?
>>> >
>>> >>  	struct user_event_enabler *enabler;
>>> >>
>>> >> +	t->user_event_mm = NULL;
>>> >> +	mm = user_event_mm_alloc(t);
>>> >
>>> >I don't see why you moved the mm assignment down here. The
>>> >t->user_event_mm
>>> >is not used in user_event_mm_alloc().
>>> 
>>> Uff, not wrong, I must be dummy dumb dumb, well, I base my reviews off
>>> 
>>> Does this fix the bug? And is this a small fix?
>>> 
>> 
>> The bug is fixed because it needs to NULL out that value. I asked from
>v1
>> to move that change to this function. But this function only needs to
>add
>> that line before the return. It doesn't need to modify anything else in
>> that function.
>> 
>> That is, something like this:
>> 
>> diff --git a/kernel/trace/trace_events_user.c
>b/kernel/trace/trace_events_user.c
>> index 8c82ecb735f4..6b89d225b189 100644
>> --- a/kernel/trace/trace_events_user.c
>> +++ b/kernel/trace/trace_events_user.c
>> @@ -868,6 +868,9 @@ 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_enabler *enabler;
>> 
>> +	/* On failure, do not free parent's copy */
>> +	t->user_event_mm = NULL;
>> +
>>  	if (!mm)
>>  		return;
>> 
>> 
>> -- Steve
>
>Hello Steve,
>
>Thanks for this. I thought that my previous version was okay
>so that it was not required to read the details of
>user_event_mm_alloc() to get convinced whether user_event_mm is
>accessed or not, but it's true that in the end, this is not
>required. Your fix is simpler. Do you want me to send a v3
>with that simplification and the same changelog as in the v2?
>
>Regards,
>Jérémy
I wouldn't mind, add my tag, I review differently from Steven, so Steven
may not be happy at me :(

I review on


1: does this do what it's intended
2: does it fix X?
3: Any comments, any new functions used instead, any way to get the line count shorter?


And others, I apologise if I'm wrong. I just review in a different style.
--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/