Pull tracing fixes from Steven Rostedt:
- Fix NULL dereference when copying histogram field variable
If event_hist_trigger_parse() creates an action that uses one of
pseudo fields for its keys, the hist_data field element will have a
NULL for its field member. When looking to match that key, the lookup
uses key_field->field->name which will dereference a NULL pointer.
Instead use the helper function that handles such cases:
expr_field_str().
- Take the trace_types_lock when dropping the buffer_meta reference
The tracing_buffer_meta_open() error return drops the trace_array
reference with __trace_array_put() instead of trace_array_put() where
the former doesn't take the lock and the latter does.
- Fix NULL pointer dereference when reading per_cpu/cpuN/buffer_meta
These files are created for every possible CPU. But it only contains
data for CPUs that have come online. If one is read for a CPU that
hasn't come online yet, it triggers a NULL pointer dereference.
Have reading of CPUs that have not come on line yet return -ENODEV.
- Fix an out-of-bounds shift in the log2 histogram modifier
Histograms can take a log2 modifier to have the buckets defined by
log2 increments. If a shift is greater than ULONG_MAX or above 2^63,
the key falls into a lower order bucket.
Use order_base_2() instead of "ilog2(n - 1) + 1" to have all
overflowed keys go into the top bucket.
- Fix the persistent ring buffer rb_meta_subbuf_idx() accounting
The algorithm for finding the index of a sub-buffer in a persistent
ring buffer buffer did not take into account the meta data at the
beginning of the buffer causing the wrong index to be returned.
As rb_subbufs_from_meta() does the proper accounting to find the
first sub-buffer, use that function instead of repeating the logic
(incorrectly) and make the calculations based off of that.
- Fix the symbol name checks for the remote buffer code
The remote ring buffer has a build time check to make sure its not
using any code that will not be in the hypervisor. For various
architectures, there has been false positive reports from this
checker.
Fix them.
- Fix false warning in ring_buffer_map_get_reader()
There is a WARN_ON() after rb_get_reader_page() if no reader is
returned. But there are legitimate cases where this can happen.
Remove the WARN_ON() and also a check that covers one of the
legitimate cases that tries to exit out early. There's no need for
the check if it just simply allows the return of NULL from
rb_get_reader_page().
- Fix the ignored return value from resize_buffer_duplicate_size()
Some clean up of the code in tracing_snapshot_write() removed
checking the return value of resize_buffer_duplicate_size() and if
that failed it would not be detected.
- Fix the histogram timestamps for buffered events
The histogram timestamps are taken from the events that are recorded.
It currently reads ring_buffer_event_time_stamp() but if the event is
buffered it does not read it properly. Use tracing_event_time_stamp()
which handles both buffered and normal events.
- Do not initialize tracefs if tracing is disabled at boot
If tracing fails to allocate or some other error is detected,
tracing_disabled is set. Do not initialize tracefs in this case as it
may expect data to be initialized but is not.
- Free eventfs inode after an RCU grace period
Currently eventfs_inodes are freed after a SRCU grace period as the
iterations in the code use SRCU. There are locations that the inode
may still be accessed from the VFS that uses RCU which does not have
the same synchronization as SRCU. Add an RCU wait before free eventfs
inodes.
- Do not fail persistent and remote buffers under memory pressure
As persistent and remote buffers have their own pre-allocated memory,
there's no reason to check how much memory is available before
created them. By checking the memory pressure, these buffers may fail
to be created even though their memory is already present.
- Account for subbuffer size when checking how much memory is available
Before allocating a ring buffer, the size is checked against the
current memory pressure of the system. But it assumes every
sub-buffer is of page size. This is no longer the case. Use the
actual sub-buffer size when calculating how much memory will be used.
- Reject non-stack fields for synthetic stacktrace fields
Stack traces are special fields and the checks currently do not do
enough to make sure that the fields being processed as stack traces
are indeed stack traces. Update the logic to properly detect what is
or is not a stack trace.
- Fix a crash in print_graph_function_event()
When a tracer does not define static or default flags the
tr->current_trace_flags is set to NULL. If the ring buffer is not
cleared when switching tracers, the previous tracers events could be
read. The function_graph entries will dereference the
tr->current_trace_flags and cause a crash.
Set the tr->current_trace_flags to a empty set of flags to prevent it
from being NULL.
- Separate tracing filter predicate count from allocation
The amount allocated for the predicate program is the number of
predicates defined plus two for adding "true" and "false" at the end.
The nr_preds is incremented by two to accommodate the booleans but is
then used as the limit check for number of predicates. This is
incorrect as it is now two higher than what is expected.
Use a separate variable for the size of the predicate array.
- Fix the filter operator stack check
The operator stack increments the pointer and then writes the content
onto the stack. The check for overflow compared the current pointer
to see if it was greater than the stack size before doing so. This is
an off by 2 error as the write will be at the size - 1 and the
compare is done before the increment. Adjust the check accordingly.
* tag 'trace-v7.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace:
tracing: Check operator stack capacity before pushing
tracing: Keep the predicate limit separate from program capacity
tracing: Fix crash in print_graph_function_event()
tracing/histograms: Reject non-stacktrace fields for synthetic stacktrace fields
ring-buffer: Use subbuf_order in available memory estimation
ring-buffer: Skip si_mem_available() for persistent and remote buffers
eventfs: Free the eventfs_inode after an rcu grace period too
tracing: Do not initialize tracefs work if tracing is disabled
tracing: Fix hist trigger timestamps for buffered events
tracing: Fix ignored return value of resize_buffer_duplicate_size()
ring-buffer: Fix false warning in ring_buffer_map_get_reader()
tracing: Fix simple_ring_buffer undef check on sparc64
tracing: Fix simple_ring_buffer undef check on parisc
tracing: Fix empty symbol names in simple_ring_buffer undef check
ring-buffer: Fix the sub-buffer array base in rb_meta_subbuf_idx()
tracing: Fix the shift out of bounds in the log2 histogram modifier
ring-buffer: Do not show buffer_meta for CPUs without a buffer
tracing: Take trace_types_lock when dropping the buffer_meta reference
tracing: Fix NULL dereference when copying keys for a field variable
predicate_parse() uses the first operator stack entry before processing
any opening parentheses. A push then increments top before writing.
The existing check still permits that write when top already points to
the last allocated entry, which can access beyond the operator stack.
Reject the push when top reaches the last allocated entry.
Cc: [email protected]
Fixes: 39f7c41c90 ("tracing/filters: Enable filtering a cpumask field by another cpumask")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Taeyang Lee <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
predicate_parse() adds two to nr_preds to reserve the terminal TRUE
and FALSE entries. It also uses that increased value as the limit for
parsed predicates. If the initial count is smaller than the number
actually parsed, the program can fill the allocation and subsequent
accesses can go past its end.
Keep nr_preds as the predicate limit and use a separate value for the
allocation size. This leaves space for both terminal entries and a
zeroed entry when parsing stops at the limit.
Cc: [email protected]
Fixes: 39f7c41c90 ("tracing/filters: Enable filtering a cpumask field by another cpumask")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Taeyang Lee <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
tracing_set_tracer() assigns the flags of the newly selected tracer
with:
tr->current_trace_flags = t->flags ? : t->tracer->flags;
If the new tracer defines neither static flags nor default flags, like
hwlat, irqsoff or wakeup, add_tracer() has no per instance flags copy
for it, and both operands are NULL, so tr->current_trace_flags ends up
being NULL as well.
The ring buffer is not cleared when switching tracers: it is reset
inside tracer_init(), after current_trace_flags was already assigned.
If the buffer still holds function_graph entries of the previous
tracer, printing one of them dereferences the NULL pointer. This was
hit by cat trace during an ftrace stress test:
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor read access in kernel mode
RIP: 0010:print_graph_function_event+0xe/0x20
Call Trace:
s_show+0x4d/0x110
seq_read_iter+0x2d4/0x490
seq_read+0x130/0x170
vfs_read+0xb5/0x370
ksys_read+0x6d/0xe0
do_syscall_64+0xbc/0x450
The snapshot buffer is not reset at all, so this can be reproduced
without a race:
echo function_graph > /sys/kernel/tracing/current_tracer
echo 1 > /sys/kernel/tracing/snapshot
echo hwlat > /sys/kernel/tracing/current_tracer
cat /sys/kernel/tracing/snapshot
Rather than teaching every user of tr->current_trace_flags about this
corner case, make sure it is never NULL: when the newly selected
tracer has no flags at all, tracing_set_tracer() falls back to a
static empty no_tracer_flags. run_tracer_selftest() can hit the same
case, so the same fallback is added there too. The existing users
already handle opts == NULL, so these tracers keep behaving as before.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: e29aa918a9 ("tracing: Have function graph tracer define options per instance")
Suggested-by: Steven Rostedt <[email protected]>
Signed-off-by: Hao Ge <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
In check_synth_field(), when a synthetic event field is a stacktrace
(field->is_stack) and the histogram field is not a "long[" stacktrace
array, validation falls through to the generic size and signedness
comparison.
Because parse_synth_field() sets a stacktrace field's type to
"__data_loc unsigned long[]", its size is sizeof(u64) and
synth_field_signed() marks it as signed. Consequently, any 8-byte
signed scalar histogram field passes the fallback check, and its scalar
value is subsequently dereferenced as a kernel stacktrace pointer in
get_field_size() and trace_stack().
Furthermore, checking strstr(hist_field->type, "long[") is insufficient
because tracepoint fields defined with __bitmask() or __rel_bitmask()
also have type "__data_loc unsigned long[]" or "__rel_loc unsigned
long[]", whilst create_hist_field() treats them as 4-byte scalar values
without HIST_FIELD_FL_STACKTRACE.
Propagate HIST_FIELD_FL_STACKTRACE to variable references and field
variables (whilst excluding variable aliases from per-element
stacktrace capture), and reject any mismatch between
HIST_FIELD_FL_STACKTRACE and field->is_stack in check_synth_field().
Cc: [email protected]
Fixes: 00cf3d672a ("tracing: Allow synthetic events to pass around stacktraces")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Lee Jones <[email protected]>
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
To avoid failing in the middle of an allocation of a ring-buffer,
__rb_allocate_pages() looks at the available memory estimation with
si_mem_available(). Use the subbuffer order with this estimation to fail
early if necessary for ring-buffers with an order > 0.
Link: https://patch.msgid.link/[email protected]
Fixes: f9b94daa54 ("ring-buffer: Set new size of the ring buffer sub page")
Reviewed-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Vincent Donnefort <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
traceprobe_expand_meta_args expands probe arguments based on BTF info.
Return an error if we have arguments that need expanding but BTF is not
supported.
The params variable is now unused, it can be removed.
Link: https://lore.kernel.org/all/[email protected]/
Fixes: d157d76944 ("tracing/probes: Support BTF field access from $retval")
Signed-off-by: Martin Kaiser <[email protected]>
Reviewed-by: Steven Rostedt <[email protected]>
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
Persistent and remote buffer memory is pre-allocated by the time
__rb_allocate_pages() is called. Skip si_mem_available() to avoid
wrongly failing the allocation under memory pressure.
For the same reason, unset user_thread.
Link: https://patch.msgid.link/[email protected]
Fixes: be68d63a13 ("ring-buffer: Add ring_buffer_alloc_range()")
Fixes: 2e67fabd8b ("ring-buffer: Introduce ring-buffer remotes")
Signed-off-by: Vincent Donnefort <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
If trace buffer allocation fails during early boot (e.g. in
early_trace_init() -> allocate_trace_buffers()), tracing_disabled is
left set to 1 and the global trace array events list remains
uninitialized.
Later during boot, tracer_init_tracefs() (invoked via fs_initcall)
queues tracerfs_init_work asynchronously without checking if tracing was
disabled. When tracer_init_tracefs_work_func() runs, it calls
event_trace_init() and init_tracer_tracefs(&global_trace, NULL)
unconditionally. This leads to __find_event_file() dereferencing an
uninitialized tr->events list, causing an early boot NULL pointer
dereference Oops:
BUG: Kernel NULL pointer dereference on read at 0x00000010
Faulting instruction address: 0xc000000000485be0
Oops: Kernel access of bad area, sig: 7 [#1]
...
NIP [c000000000485be0] __find_event_file+0x70/0x3c0
LR [c000000000448874] init_tracer_tracefs+0x274/0xc80
Call Trace:
init_tracer_tracefs+0x274/0xc80
tracer_init_tracefs_work_func+0x50/0x320
process_one_work+0x1e8/0x5c0
worker_thread+0x1dc/0x3d0
kthread+0x194/0x1b0
start_kernel_thread+0x14/0x18
Check tracing_disabled at the beginning of tracer_init_tracefs_work_func()
and return immediately if tracing is disabled or failed to initialize,
preventing the crash.
Cc: [email protected]
Fixes: 6621a70046 ("tracing: make tracer_init_tracefs initcall asynchronous")
Link: https://patch.msgid.link/[email protected]
Reported-by: Michal Suchánek <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Amit Machhiwal <[email protected]>
Tested-by: Michal Suchánek <[email protected]>
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Syzkaller triggered a WARN_ON_ONCE(!nest) warning:
WARNING: kernel/trace/ring_buffer.c:821 at ring_buffer_event_time_stamp
Call trace:
ring_buffer_event_time_stamp
hist_field_timestamp
hist_fn_call
event_hist_trigger
event_triggers_call
__event_trigger_test_discard
trace_event_buffer_commit
do_trace_event_raw_event_sched_switch
trace_event_raw_event_sched_switch
The warning is probabilistically reproducible and is related to syzkaller
executing the following commands:
echo 'prev_pid == 999999' > events/sched/sched_switch/filter
echo 'hist:keys=common_timestamp' > events/sched/sched_switch/trigger
The specific triggering process is as follows:
CPU 0 CPU 1
(context switching) (echo 'hist:keys=common_timestamp' > \
events/sched/sched_switch/trigger)
do_trace_event_raw_event_##call
trace_event_buffer_lock_reserve
if (!tr->no_filter_buffering_ref &&
trace_file->flags & EVENT_FILE_FL_FILTERED)
return entry; // return directly
event_hist_trigger_parse
hist_register_trigger
tracing_set_filter_buffering(file->tr, true);
tr->no_filter_buffering_ref++;
__trace_buffer_lock_reserve
rb_start_commit
local_inc(&cpu_buffer->committing); // skipped and not executed
trace_event_buffer_commit
__event_trigger_test_discard
event_triggers_call
event_hist_trigger
hist_fn_call
hist_field_timestamp
ring_buffer_event_time_stamp
nest = local_read(&cpu_buffer->committing);
WARN_ON_ONCE(!nest) // trigger warning
The root cause is that when an event file has a filter attached and the
hist trigger has not yet been attached, trace_event_buffer_lock_reserve()
writes the event into the per-CPU trace_buffered_event temp buffer instead
of reserving it in the ring buffer, so cpu_buffer->committing is not
incremented. When the event is later processed at commit time by a hist
trigger, hist_field_timestamp() -> ring_buffer_event_time_stamp() sees a
zero committing count, triggering WARN_ON_ONCE(!nest).
tracing_event_time_stamp() was added by commit d8279bfc5e ("tracing:
Add tracing_event_time_stamp() API") for exactly this case: if the event
is the per-CPU trace_buffered_event, it returns the current ring buffer
timestamp. But it never gained a caller. Use it in hist_field_timestamp(),
so that a hist trigger that races onto a buffered event records the
current time, which is only marginally later than when the event was
recorded, instead of warning and recording a bogus timestamp.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: b94bc80df6 ("tracing: Use a no_filter_buffering_ref to stop using the filter buffer")
Signed-off-by: Tengda Wu <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
When tr->allocated_snapshot is true, resize_buffer_duplicate_size() is
called to update the snapshot buffer size. However, its return value was
not checked. In case of a failed resize this could lead next snapshot
being incomplete.
Add resize_buffer_duplicate_size() result check and return error if
resize fails.
Compile tested only.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 180e4e3909 ("tracing: Add snapshot refcount")
Signed-off-by: Dmitry Kandybka <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
The mmap reader can warn when it catches up with a writer that is still
committing events. rb_get_reader_page() returns NULL in this case, but
ring_buffer_map_get_reader() treats that result as an error.
The initial rb_per_cpu_empty() check filters out an empty buffer, but it
does not guarantee that the next reader page has committed data. Writers
do not take reader_lock and can advance commit_page before publishing the
committed length on the new page. rb_get_reader_page() can swap to that
page, catch up with the writer and return NULL.
Handle NULL through the existing no-data exit without warning. Remove the
caller's reader_page == commit_page check as well, since the reader-page
helper already handles that case. The existing metadata update and return
path remain in place.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 117c39200d ("ring-buffer: Introducing ring-buffer mapping functions")
Assisted-by: Gemini:gemini-3.8-flash syzbot
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=de3d7f9bcc9212f3fae1
Link: https://syzkaller.appspot.com/ai_job?id=488adc18-a10e-42d2-a205-25327c38837f
Signed-off-by: Krystian Kaniewski <[email protected]>
Reviewed-by: Vincent Donnefort <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
On a persistent ring buffer, rb_meta_subbuf_idx() can return an index one
sub-buffer too high. rb_setup_ids_meta_page() then fills subbuf_ids[]
shifted by one and leaves subbuf_ids[0] NULL, which __rb_map_vma()
dereferences when userspace maps trace_pipe_raw. rb_update_meta_reader()
writes two such indices into meta->buffers[], which rb_cpu_meta_valid()
would reject on the next boot, discarding the previous boot's trace.
A CPU's sub-buffer array starts after its meta header and the nr_subbufs
integers that follow, aligned up to a sub-buffer. rb_meta_subbuf_idx()
inverts that calculation but skips only the integers, not the header, so
its base is one sub-buffer low whenever the omitted header size crosses
the alignment boundary.
Invert the calculation with rb_subbufs_from_meta() rather than repeating
it. rb_range_buffer() maps an index back to an address through the same
helper, so the two directions can no longer disagree.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: b14d032973 ("ring-buffer: Add ring_buffer_meta data")
Signed-off-by: Donggeun Yoo <[email protected]>
Reviewed-by: Vincent Donnefort <[email protected]>
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
The .log2 key modifier files a value under its base-2 order:
# echo 'hist:keys=bytes_req.log2' > events/kmem/kmalloc/trigger
hist_field_log2() computes that as ilog2(roundup_pow_of_two(val)). The
out-of-line form of roundup_pow_of_two() is
return 1UL << fls_long(n - 1);
which shifts by 64 on a 64-bit kernel both when n is 0, where n - 1
wraps to ULONG_MAX, and when n is above 2^63. Both are ordinary keys,
because the field fetch hands every value over as a u64: a syscall
argument of 0 reaches the first, and one above 2^63 reaches the second.
Keying sys_enter_lseek on offset.log2 and passing 0, 1, 2, 3, 1000,
2^63+1 and U64_MAX gives
{ offset: ~ 2^1 } hitcount: 1
{ offset: ~ 2^10 } hitcount: 1
{ offset: ~ 2^2 } hitcount: 1
{ offset: ~ 2^0 } hitcount: 4
Keys above 2^63 land in the lowest bucket. With CONFIG_UBSAN_SHIFT=y
the shift is also reported:
UBSAN: shift-out-of-bounds in include/linux/log2.h:57:13
shift exponent 64 is too large for 64-bit type 'long unsigned int'
__ubsan_handle_shift_out_of_bounds.cold+0xdd/0x1cb
hist_fn_call.cold+0x8b/0xd9
event_hist_trigger+0x1cc/0x790
ftrace_syscall_enter+0x197/0x360
do_syscall_64+0x402/0x4b0
Use order_base_2(), which is ilog2(n - 1) + 1 for n > 1 and 0 below
that. It returns what the old expression returned on every input the
old one was defined for. Where it was not, 0 stays in bucket 0, which
is where x86_64 had been putting it, and the keys above 2^63 move into
bucket 64.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 4b94f5b7b4 ("tracing: Add hist trigger 'log2' modifier")
Signed-off-by: Donggeun Yoo <[email protected]>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Steven Rostedt <[email protected]>
The per_cpu/cpuN/buffer_meta files are created for every possible CPU,
but buffer->buffers[cpu] is only allocated once that CPU comes online.
Reading the file of a CPU that never came online dereferences a NULL
cpu_buffer in rbm_start().
Fail the open with -ENODEV when the CPU has no buffer, as the other
per CPU ring buffer interfaces do.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 950032ffce ("ring-buffer: Add output of ring buffer meta page")
Reported-by: Sashiko <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: David Carlier <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
tracing_buffer_meta_open() drops the trace_array reference with
__trace_array_put() when ring_buffer_meta_seq_init() fails, without
holding trace_types_lock. Use trace_array_put() like every other open
error path.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 950032ffce ("ring-buffer: Add output of ring buffer meta page")
Signed-off-by: David Carlier <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
In event_hist_trigger_parse() where it needs to create actions like
"onmatch", it calls:
event_hist_trigger_parse() {
create_actions() {
action_create() {
trace_action_create() {
trace_action_create_field_var() {
create_field_var_hist()
Where create_field_var_hist() does a loop on the hist_data
representing the keys. The issue is, if the keys uses one of the
pseudo field types (like common_cpu), the hist_data field element
will have NULL for its field member causing a NULL pointer
dereference when accessing the key_field->field->name.
# echo 'hist:keys=common_cpu:ts0=common_timestamp.usecs' > \
events/sched/sched_waking/trigger
# echo 'my_synth u64 lat; int prio' > synthetic_events
# echo 'hist:keys=common_cpu:wakeup_lat=common_timestamp.usecs-$ts0:\
onmatch(sched.sched_waking).my_synth($wakeup_lat,prio)' > \
events/sched/sched_switch/trigger
KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
RIP: 0010:create_field_var_hist+0x771/0x1380
Call Trace:
trace_action_create_field_var+0x360/0x600
trace_action_create+0x393/0xe80
event_hist_trigger_parse+0x3e47/0x69e0
trigger_process_regex+0x1a6/0x250
event_trigger_write+0xce/0x160
Instead of accessing it directly, use the proper handler
expr_field_str() to get the name.
The three previous patches are prerequisites: without them a .buckets
key and a common_stacktrace key each render into a command that cannot
be parsed back.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 02205a6752 ("tracing: Add support for 'field variables'")
Signed-off-by: Donggeun Yoo <[email protected]>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Steven Rostedt <[email protected]>
unregister_fprobe() and unregister_fprobe_async() (used by BPF
kprobe-multi) rely on standard RCU grace periods (synchronize_rcu()
and call_rcu()) to wait until in-flight fprobe handlers complete before
freeing the fprobe.
However, if an fprobe handler executes while RCU is not watching (such
as in the idle loop or nohz_full extended quiescent states), standard
RCU does not track preemption-disabled sections. Consequently,
synchronize_rcu() does not wait for those executions, which can lead
to a use-after-free if the fprobe is freed immediately after
unregistration. Ensure handlers exit early when !rcu_is_watching().
Furthermore, fprobe_fgraph_entry() and fprobe_ftrace_entry() previously
used guard(rcu)() and rcu_read_lock(), which invoke lockdep on every
hit under CONFIG_PROVE_LOCKING. This adds overhead and can cause lockdep
recursion if probed functions interact with lockdep.
Since rhltable_lookup() and rhl_for_each_entry_rcu() use
rcu_dereference_all_check() (which checks rcu_read_lock_any_held()),
holding preemption disabled via rcu_read_lock_sched_notrace() is fully
valid and sufficient so long as rcu_is_watching() is true.
Define and use guard(rcu_sched_notrace)() across fprobe_ftrace_entry(),
fprobe_fgraph_entry(), and fprobe_return(). This eliminates fast-path
rcu_read_lock() and lockdep overhead while guaranteeing safe grace
period synchronization.
Link: https://lore.kernel.org/all/179064115227.394389.16910234241400391996.stgit@devnote2/
Reported-by: Sashiko <[email protected]>
Closes: https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3
Fixes: 657b594b20 ("fprobe: Fix unregister_fprobe() to wait for RCU grace period")
Cc: [email protected]
Assisted-by: LLM
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
Reviewed-by: Paul E. McKenney <[email protected]>
Pull probe fixes from Masami Hiramatsu:
- kprobes: Fix permanent hang when flushing the kprobe optimizer
Fix a deadlock when disabling kprobe optimization via sysctl or
debugfs where flushers hung waiting for optimizer_completion.
Replaced the completion with an optimizer_passes counter and
wait_var_event_mutex() under kprobe_mutex so concurrent flushers can
wait and wake up safely.
- fprobe: Terminate the fgraph_data list when the reservation is not
filled
Fix an issue where unused shadow stack data left uninitialized by
fprobe_fgraph_entry() was misparsed as stale fprobe headers on
return. Explicitly write a zero word to terminate the list and update
read_fprobe_header() to handle the zeroed slot properly.
- ftracetest: Fix unique symbol check in kprobe_non_uniq_symbol.tc
Fix false test failures in kprobe_non_uniq_symbol.tc on architectures
like s390 where a symbol exists once in core kernel but also in
modules. Anchor the /proc/kallsyms search regex to the end of the
line so that module symbols are not incorrectly counted.
* tag 'probes-fixes-v7.3-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace:
kprobes: Fix permanent hang when flushing the kprobe optimizer
fprobe: Terminate the fgraph_data list when the reservation is not filled
selftests/ftrace: Fix unique symbol check in kprobe_non_uniq_symbol.tc
fprobe_fgraph_entry() reserves shadow stack space for every fprobe with
an exit handler, but only fills it for those whose entry handler returns
0. fgraph_reserve_data() does not clear the area, so fprobe_return()
parses the unused tail as headers left over from an earlier call, and an
exit handler can run twice or despite its entry handler asking to skip
it.
Write a zero word after the last entry to terminate the walk. A zeroed
slot does not decode to a NULL fprobe on the arches that encode the
header into one unsigned long, since arch_decode_fprobe_header_fp() ORs
in FPROBE_HEADER_MSB_PATTERN, so make read_fprobe_header() return NULL
for a zeroed slot.
Link: https://lore.kernel.org/all/[email protected]/
Fixes: e0a384434a ("tracing: fprobe: do not zero out unused fgraph_data")
Cc: [email protected]
Suggested-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: David Carlier <[email protected]>
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
ring_buffer_subbuf_order_set() stores the new order and only then walks
the CPUs, returning -EBUSY if any of them has resizing disabled. A user
mapped buffer has resizing disabled, and __rb_map_vma() reads
buffer->subbuf_order without buffer->mutex, so an mmap of an already
mapped CPU racing the failing order change sizes the mapping with the
new order and inserts pages past the sub-buffer into the VMA.
Check the CPUs before storing the new order.
Cc: [email protected]
Fixes: 117c39200d ("ring-buffer: Introducing ring-buffer mapping functions")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: David Carlier <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
The number of pages per remote ring buffer is capped by
ring_buffer_desc::nr_page_va (32 bits). A buffer_size large enough to
overflow that field would silently allocate a descriptor smaller than
what was asked for.
Return SIZE_MAX from trace_buffer_desc_size() on nr_page_va overflow.
Link: https://patch.msgid.link/[email protected]
Fixes: 2e67fabd8b ("ring-buffer: Introduce ring-buffer remotes")
Signed-off-by: Vincent Donnefort <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
trace_buffer_desc_size() and trace_remote_alloc_buffer() undercount the
required pages because every ring buffer page contains a header
(BUF_PAGE_HDR_SIZE). Account for that header to ensure allocated remote
ring buffers aren't smaller than requested by the user.
The newly introduced helper __calc_nr_pages_ring_buffer_desc() can
return a value that overflows the descriptor nr_pages field (32 bits).
Link: https://patch.msgid.link/[email protected]
Fixes: 2e67fabd8b ("ring-buffer: Introduce ring-buffer remotes")
Signed-off-by: Vincent Donnefort <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
rb_wake_up_waiters() is a irq_work callback which is initialized with
init_irq_work(). As such it will be invoked in thread context on
PREEMPT_RT. Invoking the callback in IRQ context on PREEMPT_RT is not an
option due its usage of wake_up_all(). Since this callback may run in
thread context, it needs to acquire ring_buffer_per_cpu::reader_lock with
disabling interrupts and may not assume that they are disabled.
Use raw_spinlock_irqsave() to acquire ring_buffer_per_cpu::reader_lock.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 68282dd930 ("ring-buffer: Fix resetting of shortest_full")
Reviewed-by: Vincent Donnefort <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
When a tracer option file is opened, it is passed a descriptor that points
to an element on the trace_array's topts array. This element has
information to find the trace array and other information. It uses this
element to take a reference of the trace_array so that the trace_array
does not get removed while this file is opened.
Unfortunately, there's a race condition where the element itself could be
freed by the removal of the instance the trace_array represents causing a
use-after-free as this element that is used to find the trace_array to
increment its reference counter is also freed when the instance is
removed.
To solve this, add a trace_array_tracer_options_get() helper function that
will take the address of the element that is passed to the open function
by the inode->i_private pointer and search all the trace_arrays under a
lock to find the one that the element's address is in the range of the
trace_arrays topts array elements. When a match happens, that trace_array's
reference would be increased.
Note, there's a race where if an admin was deleting and creating trace
instances at the same time and the memory of the old trace_array's array
matched the memory of the new trace_array that it could in theory open the
option from the wrong trace array. But we do not care because it would be
stupid to perform that kind of action. As long as the only thing that can
happen is that the option from the wrong trace array is used and doesn't
crash the kernel it will only make the user confused. But if they are
doing something stupid like this, they are already confused, so no harm
done.
Cc: [email protected]
Link: https://patch.msgid.link/20260910221209.62dad8d3@robin
Fixes: 7e2cfbd2d3 ("tracing: Have option files inc the trace array ref count")
Reported-by: [email protected]
Closes: https://lore.kernel.org/linux-trace-kernel/[email protected]/
Signed-off-by: Steven Rostedt <[email protected]>
ring_buffer_read_page_size() takes a parameter named rpage, but its
kernel-doc describes page. As a result, kernel-doc reports rpage as
undescribed and page as an excess parameter description.
Rename the documentation entry to match the function.
Link: https://patch.msgid.link/[email protected]
Fixes: dae8dda341 ("tracing: Fix subbuf resize races with trace_pipe_raw readers")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <[email protected]>
Reviewed-by: Vincent Donnefort <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
While ftrace_set_clr_event() modifies its input buffer during parsing,
before returning to the caller the buffer is supposed to be restored
to its original state.
This works correctly for the colon between the subsystem and event
but not the colon at the beginning of :mod:.
Restore the colon, so the :mod: trailer is not stripped after
ftrace_set_clr_event().
Cc: [email protected]
Fixes: 4c86bc531e ("tracing: Add :mod: command to enabled module events")
Link: https://patch.msgid.link/20260908-tracing-cli-event-filter-v2-1-05396a3fb663@linutronix.de
Signed-off-by: Thomas Weißschuh <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
"cpu", "CPU", "stacktrace" and "STACKTRACE" are generic fields, defined
with an offset and a size of zero so that the filter code can match them
by name. parse_field() maps them onto their common_* equivalents for
backward compatibility, but unlike the common_* names it hands the
placeholder back to the caller instead of NULL.
create_hist_field() takes a non-NULL field as a promise that the record
carries a stacktrace and picks HIST_FIELD_FN_STACK, so the __data_loc
word is read from offset 0, that is from common_type, and its low 16
bits are followed as an offset into the record. What is found there
becomes the length of an unbounded memcpy. Pick an event whose id is
small enough that the offset stays inside its own record and the length
is a kernel text address:
# cd /sys/kernel/tracing
# echo 'hist:keys=STACKTRACE' > events/ftrace/print/trigger
# echo hello > trace_marker
Oops: general protection fault, probably for non-canonical address
RIP: 0010:rb_next+0x23/0x60
</IRQ>
RIP: 0010:memcpy+0xc/0x30
event_hist_trigger+0x2e7/0x12c0
Kernel panic - not syncing: Fatal exception in interrupt
Leave the field NULL, which is what the comment above the branch says
the code does and what common_stacktrace already does. FILTER_CPU and
FILTER_COMM are left alone, their create_hist_field() branches never
look at the field.
Cc: [email protected]
Fixes: 4b512860bd ("tracing: Rename stacktrace field to common_stacktrace")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
parse_field() sets HIST_FIELD_FL_STACKTRACE from the ".stacktrace"
modifier before it looks the field name up, and nothing afterwards
checks that the name resolved to a field which holds a stacktrace.
create_hist_field() picks HIST_FIELD_FN_STACK on the strength of the
field pointer alone, which reads a __data_loc word from the record and
follows its low 16 bits as an offset into the same record.
event_hist_trigger() takes the first word there as an entry count and
copies that many longs into a 31 entry array:
n_entries = *stack;
memcpy(entries, ++stack, n_entries * sizeof(unsigned long));
Neither end of that copy is bounded, and the count is whatever the event
holds at the offset, so any field will do:
# cd /sys/kernel/tracing/events/sched/sched_process_fork
# echo 'hist:keys=parent_pid.stacktrace' > trigger
# (true)
BUG: kernel NULL pointer dereference, address: 0000000000000008
RIP: 0010:rb_insert_color+0x18/0x130
timerqueue_linked_add+0x7e/0xd0
enqueue_hrtimer+0x39/0xb0
__hrtimer_run_queues+0x10f/0x1f0
</IRQ>
RIP: 0010:memcpy+0xc/0x30
event_hist_trigger+0x165/0x690
The timer interrupt landed on the rbtree the copy had already run over.
No debug options are needed for this; KASAN reports the same write as an
out-of-bounds read of 13835058055416381440 bytes.
Documentation/trace/histogram.rst already states the rule, "must be a
long[] type", so enforce it once the name has been resolved. Names which
resolve to no field at all, "hitcount.stacktrace" and the common_*
pseudo-fields, are refused for the same reason: they hold no stacktrace
to read.
Cc: [email protected]
Fixes: cc5fc8bfc9 ("tracing/histogram: Add stacktrace type")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Commit 6f86bdeab6 ("tracing: Fix bad hist from corrupting named_triggers
list") described how a trigger that is registered but not on file->triggers
ends up freed while still on the global named_triggers list, and moved the
registration down so that hist_trigger_enable() follows it immediately. One
path still gets there. hist_trigger_enable() adds the trigger and takes it
straight back out when the event cannot be enabled:
list_add_tail_rcu(&data->list, &file->triggers);
update_cond_flag(file);
if (trace_event_trigger_enable_disable(file, 1) < 0) {
list_del_rcu(&data->list);
update_cond_flag(file);
ret--;
}
so the list walk in hist_unregister_trigger() matches nothing, test stays
NULL, and the ->free() that would call del_named_trigger() is skipped.
out_unreg falls through to out_free, which frees the trigger anyway:
BUG: KASAN: slab-use-after-free in find_named_trigger+0xac/0xc0
Read of size 8 at addr ffff8880091d3160 by task init/1
find_named_trigger+0xac/0xc0
hist_register_trigger+0xc1/0xa00
event_hist_trigger_parse+0x3146/0x6af0
event_trigger_write+0xce/0x160
Freed by task 69:
kfree+0x154/0x420
trigger_kthread_fn+0xfd/0x160
Leave the trigger where hist_unregister_trigger() can find it and let that
undo the registration, which is the only code that knows all of what
cmd_ops->init() took: the named list entry, the hist_pad reference, the
reference on the trigger a named histogram is shared with, and the copied
cmd_ops. It also pairs the failed trace_event_trigger_enable_disable(),
whose sm_ref and buffered event reference are otherwise left behind.
Since ->free() releases trigger_data and, for a trigger that does not share
its histogram, hist_data with it, out_unreg can no longer fall through to
out_free. For a trigger that does share, hist_register_trigger() has
already destroyed the caller's hist_data, so the fall-through was reading
freed memory there as well.
Move the enable_timestamps check in hist_unregister_trigger() above the
->free() call for the same reason: hist_data does not outlive it once the
trigger being removed is the one that owns it.
Cc: [email protected]
Fixes: 067fe038e7 ("tracing: Add variable reference handling to hist triggers")
Reported-by: Sashiko AI <[email protected]>
Closes: https://lore.kernel.org/linux-trace-kernel/[email protected]/
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
event_hist_trigger_named_init() puts the trigger on the global
named_triggers list and only then takes the reference on the trigger it
shares its histogram with:
data->ref++;
save_named_trigger(data->named_data->name, data);
ret = event_hist_trigger_init(data->named_data);
if (ret < 0) {
kfree(data->cmd_ops);
data->cmd_ops = &trigger_hist_cmd;
}
return ret;
event_hist_trigger_init() fails when alloc_hist_pad() cannot allocate, and
nothing takes the trigger back off the list on the way out.
event_hist_trigger_parse() frees it, and the next lookup by name reads the
freed object:
BUG: KASAN: slab-use-after-free in find_named_trigger+0xac/0xc0
Read of size 8 at addr ffff888009346860 by task init/1
find_named_trigger+0xac/0xc0
hist_register_trigger+0xc1/0xa00
event_hist_trigger_parse+0x3146/0x6af0
event_trigger_write+0xce/0x160
Freed by task 67:
kfree+0x154/0x420
trigger_kthread_fn+0xfd/0x160
Do the reference first and publish once it has succeeded, so that nothing
which can fail runs after the trigger becomes findable.
Cc: [email protected]
Fixes: 7ab0fc61ce ("tracing: Move histogram trigger variables from stack to per CPU structure")
Reported-by: Sashiko AI <[email protected]>
Closes: https://lore.kernel.org/linux-trace-kernel/[email protected]/
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Acked-by: Tom Zanussi <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
hist_register_trigger() puts the trigger on the global named_triggers
list in cmd_ops->init(), and only then sets the trace clock:
if (data->cmd_ops->init) {
ret = data->cmd_ops->init(data);
if (ret < 0)
goto out;
}
if (hist_data->enable_timestamps) {
ret = tracing_set_clock(file->tr, hist_data->attrs->clock);
if (ret) {
hist_err(tr, HIST_ERR_SET_CLOCK_FAIL, errpos(clock));
goto out;
}
The clock string is not checked anywhere before that call, so a named
trigger using common_timestamp with an unknown clock fails after it has
already become findable. event_hist_trigger_parse() then frees it
without taking it off the list, and the next lookup by name reads the
freed object:
~# cd /sys/kernel/tracing/events/sched/sched_switch
~# echo 'hist:name=foo:keys=common_pid:ts=common_timestamp:clock=bogus' > trigger
bash: echo: write error: Invalid argument
~# echo 'hist:name=foo:keys=common_pid' > trigger
BUG: KASAN: slab-use-after-free in find_named_trigger+0xac/0xc0
Read of size 8 at addr ffff88800915d760 by task init/1
find_named_trigger+0xac/0xc0
hist_register_trigger+0xc1/0x900
event_hist_trigger_parse+0x3146/0x6af0
event_trigger_write+0xce/0x160
Freed by task 63:
kfree+0x154/0x420
trigger_kthread_fn+0xfd/0x160
Set the clock before the trigger is registered, so that nothing which
can fail runs after it is published, the way commit 6f86bdeab6
("tracing: Fix bad hist from corrupting named_triggers list") moved the
registration below the rest of the setup.
tracing_set_filter_buffering() is reference counted, so the init failure
path has to drop the reference that the clock block now takes first.
Cc: [email protected]
Fixes: a4072fe85b ("tracing: Add a clock attribute for hist triggers")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Correct "availabe" to "available", reported by scripts/checkpatch.pl using
the misspelling list in scripts/spelling.txt. Only touches comments, no
code changes.
Link: https://patch.msgid.link/[email protected]
Assisted-by: Cursor:claude-opus-5
Signed-off-by: Hemanth Selam <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
The .percent and .graph modifiers exist only for histogram values, but a
value carrying either of them has been rejected since v6.3. The example
in Documentation/trace/histogram.rst,
# echo 'hist:keys=prev_comm:vals=hitcount.percent:nohitcount' > \
events/sched/sched_switch/trigger
returns -EINVAL.
parse_field() sets the two flags only when the field is neither a key nor
a variable, that is, only on a value:
} else if (strncmp(modifier, "percent", 7) == 0) {
if (*flags & (HIST_FIELD_FL_VAR | HIST_FIELD_FL_KEY))
goto error;
*flags |= HIST_FIELD_FL_PERCENT;
__create_val_field() then rejects a value for carrying them, so no field
can reach hist_trigger_print_val(), where both are implemented.
commit e0213434fe ("tracing: Do not let histogram values have some
modifiers") added the check after a value with .buckets oopsed in
hist_field_name(). That happens because .buckets and .log2 make
create_hist_field() build a nested field in operands[0] which
hist_field_name() then walks into. The percent and graph flags do not
create an operand and are not read by hist_field_name(); they are only
used when printing a value.
Stop rejecting the two flags on a value. The check for variables is left
alone, where they are unreachable anyway because parse_field() rejects a
variable carrying them first.
With the two flags removed, the trigger above installs and prints as
documented:
{ prev_comm: rcu_preempt } hitcount (%): 0.00
{ prev_comm: init } hitcount (%): 99.98
Totals:
Hits: 237896
Cc: [email protected]
Fixes: e0213434fe ("tracing: Do not let histogram values have some modifiers")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
print_entries() uses n_entries both as the number of sort entries and as
its own return value, so the -ENOMEM it stores when the stats allocation
fails overwrites the count that the cleanup still needs:
n_entries = tracing_map_sort_entries(map, ...);
if (n_entries < 0)
return n_entries;
...
if (!stats) {
n_entries = -ENOMEM;
goto out;
}
...
out:
tracing_map_destroy_sort_entries(sort_entries, n_entries);
tracing_map_destroy_sort_entries() takes an unsigned int and loops up to
it, so -ENOMEM arrives as 4294967284. It walks an array of at most
map->max_elts pointers and calls destroy_sort_entry(), which dereferences
and frees, on whatever lies past the end.
Reading the hist file of a trigger with a .percent value, with that
allocation forced to fail:
BUG: KASAN: vmalloc-out-of-bounds in tracing_map_destroy_sort_entries+0xa0/0xb0
Read of size 8 at addr ffffc90000045000 by task init/1
tracing_map_destroy_sort_entries+0xa0/0xb0
hist_show+0x6f7/0x1df0
seq_read_iter+0x2b8/0x1190
vfs_read+0x176/0xa40
The buggy address belongs to a 4-page vmalloc region starting at
ffffc90000041000 allocated at tracing_map_sort_entries+0x5c/0xd50
A few pages further the fault is fatal. The registers at the oops confirm
the bound: the loop's end pointer less the array start, over the pointer
size, is 4294967284.
Return the error in a separate variable and leave n_entries holding the
count, the way tracing_map_sort_entries() does on its own error path.
The stats block is only entered for a value carrying .percent or .graph,
which __create_val_field() has rejected since v6.3, so this cannot be
reached in mainline as it stands. It becomes reachable again with
"tracing: hist: let values keep the percent and graph modifiers", so it
should be applied first.
Cc: [email protected]
Fixes: abaa5258ce ("tracing: Add .percent suffix option to histogram values")
Link: https://patch.msgid.link/[email protected]
Reported-by: [email protected]
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Donggeun Yoo <[email protected]>
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Writing a hist trigger whose value or variable carries a modifier that is
not allowed there leaks the fields that were built for it.
__create_val_field() takes the field from parse_expr() and stores it in
hist_data->fields[] only after the modifier checks have run:
hist_field = parse_expr(hist_data, file, field_str, flags, var_name,
&n_subexprs);
...
if (hist_field->flags & HIST_FIELD_FL_VAR) {
if (hist_field->flags & (...))
goto err;
} else {
if (hist_field->flags & (...))
goto err;
}
hist_data->fields[val_idx] = hist_field;
Both checks jump past that store, and the err label returns without
freeing anything. The error unwinds to create_hist_data(), which calls
destroy_hist_data() -> destroy_hist_fields(), and that reaches a field
only by walking fields[]. A field that never got there is unreachable.
commit e0213434fe ("tracing: Do not let histogram values have some
modifiers") set ret to -EINVAL and fell through to the store, which left
the field owned by fields[] and freed along with the rest of hist_data.
Splitting the check into a value case and a variable case replaced that
fall-through with a goto that skips it.
With CONFIG_DEBUG_KMEMLEAK, 200 writes of
# echo 'hist:keys=prev_pid:vals=next_pid.log2' > \
events/sched/sched_switch/trigger
each correctly rejected with -EINVAL, leave 332 unreferenced objects
(63744 bytes) reported at create_hist_field(); 200 install and remove
cycles of a valid trigger leave none. A '.log2' field is two
allocations, since create_hist_field() puts the plain field in
operands[0] of the log2 field, and both are reported.
Use destroy_hist_field() rather than __destroy_hist_field() so that
operands[0] is freed as well. It returns early for HIST_FIELD_FL_VAR_REF,
which is what an operand owned by hist_data->var_refs[] needs; the
rejected field itself is never a var ref, because a var ref never carries
a modifier flag.
Cc: [email protected]
Fixes: e30fbc618e ("tracing/histograms: Allow variables to have some modifiers")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
create_var_ref() allocates a VAR_REF hist_field and then calls
init_var_ref() to fill it in. When that fails the field is leaked.
commit 656fe2ba85 ("tracing: Use hist trigger's var_ref array to destroy
var_refs") made destroy_hist_field() return early for
HIST_FIELD_FL_VAR_REF, since var refs are freed by walking the trigger's
var_refs[] array instead. create_var_ref() adds the field to that array
only after init_var_ref() has succeeded, so on this path the field is in
neither place and nothing frees it. The call was correct when it was
written, before var refs were taken out of destroy_hist_field().
init_var_ref() cannot free it either. The caller owns the field, so
init_var_ref() undoes only its own string allocations and leaves the
field alone. Freeing it there would leave create_var_ref() passing freed
memory to destroy_hist_field(), which reads its flags.
Call __destroy_hist_field(), which frees the field without consulting
the flag.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 656fe2ba85 ("tracing: Use hist trigger's var_ref array to destroy var_refs")
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Using the same variable three or more times in one hist trigger leaks the
variable reference and its strings when the trigger is removed.
commit 656fe2ba85 ("tracing: Use hist trigger's var_ref array to destroy
var_refs") made a trigger's var_refs[] array the only owner of a var ref:
destroy_hist_field() returns early for HIST_FIELD_FL_VAR_REF, so the field
expressions never destroy one. One entry, freed once, no count needed.
commit 8bcebc77e8 ("tracing: Fix histogram code when expression has same
var as value") then made repeated references share one object and added a
count of them. Only the increment side exists, since those expressions
still return early and never drop a reference, so __destroy_hist_field()
sees how many references were created rather than how many are left. It
frees when the decremented count is 0 or 1, so two references work and
three or more leak.
Sharing kept one array entry per object, and create_var_ref() searches and
appends within a single trigger, so nothing outside it holds the object.
Removing a trigger whose variables are still referenced is already refused
by check_var_refs() with -EBUSY. Drop the count and free unconditionally.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 8bcebc77e8 ("tracing: Fix histogram code when expression has same var as value")
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
When a graph entry does not fit in the trace_seq, print_graph_entry()
saves it in the iterator's fgraph_data and reprints it on the next read.
The entry has already been consumed from the ring buffer by then, so the
copy is all that is left of it.
The copy is sized with iter->ent_size, which no longer describes the
saved entry but whatever entry the iterator has moved on to. The
argument count is derived from the same field, so a 72 byte entry saved
and then reprinted ahead of a 48 byte return entry loses its arguments.
Record the size next to the failure flag, so that the two are always set
together, and restore it before reprinting.
Cc: [email protected]
Fixes: ff5c9c576e ("ftrace: Add support for function argument to graph tracer")
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
FGRAPH_MAX_INDEX has no user, and it expands to FGRAPH_INDEX_SIZE and
FGRAPH_RET_INDEX, neither of which is defined anywhere in the tree. It
was added in that form by commit 91c46b0aa9 ("function_graph:
Implement fgraph_reserve_data() and fgraph_retrieve_data()"), which
introduced the current data word layout under new names, so anything
referencing it would have failed to build ever since.
Remove it.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
destroy_user_event() destroys the event's fields before attempting to
remove the trace event call. If user_event_set_call_visible() fails,
e.g. because the event is still enabled and trace_remove_event_call()
returns -EBUSY, the event is left registered with an irreversibly
destroyed field list. Any subsequent interaction with the event then
operates on an empty field list while it is still fully visible in
tracefs.
Move the field destruction after the call removal, and splice the
field list back onto the event when the removal fails so the event
remains in a consistent state.
Cc: [email protected]
Link: https://patch.msgid.link/[email protected]
Fixes: 7f5a08c79d ("user_events: Add minimal support for trace_event into ftrace")
Signed-off-by: Henry Martin <[email protected]>
Reviewed-by: Beau Belgrave <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Pull tracing fixes from Steven Rostedt:
- Fix several tracefs files that did not take the trace_array reference
A trace instance can be created and destroyed in the tracefs
"instances" directory via mkdir and rmdir respectively. The instance
is represented by a trace_array descriptor.
Most tracefs files pass the trace_array as the private data of the
inode to the open/read/write functions. Since there is no locking
between the time a task opens a file and the deletion of the instance
(and the freeing of the trace_array), each open needs to get a
reference to the trace_array and each close must remove it.
An instance can't be removed if there's any reference taken on its
trace_array. The open function uses trace_array_get() that takes a
lock (preventing removal of instances) and iterates the list of all
existing trace_arrays and if it finds a match, it takes the reference
and releases the lock. If it doesn't find a match, it causes the open
to return -ENODEV.
There were some added files that did not take the trace_array
reference on open that needed to be fixed. Sashiko also correctly
pointed out that there were some files that took an address of an
field or element of the trace_array which had a pointer back to the
trace_array to take its reference on open. But this leaves a slight
race between referencing this element to get the trace_array as the
element itself could be freed. To solve this, some helper functions
were created to look for trace_arrays with this field or element in
the search so that the element did not have to be dereferenced before
the trace_array's reference was taken.
- Add a lock around ftrace_ops initialization
When a ftrace_ops is first used by ftrace, some internal
initialization is performed on the ops. But if multiple tasks were
calling functions that did this initialization, it could race and
perform doing the initialization more than once, corrupting the
internal data. Add a lock in the initialization code to prevent this
from happening.
- Fix splice reads on mmapped buffers
The logic in the ring buffer splice code for mmapped buffers is
supposed to do a copy of the memory as the mapped buffers can't be
given to splice. But there was an if statement within the copy code
that would return a -1 if a request for a full page was done and it
wasn't a partial read. This is because this logic was written before
mmapped buffers existed and this case didn't make sense at the time.
For mmapped buffers it makes perfect sense and by returning early can
drop a lot of pages unnecessarily.
- Have the persistent ring buffer validation check nr_subbufs
Sashiko reported that the validation code was relying on the saved
nr_subbufs to match the calculated nr_pages + 1 and if they were off,
that the code could cause corruption. Sashiko is correct, and the
saved nr_subbufs should be validated before assuming it is correct.
- Do not allow more than one instance with the same name on cmdline
If an admin were to add more than one trace instances with the same
name they all would be created, but only the first one would be
accessible via tracefs. This used to not be allowed but some
restructuring of code has since made it possible.
- Fix the race between subbuf resize and trace_pipe_raw readers
If a task was reading trace_pipe_raw while another task was changing
the ring buffer subbuf size, it could crash the reader. The
trace_pipe_raw readers do get their own copy of the page from the
buffer, but the code needs some restructuring to not have the resize
of the subbuffers cause issues.
- Cap the size of the mapped (static) ring buffer nr_pages
The meta data used for ring buffer mapped buffers is 32 bit in size.
A normal ring buffer could (in theory) have more than 4 billion
pages. But this is not allowed by mapped buffers, so enforce it.
* tag 'trace-v7.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace:
ring-buffer: Use a macro for static buffer bits
tracing: Fix comment in tracing_buffers_splice_read()
ring-buffer: Prevent truncation of nr_pages / nr_subbufs
ring-buffer: Cap static ring buffer nr_pages
tracing: Fix subbuf resize races with trace_pipe_raw readers
tracing: Fix to avoid creating trace instances with duplicate names
ring-buffer: Add checking nr_subbufs to persistent ring buffer validation
ring-buffer: Allow splice reads on static buffers
tracing: Take trace_array reference when opening options file
ftrace: Synchronize the initialization of ftrace_ops
ftrace: Take trace_array reference before accessing its ftrace_ops
tracing: Have show_event_filters/triggers files take trace array ref
This is another run of the Coccinelle script for converting kmalloc()
family of allocations to kmalloc_obj() via the existing rules in
scripts/coccinelle/api/kmalloc_objs.cocci
This catches both the set of kmalloc() uses added since the first
kmalloc_obj() conversions in v7.0 and adds a large group missed in the
first pass due to Coccinelle not interacting well with the cleanup.h
scoped_...() family of macros[1]. I worked around this with spatch's
"--macro-file" argument to a file with all the scoped_...() macros mapped
to Coccinelle's YACFE_ITERATOR[2] as that was the closest viable control
flow indicator I could find.
Build tested allmodconfig on x86, arm64, arm, loongarch, mips, powerpc,
riscv, and s390 with no new warnings.
Link: https://lore.kernel.org/lkml/202609021314.8A9C0B8@keescook/ [1]
Link: https://github.com/coccinelle/coccinelle/blob/master/standard.h [2]
Signed-off-by: Kees Cook <[email protected]>
Instead of hard coding 30 for the number of bits used for the static
buffer ids in two places, create a macro. This way if it changes in the
future, it will change in all the locations that use it.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Steven Rostedt <[email protected]>
The comment about returning an error if the read fails on the first
iteration is slightly incorrect. It makes it sound like the only reason it
could fail on a later iteration is if the subbuf order changed. That is
incorrect, it could also fail if the length passed in was not a multiple
of the subbuf size. Fix the comment.
Link: https://lore.kernel.org/all/[email protected]/
Link: https://patch.msgid.link/[email protected]
Fixes: dae8dda341 ("tracing: Fix subbuf resize races with trace_pipe_raw readers")
Signed-off-by: Steven Rostedt <[email protected]>
Linus Torvalds