Skip to content

Commit c0591b1

Browse files
petrpavlurostedt
authored andcommitted
tracing: Fix a possible race when disabling buffered events
Function trace_buffered_event_disable() is responsible for freeing pages backing buffered events and this process can run concurrently with trace_event_buffer_lock_reserve(). The following race is currently possible: * Function trace_buffered_event_disable() is called on CPU 0. It increments trace_buffered_event_cnt on each CPU and waits via synchronize_rcu() for each user of trace_buffered_event to complete. * After synchronize_rcu() is finished, function trace_buffered_event_disable() has the exclusive access to trace_buffered_event. All counters trace_buffered_event_cnt are at 1 and all pointers trace_buffered_event are still valid. * At this point, on a different CPU 1, the execution reaches trace_event_buffer_lock_reserve(). The function calls preempt_disable_notrace() and only now enters an RCU read-side critical section. The function proceeds and reads a still valid pointer from trace_buffered_event[CPU1] into the local variable "entry". However, it doesn't yet read trace_buffered_event_cnt[CPU1] which happens later. * Function trace_buffered_event_disable() continues. It frees trace_buffered_event[CPU1] and decrements trace_buffered_event_cnt[CPU1] back to 0. * Function trace_event_buffer_lock_reserve() continues. It reads and increments trace_buffered_event_cnt[CPU1] from 0 to 1. This makes it believe that it can use the "entry" that it already obtained but the pointer is now invalid and any access results in a use-after-free. Fix the problem by making a second synchronize_rcu() call after all trace_buffered_event values are set to NULL. This waits on all potential users in trace_event_buffer_lock_reserve() that still read a previous pointer from trace_buffered_event. Link: https://lore.kernel.org/all/[email protected]/ Link: https://lkml.kernel.org/r/[email protected] Cc: [email protected] Fixes: 0fc1b09 ("tracing: Use temp buffer when filtering events") Signed-off-by: Petr Pavlu <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 34209fe commit c0591b1

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

kernel/trace/trace.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2791,13 +2791,17 @@ void trace_buffered_event_disable(void)
27912791
free_page((unsigned long)per_cpu(trace_buffered_event, cpu));
27922792
per_cpu(trace_buffered_event, cpu) = NULL;
27932793
}
2794+
27942795
/*
2795-
* Make sure trace_buffered_event is NULL before clearing
2796-
* trace_buffered_event_cnt.
2796+
* Wait for all CPUs that potentially started checking if they can use
2797+
* their event buffer only after the previous synchronize_rcu() call and
2798+
* they still read a valid pointer from trace_buffered_event. It must be
2799+
* ensured they don't see cleared trace_buffered_event_cnt else they
2800+
* could wrongly decide to use the pointed-to buffer which is now freed.
27972801
*/
2798-
smp_wmb();
2802+
synchronize_rcu();
27992803

2800-
/* Do the work on each cpu */
2804+
/* For each CPU, relinquish the buffer */
28012805
on_each_cpu_mask(tracing_buffer_mask, enable_trace_buffered_event, NULL,
28022806
true);
28032807
}

0 commit comments

Comments
 (0)