Skip to content

Commit 6856b5c

Browse files
rostedtgregkh
authored andcommitted
ftrace: Fix accounting of adding subops to a manager ops
commit 38b1406 upstream. Function graph uses a subops and manager ops mechanism to attach to ftrace. The manager ops connects to ftrace and the functions it connects to is defined by a list of subops that it manages. The function hash that defines what the above ops attaches to limits the functions to attach if the hash has any content. If the hash is empty, it means to trace all functions. The creation of the manager ops hash is done by iterating over all the subops hashes. If any of the subops hashes is empty, it means that the manager ops hash must trace all functions as well. The issue is in the creation of the manager ops. When a second subops is attached, a new hash is created by starting it as NULL and adding the subops one at a time. But the NULL ops is mistaken as an empty hash, and once an empty hash is found, it stops the loop of subops and just enables all functions. # echo "f:myevent1 kernel_clone" >> /sys/kernel/tracing/dynamic_events # cat /sys/kernel/tracing/enabled_functions kernel_clone (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 # echo "f:myevent2 schedule_timeout" >> /sys/kernel/tracing/dynamic_events # cat /sys/kernel/tracing/enabled_functions trace_initcall_start_cb (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 run_init_process (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 try_to_run_init_process (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 x86_pmu_show_pmu_cap (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 cleanup_rapl_pmus (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 uncore_free_pcibus_map (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 uncore_types_exit (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 uncore_pci_exit.part.0 (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 kvm_shutdown (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 vmx_dump_msrs (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 vmx_cleanup_l1d_flush (1) tramp: 0xffffffffc0309000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 [..] Fix this by initializing the new hash to NULL and if the hash is NULL do not treat it as an empty hash but instead allocate by copying the content of the first sub ops. Then on subsequent iterations, the new hash will not be NULL, but the content of the previous subops. If that first subops attached to all functions, then new hash may assume that the manager ops also needs to attach to all functions. Cc: stable@vger.kernel.org Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Heiko Carstens <hca@linux.ibm.com> Cc: Sven Schnelle <svens@linux.ibm.com> Cc: Vasily Gorbik <gor@linux.ibm.com> Cc: Alexander Gordeev <agordeev@linux.ibm.com> Link: https://lore.kernel.org/20250220202055.060300046@goodmis.org Fixes: 5fccc75 ("ftrace: Add subops logic to allow one ops to manage many") Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 1bfc1f1 commit 6856b5c

File tree

1 file changed

+22
-11
lines changed

1 file changed

+22
-11
lines changed

kernel/trace/ftrace.c

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3219,15 +3219,22 @@ static struct ftrace_hash *copy_hash(struct ftrace_hash *src)
32193219
* The filter_hash updates uses just the append_hash() function
32203220
* and the notrace_hash does not.
32213221
*/
3222-
static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash)
3222+
static int append_hash(struct ftrace_hash **hash, struct ftrace_hash *new_hash,
3223+
int size_bits)
32233224
{
32243225
struct ftrace_func_entry *entry;
32253226
int size;
32263227
int i;
32273228

3228-
/* An empty hash does everything */
3229-
if (ftrace_hash_empty(*hash))
3230-
return 0;
3229+
if (*hash) {
3230+
/* An empty hash does everything */
3231+
if (ftrace_hash_empty(*hash))
3232+
return 0;
3233+
} else {
3234+
*hash = alloc_ftrace_hash(size_bits);
3235+
if (!*hash)
3236+
return -ENOMEM;
3237+
}
32313238

32323239
/* If new_hash has everything make hash have everything */
32333240
if (ftrace_hash_empty(new_hash)) {
@@ -3291,16 +3298,18 @@ static int intersect_hash(struct ftrace_hash **hash, struct ftrace_hash *new_has
32913298
/* Return a new hash that has a union of all @ops->filter_hash entries */
32923299
static struct ftrace_hash *append_hashes(struct ftrace_ops *ops)
32933300
{
3294-
struct ftrace_hash *new_hash;
3301+
struct ftrace_hash *new_hash = NULL;
32953302
struct ftrace_ops *subops;
3303+
int size_bits;
32963304
int ret;
32973305

3298-
new_hash = alloc_ftrace_hash(ops->func_hash->filter_hash->size_bits);
3299-
if (!new_hash)
3300-
return NULL;
3306+
if (ops->func_hash->filter_hash)
3307+
size_bits = ops->func_hash->filter_hash->size_bits;
3308+
else
3309+
size_bits = FTRACE_HASH_DEFAULT_BITS;
33013310

33023311
list_for_each_entry(subops, &ops->subop_list, list) {
3303-
ret = append_hash(&new_hash, subops->func_hash->filter_hash);
3312+
ret = append_hash(&new_hash, subops->func_hash->filter_hash, size_bits);
33043313
if (ret < 0) {
33053314
free_ftrace_hash(new_hash);
33063315
return NULL;
@@ -3309,7 +3318,8 @@ static struct ftrace_hash *append_hashes(struct ftrace_ops *ops)
33093318
if (ftrace_hash_empty(new_hash))
33103319
break;
33113320
}
3312-
return new_hash;
3321+
/* Can't return NULL as that means this failed */
3322+
return new_hash ? : EMPTY_HASH;
33133323
}
33143324

33153325
/* Make @ops trace evenything except what all its subops do not trace */
@@ -3504,7 +3514,8 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int
35043514
filter_hash = alloc_and_copy_ftrace_hash(size_bits, ops->func_hash->filter_hash);
35053515
if (!filter_hash)
35063516
return -ENOMEM;
3507-
ret = append_hash(&filter_hash, subops->func_hash->filter_hash);
3517+
ret = append_hash(&filter_hash, subops->func_hash->filter_hash,
3518+
size_bits);
35083519
if (ret < 0) {
35093520
free_ftrace_hash(filter_hash);
35103521
return ret;

0 commit comments

Comments
 (0)