mirror of
https://github.com/torvalds/linux.git
synced 2026-07-27 09:36:22 +02:00
tracing fix for 7.2:
- Move rb_desc->nr_page_va before updating dynamic array The rb_descr->page_va is a dynamic array counted by nr_page_va. But the updating of the page_va[] is done before the nr_page_va is incremented causing a build with CONFIG_UBSAN_BOUNDS to flag it as an overflow. Move the increment of the counted by value before the array element is updated. - Propagate errors from remote event bulk updates The return value of trace_remote_enable_event() was not being checked by remote_events_dir_enable_write() where it would silently fail. Have it check the return value and propagate that back up to user space. - Fix resource leak on mmiotrace trace_pipe close The mmiotrace tracer was created in 2008 before the trace_pipe had a close callback to allow tracers to do clean up from trace_pipe open. The trace_pipe close cleanup callback was added in 2009 but the mmiotrace tracer was not updated. It had a hack to do the cleanup in the read call, where it may leak if user space did not read the entire buffer. Add a callback to mmiotrace trace_pipe close do to the cleanup properly. - Fix a possible NULL pointer dereference in the mmiotrace tracer If the mmio_pipe_open() fails to find a PCI device, it will set the hiter->dev pointer to NULL. The read function will blindly dereference that pointer. Fix the read call to check to see if that pointer is populated before dereferencing it. - Fix union collision of module and refcnt for dynamic events In 'struct trace_event_call', the 'module' pointer and the 'refcnt' atomic variable share the same memory space in a union. The filter on module logic only checked if the 'module' was set to determine if the event belonged to the module. As dynamic events are always builtin, it doesn't need the 'module' field of the structure and used a refcount. But the module filtering logic would then mistaken these dynamic events as a module and call module_name(event->module) on it. Add a check to see if the event is a dynamic event and if so, do not check it for being part of the given module. - Reset the top level buffer in selftests before running instances The ftracetest selftest initializes each instance before executing the tests. But it does not reset the top level buffer. Dynamic events are only added and removed by the top level so any left over dynamic events will not be removed by the reset in the instances. Left over dynamic events can cause the tests to incorrectly fail. Reset the top level buffer before running the instances. - Make the context_switch counter 64 bit The code to read user space for a system call trace event or for a trace_marker will disable migration, enable preemption, read user space into a per CPU buffer, disable preemption and enable migration again. It checks if the per CPU context switch counter to see if it changed, and if it did not, it would know that the per CPU buffer was not touched by another task. But the save counter was 32 bit and it would compare it to the 64 bit context_switch variable. A long running system could have the context_switch variable greater that 1<<32 in which case the compare will always fail. The compare will promote the 32 bit int saved value to 64 bit and compare it to the full 64 bit counter. Since the top 32 bits of the saved value was zero, it would never match. - Fix a use-after-free of the event_enable trigger The event_enable trigger allows for enabling one event when another event is triggered. When the trigger is removed, it must go through a synchronization phase to make sure it is not triggered again. The trigger itself is delayed by the "bulk delay" logic that was recently added. But the code that frees the event_enable data used to rely on the trigger code to do the synchronization. Now that the code uses the call RCU functions (and a workqueue), that delay no longer is there. Add a callback private_data_free() function that allows triggers to clean up data after the synchronization phase has completed. - Move the module_ref counter into the delay callback Since an event of the event_enable trigger can enable an event for a module, it ups the module ref count for that event's module. This prevents the event from trying to enable an event that no longer exists and cause a use-after-free bug. The ref counter was set back down when the trigger was removed but not after thy synchronization phase. This could lead to the module data being accessed after module was unloaded. Move the module ref decrement into the private_data_free() callback of the event_enable trigger. - Add mutex to protect parser in ftrace filtering The set_ftrace_filter file uses a parsing descriptor that is allocated at open and modified by writes. If multiple threads were to write to the descriptor at the same time, it can corrupt the parser. Add a mutex around the modifications of the parser descriptor. - Fix possible corruption in perf syscall tracing The perf system call trace events can now read user space. To do so, the reads of user space enable preemption and disables it again. During this time that preemption is enabled, the task can migrate. The perf event list head is assigned via a per CPU pointer. It is done before the user space part is called. If the user space reading migrates the task to another CPU, then the head pointer is no longer valid. Re-assign the head pointer after the reading of user space to keep it using the correct data. -----BEGIN PGP SIGNATURE----- iIoEABYKADIWIQRRSw7ePDh/lE+zeZMp5XQQmuv6qgUCamYDGhQccm9zdGVkdEBn b29kbWlzLm9yZwAKCRAp5XQQmuv6qkhYAP9G5wDTVQQzitl900iWp9rvQ2Qm5UWN JDnK2HO1elmj0AD8CRiHBI5W3O2yUmoO4bOFZ9YFXz+DqJ1jwkDs5FfqjAU= =EMD9 -----END PGP SIGNATURE----- Merge tag 'trace-v7.2-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace Pull tracing fixes from Steven Rostedt: - Move rb_desc->nr_page_va before updating dynamic array The rb_descr->page_va is a dynamic array counted by nr_page_va. But the updating of the page_va[] is done before the nr_page_va is incremented causing a build with CONFIG_UBSAN_BOUNDS to flag it as an overflow. Move the increment of the counted by value before the array element is updated. - Propagate errors from remote event bulk updates The return value of trace_remote_enable_event() was not being checked by remote_events_dir_enable_write() where it would silently fail. Have it check the return value and propagate that back up to user space. - Fix resource leak on mmiotrace trace_pipe close The mmiotrace tracer was created in 2008 before the trace_pipe had a close callback to allow tracers to do clean up from trace_pipe open. The trace_pipe close cleanup callback was added in 2009 but the mmiotrace tracer was not updated. It had a hack to do the cleanup in the read call, where it may leak if user space did not read the entire buffer. Add a callback to mmiotrace trace_pipe close do to the cleanup properly. - Fix a possible NULL pointer dereference in the mmiotrace tracer If the mmio_pipe_open() fails to find a PCI device, it will set the hiter->dev pointer to NULL. The read function will blindly dereference that pointer. Fix the read call to check to see if that pointer is populated before dereferencing it. - Fix union collision of module and refcnt for dynamic events In 'struct trace_event_call', the 'module' pointer and the 'refcnt' atomic variable share the same memory space in a union. The filter on module logic only checked if the 'module' was set to determine if the event belonged to the module. As dynamic events are always builtin, it doesn't need the 'module' field of the structure and used a refcount. But the module filtering logic would then mistaken these dynamic events as a module and call module_name(event->module) on it. Add a check to see if the event is a dynamic event and if so, do not check it for being part of the given module. - Reset the top level buffer in selftests before running instances The ftracetest selftest initializes each instance before executing the tests. But it does not reset the top level buffer. Dynamic events are only added and removed by the top level so any left over dynamic events will not be removed by the reset in the instances. Left over dynamic events can cause the tests to incorrectly fail. Reset the top level buffer before running the instances. - Make the context_switch counter 64 bit The code to read user space for a system call trace event or for a trace_marker will disable migration, enable preemption, read user space into a per CPU buffer, disable preemption and enable migration again. It checks if the per CPU context switch counter to see if it changed, and if it did not, it would know that the per CPU buffer was not touched by another task. But the save counter was 32 bit and it would compare it to the 64 bit context_switch variable. A long running system could have the context_switch variable greater that 1<<32 in which case the compare will always fail. The compare will promote the 32 bit int saved value to 64 bit and compare it to the full 64 bit counter. Since the top 32 bits of the saved value was zero, it would never match. - Fix a use-after-free of the event_enable trigger The event_enable trigger allows for enabling one event when another event is triggered. When the trigger is removed, it must go through a synchronization phase to make sure it is not triggered again. The trigger itself is delayed by the "bulk delay" logic that was recently added. But the code that frees the event_enable data used to rely on the trigger code to do the synchronization. Now that the code uses the call RCU functions (and a workqueue), that delay no longer is there. Add a callback private_data_free() function that allows triggers to clean up data after the synchronization phase has completed. - Move the module_ref counter into the delay callback Since an event of the event_enable trigger can enable an event for a module, it ups the module ref count for that event's module. This prevents the event from trying to enable an event that no longer exists and cause a use-after-free bug. The ref counter was set back down when the trigger was removed but not after thy synchronization phase. This could lead to the module data being accessed after module was unloaded. Move the module ref decrement into the private_data_free() callback of the event_enable trigger. - Add mutex to protect parser in ftrace filtering The set_ftrace_filter file uses a parsing descriptor that is allocated at open and modified by writes. If multiple threads were to write to the descriptor at the same time, it can corrupt the parser. Add a mutex around the modifications of the parser descriptor. - Fix possible corruption in perf syscall tracing The perf system call trace events can now read user space. To do so, the reads of user space enable preemption and disables it again. During this time that preemption is enabled, the task can migrate. The perf event list head is assigned via a per CPU pointer. It is done before the user space part is called. If the user space reading migrates the task to another CPU, then the head pointer is no longer valid. Re-assign the head pointer after the reading of user space to keep it using the correct data. * tag 'trace-v7.2-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace: tracing: perf: Fix stale head for perf syscall tracing ftrace: Add global mutex to serialize trace_parser access tracing: Delay module ref count for "enable_event" trigger tracing: Fix use-after-free freeing trigger private data tracing: Fix context switch counter truncation selftests/ftrace: Reset triggers at top level before instance loop tracing: Fix union collision of module and refcnt for dynamic events tracing: Fix mmiotrace possible NULL dereferencing of hiter->dev tracing: Fix resource leak on mmiotrace trace_pipe close tracing: Propagate errors from remote event bulk updates tracing/remotes: Fix page_va[] access before counter update in trace_remote_alloc_buffer()
This commit is contained in:
Linus Torvalds
commit
864be1277d
|
|
@ -1097,6 +1097,12 @@ struct ftrace_ops global_ops = {
|
|||
FTRACE_OPS_FL_PID,
|
||||
};
|
||||
|
||||
/*
|
||||
* parser_lock - Protects trace_parser state against concurrent operations.
|
||||
* Held across trace_get_user() and subsequent buffer parsing to prevent races.
|
||||
*/
|
||||
static DEFINE_MUTEX(parser_lock);
|
||||
|
||||
/*
|
||||
* Used by the stack unwinder to know about dynamic ftrace trampolines.
|
||||
*/
|
||||
|
|
@ -5842,6 +5848,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
|
|||
/* iter->hash is a local copy, so we don't need regex_lock */
|
||||
|
||||
parser = &iter->parser;
|
||||
|
||||
guard(mutex)(&parser_lock);
|
||||
read = trace_get_user(parser, ubuf, cnt, ppos);
|
||||
|
||||
if (read >= 0 && trace_parser_loaded(parser) &&
|
||||
|
|
@ -6984,12 +6992,14 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
|
|||
iter = file->private_data;
|
||||
|
||||
parser = &iter->parser;
|
||||
mutex_lock(&parser_lock);
|
||||
if (trace_parser_loaded(parser)) {
|
||||
int enable = !(iter->flags & FTRACE_ITER_NOTRACE);
|
||||
|
||||
ftrace_process_regex(iter, parser->buffer,
|
||||
parser->idx, enable);
|
||||
}
|
||||
mutex_unlock(&parser_lock);
|
||||
|
||||
trace_parser_put(parser);
|
||||
|
||||
|
|
@ -7321,10 +7331,12 @@ ftrace_graph_release(struct inode *inode, struct file *file)
|
|||
|
||||
parser = &fgd->parser;
|
||||
|
||||
mutex_lock(&parser_lock);
|
||||
if (trace_parser_loaded((parser))) {
|
||||
ret = ftrace_graph_set_hash(fgd->new_hash,
|
||||
parser->buffer);
|
||||
}
|
||||
mutex_unlock(&parser_lock);
|
||||
|
||||
trace_parser_put(parser);
|
||||
|
||||
|
|
@ -7437,6 +7449,7 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
|
|||
|
||||
parser = &fgd->parser;
|
||||
|
||||
guard(mutex)(&parser_lock);
|
||||
read = trace_get_user(parser, ubuf, cnt, ppos);
|
||||
|
||||
if (read >= 0 && trace_parser_loaded(parser) &&
|
||||
|
|
|
|||
|
|
@ -6187,7 +6187,7 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
|
|||
{
|
||||
int cpu = smp_processor_id();
|
||||
char *buffer = per_cpu_ptr(tinfo->tbuf, cpu)->buf;
|
||||
unsigned int cnt;
|
||||
unsigned long long cnt;
|
||||
int trys = 0;
|
||||
int ret;
|
||||
|
||||
|
|
|
|||
|
|
@ -1941,6 +1941,7 @@ struct event_trigger_data {
|
|||
struct list_head named_list;
|
||||
struct event_trigger_data *named_data;
|
||||
struct llist_node llist;
|
||||
void (*private_data_free)(struct event_trigger_data *data);
|
||||
};
|
||||
|
||||
/* Avoid typos */
|
||||
|
|
|
|||
|
|
@ -1350,7 +1350,9 @@ __ftrace_set_clr_event_nolock(struct trace_array *tr, const char *match,
|
|||
call = file->event_call;
|
||||
|
||||
/* If a module is specified, skip events that are not that module */
|
||||
if (module && (!call->module || strcmp(module_name(call->module), module)))
|
||||
if (module &&
|
||||
((call->flags & TRACE_EVENT_FL_DYNAMIC) ||
|
||||
!call->module || strcmp(module_name(call->module), module)))
|
||||
continue;
|
||||
|
||||
name = trace_event_name(call);
|
||||
|
|
|
|||
|
|
@ -6349,6 +6349,7 @@ static void event_hist_trigger_free(struct event_trigger_data *data)
|
|||
|
||||
trigger_data_free(data);
|
||||
|
||||
tracepoint_synchronize_unregister();
|
||||
remove_hist_vars(hist_data);
|
||||
|
||||
unregister_field_var_hists(hist_data);
|
||||
|
|
@ -6388,6 +6389,7 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data)
|
|||
|
||||
del_named_trigger(data);
|
||||
trigger_data_free(data);
|
||||
tracepoint_synchronize_unregister();
|
||||
kfree(cmd_ops);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,6 +38,13 @@ static void trigger_create_kthread_locked(void)
|
|||
}
|
||||
}
|
||||
|
||||
static void trigger_data_free_one(struct event_trigger_data *data)
|
||||
{
|
||||
if (data->private_data_free)
|
||||
data->private_data_free(data);
|
||||
kfree(data);
|
||||
}
|
||||
|
||||
static void trigger_data_free_queued_locked(void)
|
||||
{
|
||||
struct event_trigger_data *data, *tmp;
|
||||
|
|
@ -52,7 +59,7 @@ static void trigger_data_free_queued_locked(void)
|
|||
tracepoint_synchronize_unregister();
|
||||
|
||||
llist_for_each_entry_safe(data, tmp, llnodes, llist)
|
||||
kfree(data);
|
||||
trigger_data_free_one(data);
|
||||
}
|
||||
|
||||
/* Bulk garbage collection of event_trigger_data elements */
|
||||
|
|
@ -75,7 +82,7 @@ static int trigger_kthread_fn(void *ignore)
|
|||
tracepoint_synchronize_unregister();
|
||||
|
||||
llist_for_each_entry_safe(data, tmp, llnodes, llist)
|
||||
kfree(data);
|
||||
trigger_data_free_one(data);
|
||||
}
|
||||
|
||||
return 0;
|
||||
|
|
@ -1717,6 +1724,14 @@ int event_enable_trigger_print(struct seq_file *m,
|
|||
return 0;
|
||||
}
|
||||
|
||||
static void enable_trigger_private_data_free(struct event_trigger_data *data)
|
||||
{
|
||||
struct enable_trigger_data *enable_data = data->private_data;
|
||||
|
||||
trace_event_put_ref(enable_data->file->event_call);
|
||||
kfree(enable_data);
|
||||
}
|
||||
|
||||
void event_enable_trigger_free(struct event_trigger_data *data)
|
||||
{
|
||||
struct enable_trigger_data *enable_data = data->private_data;
|
||||
|
|
@ -1728,9 +1743,8 @@ void event_enable_trigger_free(struct event_trigger_data *data)
|
|||
if (!data->ref) {
|
||||
/* Remove the SOFT_MODE flag */
|
||||
trace_event_enable_disable(enable_data->file, 0, 1);
|
||||
trace_event_put_ref(enable_data->file->event_call);
|
||||
data->private_data_free = enable_trigger_private_data_free;
|
||||
trigger_data_free(data);
|
||||
kfree(enable_data);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -109,7 +109,6 @@ static void mmio_pipe_open(struct trace_iterator *iter)
|
|||
iter->private = hiter;
|
||||
}
|
||||
|
||||
/* XXX: This is not called when the pipe is closed! */
|
||||
static void mmio_close(struct trace_iterator *iter)
|
||||
{
|
||||
struct header_iter *hiter = iter->private;
|
||||
|
|
@ -146,7 +145,7 @@ static ssize_t mmio_read(struct trace_iterator *iter, struct file *filp,
|
|||
goto print_out;
|
||||
}
|
||||
|
||||
if (!hiter)
|
||||
if (!hiter || !hiter->dev)
|
||||
return 0;
|
||||
|
||||
mmio_print_pcidev(s, hiter->dev);
|
||||
|
|
@ -279,6 +278,7 @@ static struct tracer mmio_tracer __read_mostly =
|
|||
.start = mmio_trace_start,
|
||||
.pipe_open = mmio_pipe_open,
|
||||
.close = mmio_close,
|
||||
.pipe_close = mmio_close,
|
||||
.read = mmio_read,
|
||||
.print_line = mmio_print_line,
|
||||
.noboot = true,
|
||||
|
|
|
|||
|
|
@ -1004,11 +1004,10 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
|
|||
desc->nr_cpus++;
|
||||
|
||||
for (id = 0; id < nr_pages; id++) {
|
||||
rb_desc->nr_page_va++;
|
||||
rb_desc->page_va[id] = (unsigned long)__get_free_page(GFP_KERNEL);
|
||||
if (!rb_desc->page_va[id])
|
||||
goto err;
|
||||
|
||||
rb_desc->nr_page_va++;
|
||||
}
|
||||
rb_desc = __next_ring_buffer_desc(rb_desc);
|
||||
}
|
||||
|
|
@ -1150,10 +1149,21 @@ static ssize_t remote_events_dir_enable_write(struct file *filp, const char __us
|
|||
|
||||
for (i = 0; i < remote->nr_events; i++) {
|
||||
struct remote_event *evt = &remote->events[i];
|
||||
int eret;
|
||||
|
||||
trace_remote_enable_event(remote, evt, enable);
|
||||
eret = trace_remote_enable_event(remote, evt, enable);
|
||||
/*
|
||||
* Save the first error and return that. Some events
|
||||
* may still have been enabled, but let the user
|
||||
* know that something went wrong.
|
||||
*/
|
||||
if (!ret && eret)
|
||||
ret = eret;
|
||||
}
|
||||
|
||||
if (ret)
|
||||
return ret;
|
||||
|
||||
return count;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1451,6 +1451,11 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
|
|||
if (syscall_get_data(sys_data, args, &user_ptr,
|
||||
&size, user_sizes, &uargs, buf_size) < 0)
|
||||
return;
|
||||
|
||||
/* The above may have caused a migration */
|
||||
head = this_cpu_ptr(sys_data->enter_event->perf_events);
|
||||
if (hlist_empty(head))
|
||||
return;
|
||||
}
|
||||
|
||||
/* get the size after alignment with the u32 buffer size field */
|
||||
|
|
|
|||
|
|
@ -503,6 +503,7 @@ for t in $TEST_CASES; do
|
|||
done
|
||||
|
||||
# Test on instance loop
|
||||
(cd $TRACING_DIR; initialize_system)
|
||||
INSTANCE=" (instance) "
|
||||
for t in $TEST_CASES; do
|
||||
test_on_instance $t || continue
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user