mirror of
https://github.com/torvalds/linux.git
synced 2026-10-23 15:32:04 +02:00
9ba46df344
7438 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Linus Torvalds
|
94905fcbd9 |
bpf: Use get_user() for kprobe_multi symbols and uprobe_multi arrays
copy_user_syms() and bpf_uprobe_multi_link_attach() check whole user arrays with access_ok() up front, then read them one element at a time with __get_user(), with other work in between: allocations and strncpy_from_user() in copy_user_syms(), validation in the uprobe_multi loop. Each of those accesses enables and disables user access on its own, so the up-front check saves nothing. On x86, __get_user() is even more expensive than get_user(): it does an lfence, where get_user() checks the address with a couple of plain instructions. Use get_user(). The access_ok() checks stay, because they also make a bad array fail with -EFAULT before anything else is looked up or allocated. Update the comment that said they were there for __get_user(). This is part of getting rid of __get_user() and __put_user() entirely. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
||
|
Linus Torvalds
|
e2c278bc37 |
tracing fixes for v7.3:
- 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. -----BEGIN PGP SIGNATURE----- iIoEABYKADIWIQRRSw7ePDh/lE+zeZMp5XQQmuv6qgUCatItoRQccm9zdGVkdEBn b29kbWlzLm9yZwAKCRAp5XQQmuv6qmfsAP9XviDyrl49/PBpuanB2qQzlMGqYQlb CwISmzgWoO1jKQEArNiSpKsIdPLfSJSLfIHAGxAZfmT42SugA4M5jqhW3Qc= =7KiP -----END PGP SIGNATURE----- Merge tag 'trace-v7.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace 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 |
||
|
Taeyang Lee
|
c47a7b11bc |
tracing: Check operator stack capacity before pushing
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: stable@vger.kernel.org
Fixes:
|
||
|
Taeyang Lee
|
803aa6fc2d |
tracing: Keep the predicate limit separate from program capacity
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: stable@vger.kernel.org
Fixes:
|
||
|
Hao Ge
|
3cc117514d |
tracing: Fix crash in print_graph_function_event()
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20261015130658.221198-1-hao.ge@linux.dev
Fixes:
|
||
|
Lee Jones
|
a244c24159 |
tracing/histograms: Reject non-stacktrace fields for synthetic stacktrace fields
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: stable@vger.kernel.org
Fixes:
|
||
|
Vincent Donnefort
|
13d530f2da |
ring-buffer: Use subbuf_order in available memory estimation
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/20261013102047.1488347-3-vdonnefort@google.com
Fixes:
|
||
|
Martin Kaiser
|
66c658033d |
tracing: probes: error if BTF is required but unavailable
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/20261014083838.31020-1-martin@kaiser.cx/
Fixes:
|
||
|
Vincent Donnefort
|
c9fc60c18b |
ring-buffer: Skip si_mem_available() for persistent and remote buffers
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/20261013102047.1488347-2-vdonnefort@google.com Fixes: |
||
|
Amit Machhiwal
|
b3b9740083 |
tracing: Do not initialize tracefs work if tracing is disabled
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: stable@vger.kernel.org
Fixes:
|
||
|
Tengda Wu
|
9414ca3b99 |
tracing: Fix hist trigger timestamps for buffered events
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
|
||
|
Dmitry Kandybka
|
3110d09bb9 |
tracing: Fix ignored return value of resize_buffer_duplicate_size()
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260928152634.4354-1-d.kandybka@gmail.com
Fixes:
|
||
|
Krystian Kaniewski
|
89bab1dba8 |
ring-buffer: Fix false warning in ring_buffer_map_get_reader()
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260924084946.20104-1-krystianmkaniewski@gmail.com
Fixes:
|
||
|
Vincent Donnefort
|
0fa993581a |
tracing: Fix simple_ring_buffer undef check on sparc64
simple_ring_buffer relies on headers that end up including
<asm/thread_info.h>. On sparc64, this emits the undefined symbol
current_thread_info_reg. Allow this symbol.
Link: https://patch.msgid.link/20260923074213.3077182-4-vdonnefort@google.com
Fixes:
|
||
|
Vincent Donnefort
|
ba875c4483 |
tracing: Fix simple_ring_buffer undef check on parisc
simple_ring_buffer uses local_set() and cmpxchg() on unsigned long. Add
them to undefsyms_base.c to extend the allowlist for the parisc
architecture.
Link: https://patch.msgid.link/20260923074213.3077182-3-vdonnefort@google.com
Fixes:
|
||
|
Vincent Donnefort
|
c902b6d709 |
tracing: Fix empty symbol names in simple_ring_buffer undef check
Sparc64 emits empty symmbol names. Skips them when establishing the
allowlist and when extracing undefined symbols.
Link: https://patch.msgid.link/20260923074213.3077182-2-vdonnefort@google.com
Fixes:
|
||
|
Donggeun Yoo
|
25132363c7 |
ring-buffer: Fix the sub-buffer array base in rb_meta_subbuf_idx()
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260918071507.1019251-1-donggeunyoo.kernel@gmail.com
Fixes:
|
||
|
Donggeun Yoo
|
647c2f97fc |
tracing: Fix the shift out of bounds in the log2 histogram modifier
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260916002735.808520-1-donggeunyoo.kernel@gmail.com
Fixes:
|
||
|
David Carlier
|
3f23cae09a |
ring-buffer: Do not show buffer_meta for CPUs without a buffer
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260914133506.4865-3-devnexen@gmail.com
Fixes:
|
||
|
David Carlier
|
2afe7575fb |
tracing: Take trace_types_lock when dropping the buffer_meta reference
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260914133506.4865-2-devnexen@gmail.com
Fixes:
|
||
|
Donggeun Yoo
|
a0777b9e53 |
tracing: Fix NULL dereference when copying keys for a field variable
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260914053443.981201-5-donggeunyoo.kernel@gmail.com
Fixes:
|
||
|
Masami Hiramatsu (Google)
|
e0a6249190 |
fprobe: Use guard(rcu_sched_notrace) and check rcu_is_watching()
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 <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3
Fixes:
|
||
|
Linus Torvalds
|
efb27d4767 |
Probes fixes for v7.3-rc4:
- 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. -----BEGIN PGP SIGNATURE----- iQFPBAABCgA5FiEEh7BulGwFlgAOi5DV2/sHvwUrPxsFAmq3llUbHG1hc2FtaS5o aXJhbWF0c3VAZ21haWwuY29tAAoJENv7B78FKz8bAhEH/0EAamjv7/EDUoUq+BOO a2gnlYqvr+zcrDVQLNgiYbvTRDfIFPOdB2LpY7Rguee3747qeL7kkNATD10WFr1F 5lXe5LaLncNIrvHDdtcT5eER5ePAuSDMSL5CwnJrRvXJw42iFsqegZ07nrvc9HFS 5zw7Ej9VnFJFxeXIY3J4U92wkntLJ3JhsNheomOtQmEZU1g5ZPAbdq0icNrC3CAb FTezYk60VG0CT/gNTSd8JFnI4P5vKlZpkFFCLLMmptW4yQU9+ZdT55JRgY7wJ8ye w5rLBTRJrFhDgNAjoehMkoAtxik3dKgede7qlwxUk0sv+gwXssw28b5zabRBJgee XQ8= =cHwO -----END PGP SIGNATURE----- Merge tag 'probes-fixes-v7.3-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace 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 |
||
|
David Carlier
|
1d653a1839 |
fprobe: Terminate the fgraph_data list when the reservation is not filled
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/20260917212407.384468-1-devnexen@gmail.com/
Fixes:
|
||
|
David Carlier
|
d860c67c05 |
ring-buffer: Check resize_disabled before publishing the new subbuf order
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: stable@vger.kernel.org
Fixes:
|
||
|
Vincent Donnefort
|
d059d8bf2c |
tracing/remotes: Catch nr_page_va overflow in ring_buffer_desc sizing
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/20260911193937.602202-3-vdonnefort@google.com
Fixes:
|
||
|
Vincent Donnefort
|
442ffa742d |
tracing/remotes: Account for ring buffer page header in size calculation
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/20260911193937.602202-2-vdonnefort@google.com
Fixes:
|
||
|
Ali Ahmet Memiş
|
bcfe2816e6 |
tracing: Don't dereference trace_event_file in deferred trigger free
The enable_event trigger defers trace_event_put_ref() to the
trigger free kthread, but the trace_event_file can already be freed
when the instance is removed.
Keep the trace_event_call directly in enable_trigger_data so the
deferred free does not access the freed trace_event_file.
Cc: stable@vger.kernel.org
Fixes:
|
||
|
Leon Hwang
|
b4dcc18b97 |
ftrace: Use rcu_assign_pointer() for tmp_ops filter hash
tmp_ops.func_hash->filter_hash is annotated __rcu, but
update_ftrace_direct_mod() assigns hash to it directly. Sparse reports an
address-space mismatch.
Use rcu_assign_pointer() for the assignment.
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260911142512.19344-1-leon.hwang@linux.dev
Fixes:
|
||
|
Sebastian Andrzej Siewior
|
815e07c8fe |
ring-buffer: Acquire the lock with irqsave in rb_wake_up_waiters()
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260911102152.YEtwkBj9@linutronix.de
Fixes:
|
||
|
Steven Rostedt
|
ed0aff60f8 |
tracing: Take trace_array reference when opening a tracer options file
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260910221209.62dad8d3@robin
Fixes:
|
||
|
Karl Mehltretter
|
7e645147df |
tracing: Fix ring_buffer_read_page_size() kernel-doc
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/20260909062917.89482-1-kmehltretter@gmail.com
Fixes:
|
||
|
Thomas Weißschuh
|
911002e99e |
tracing: Restore :mod: trailer after parsing in ftrace_set_clr_event()
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: stable@vger.kernel.org
Fixes:
|
||
|
Donggeun Yoo
|
7f711e6235 |
tracing: Fix memory corruption from a "STACKTRACE" histogram key
"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: stable@vger.kernel.org
Fixes:
|
||
|
Donggeun Yoo
|
a5e70ba87c |
tracing: Fix memory corruption from the histogram stacktrace modifier
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: stable@vger.kernel.org
Fixes:
|
||
|
Donggeun Yoo
|
92383cef66 |
tracing: Undo the registration when enabling the histogram trigger fails
Commit |
||
|
Donggeun Yoo
|
0fe23b8eab |
tracing: Take the reference before publishing the named histogram trigger
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: stable@vger.kernel.org
Fixes:
|
||
|
Donggeun Yoo
|
6ede78d056 |
tracing: Set the trace clock before registering the histogram trigger
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
|
||
|
Hemanth Selam
|
89b000ba07 |
tracing: Fix typo "availabe" in comment
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/20260907062608.13924-1-hemanth.selam@gmail.com Assisted-by: Cursor:claude-opus-5 Signed-off-by: Hemanth Selam <hemanth.selam@gmail.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> |
||
|
Donggeun Yoo
|
3d617bfd79 |
tracing: Let histogram values keep the percent and graph modifiers
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
|
||
|
Donggeun Yoo
|
06f5634ec5 |
tracing: Keep the entry count when the histogram stats allocation fails
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: stable@vger.kernel.org
Fixes:
|
||
|
Donggeun Yoo
|
230234d12c |
tracing: Free histogram the field rejected for a bad modifier
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
|
||
|
Donggeun Yoo
|
516001d53e |
tracing: Free histogram the var ref when its initialization fails
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 |
||
|
Donggeun Yoo
|
4bddcb346a |
tracing: Free histogram var refs regardless of how often they are referenced
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 |
||
|
Donggeun Yoo
|
0701995aaf |
function_graph: Use the saved entry's size when reprinting it
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: stable@vger.kernel.org
Fixes:
|
||
|
Donggeun Yoo
|
b228454870 |
fgraph: Remove unused FGRAPH_MAX_INDEX
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
|
||
|
Henry Martin
|
2deb753127 |
tracing/user_events: Don't destroy fields when event removal fails
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: stable@vger.kernel.org
Link: https://patch.msgid.link/20260904115223.2976446-1-bsdhenrymartin@gmail.com
Fixes:
|
||
|
Linus Torvalds
|
b1e00ffaf9 |
tracing fixes for v7.3:
- 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. A 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. -----BEGIN PGP SIGNATURE----- iIoEABYKADIWIQRRSw7ePDh/lE+zeZMp5XQQmuv6qgUCap3GJRQccm9zdGVkdEBn b29kbWlzLm9yZwAKCRAp5XQQmuv6qtAfAP9hDPtzctAoz+IHZq+GYejhKTu99ICG vAxTOZivuG6KYAD/UPA4P7z5ayxGM5UlT+2U81+mpCd55F2oFiIxRuFYcwk= =/d+T -----END PGP SIGNATURE----- Merge tag 'trace-v7.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace 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 |
||
|
Kees Cook
|
3a2c4d55e3 |
treewide: refresh kmalloc_obj() conversions
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 <kees+treewide@kernel.org> |
||
|
Steven Rostedt
|
d80e12156f |
ring-buffer: Use a macro for static buffer bits
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/20260904151641.17eae0aa@gandalf.local.home Signed-off-by: Steven Rostedt <rostedt@goodmis.org> |