Skip to content

Commit fa6ad96

Browse files
committed
Merge tag 'trace-v6.15-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
Pull tracing fixes from Steven Rostedt: - Initialize hash variables in ftrace subops logic The fix that simplified the ftrace subops logic opened a path where some variables could be used without being initialized, and done subtly where the compiler did not catch it. Initialize those variables to the EMPTY_HASH, which is the default hash. - Reinitialize the hash pointers after they are freed Some of the hash pointers in the subop logic were freed but may still be referenced later. To prevent use-after-free bugs, initialize them back to the EMPTY_HASH. - Free the ftrace hashes when they are replaced The fix that simplified the subops logic updated some hash pointers, but left the original hash that they were pointing to where they are no longer used. This caused a memory leak. Free the hashes that are pointed to by the pointers when they are replaced. - Fix size initialization of ftrace direct function hash The ftrace direct function hash used by BPF initialized the hash size incorrectly. It checked the size of items to a hard coded 32, which made the hash bit size of 5. The hash size is supposed to be limited by the bit size of the hash, as the bitmask is allowed to be greater than 5. Rework the size check to first pass the number of elements to fls() and then compare that to FTRACE_HASH_MAX_BITS before allocating the hash. - Fix format output of ftrace_graph_ent_entry event The field depth of the ftrace_graph_ent_entry event is of size 4 but the output showed it as unsigned long and use "%lu". Change it to unsigned int and use "%u" in the print format that is displayed to user space. - Fix the trace event filter on strings Events can be filtered on numbers or string values. The return value checked from strncpy_from_kernel_nofault() and strncpy_from_user_nofault() was used to determine if reading the strings would fault or not. It would return fault if the value was non zero, which is basically meant that it was always considering the read as a fault. - Add selftest to test trace event string filtering In order to catch the breakage of the string filtering, add a self test to make sure that it continues to work. * tag 'trace-v6.15-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace: tracing: selftests: Add testing a user string to filters tracing: Fix filter string testing ftrace: Fix type of ftrace_graph_ent_entry.depth ftrace: fix incorrect hash size in register_ftrace_direct() ftrace: Free ftrace hashes after they are replaced in the subops code ftrace: Reinitialize hash to EMPTY_HASH after freeing ftrace: Initialize variables for ftrace_startup/shutdown_subops()
2 parents 1ca0f93 + d481ee3 commit fa6ad96

File tree

4 files changed

+43
-12
lines changed

4 files changed

+43
-12
lines changed

kernel/trace/ftrace.c

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,6 +1297,8 @@ void ftrace_free_filter(struct ftrace_ops *ops)
12971297
return;
12981298
free_ftrace_hash(ops->func_hash->filter_hash);
12991299
free_ftrace_hash(ops->func_hash->notrace_hash);
1300+
ops->func_hash->filter_hash = EMPTY_HASH;
1301+
ops->func_hash->notrace_hash = EMPTY_HASH;
13001302
}
13011303
EXPORT_SYMBOL_GPL(ftrace_free_filter);
13021304

@@ -3443,6 +3445,7 @@ static int add_next_hash(struct ftrace_hash **filter_hash, struct ftrace_hash **
34433445
size_bits);
34443446
if (ret < 0) {
34453447
free_ftrace_hash(*filter_hash);
3448+
*filter_hash = EMPTY_HASH;
34463449
return ret;
34473450
}
34483451
}
@@ -3472,6 +3475,7 @@ static int add_next_hash(struct ftrace_hash **filter_hash, struct ftrace_hash **
34723475
subops_hash->notrace_hash);
34733476
if (ret < 0) {
34743477
free_ftrace_hash(*notrace_hash);
3478+
*notrace_hash = EMPTY_HASH;
34753479
return ret;
34763480
}
34773481
}
@@ -3490,8 +3494,8 @@ static int add_next_hash(struct ftrace_hash **filter_hash, struct ftrace_hash **
34903494
*/
34913495
int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
34923496
{
3493-
struct ftrace_hash *filter_hash;
3494-
struct ftrace_hash *notrace_hash;
3497+
struct ftrace_hash *filter_hash = EMPTY_HASH;
3498+
struct ftrace_hash *notrace_hash = EMPTY_HASH;
34953499
struct ftrace_hash *save_filter_hash;
34963500
struct ftrace_hash *save_notrace_hash;
34973501
int ret;
@@ -3605,6 +3609,9 @@ static int rebuild_hashes(struct ftrace_hash **filter_hash, struct ftrace_hash *
36053609
}
36063610
}
36073611

3612+
free_ftrace_hash(temp_hash.filter_hash);
3613+
free_ftrace_hash(temp_hash.notrace_hash);
3614+
36083615
temp_hash.filter_hash = *filter_hash;
36093616
temp_hash.notrace_hash = *notrace_hash;
36103617
}
@@ -3625,8 +3632,8 @@ static int rebuild_hashes(struct ftrace_hash **filter_hash, struct ftrace_hash *
36253632
*/
36263633
int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
36273634
{
3628-
struct ftrace_hash *filter_hash;
3629-
struct ftrace_hash *notrace_hash;
3635+
struct ftrace_hash *filter_hash = EMPTY_HASH;
3636+
struct ftrace_hash *notrace_hash = EMPTY_HASH;
36303637
int ret;
36313638

36323639
if (unlikely(ftrace_disabled))
@@ -3699,8 +3706,11 @@ static int ftrace_hash_move_and_update_subops(struct ftrace_ops *subops,
36993706
}
37003707

37013708
ret = rebuild_hashes(&filter_hash, &notrace_hash, ops);
3702-
if (!ret)
3709+
if (!ret) {
37033710
ret = ftrace_update_ops(ops, filter_hash, notrace_hash);
3711+
free_ftrace_hash(filter_hash);
3712+
free_ftrace_hash(notrace_hash);
3713+
}
37043714

37053715
if (ret) {
37063716
/* Put back the original hash */
@@ -5954,9 +5964,10 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
59545964

59555965
/* Make a copy hash to place the new and the old entries in */
59565966
size = hash->count + direct_functions->count;
5957-
if (size > 32)
5958-
size = 32;
5959-
new_hash = alloc_ftrace_hash(fls(size));
5967+
size = fls(size);
5968+
if (size > FTRACE_HASH_MAX_BITS)
5969+
size = FTRACE_HASH_MAX_BITS;
5970+
new_hash = alloc_ftrace_hash(size);
59605971
if (!new_hash)
59615972
goto out_unlock;
59625973

kernel/trace/trace_entries.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ FTRACE_ENTRY(funcgraph_entry, ftrace_graph_ent_entry,
8080
F_STRUCT(
8181
__field_struct( struct ftrace_graph_ent, graph_ent )
8282
__field_packed( unsigned long, graph_ent, func )
83-
__field_packed( unsigned long, graph_ent, depth )
83+
__field_packed( unsigned int, graph_ent, depth )
8484
__dynamic_array(unsigned long, args )
8585
),
8686

87-
F_printk("--> %ps (%lu)", (void *)__entry->func, __entry->depth)
87+
F_printk("--> %ps (%u)", (void *)__entry->func, __entry->depth)
8888
);
8989

9090
#ifdef CONFIG_FUNCTION_GRAPH_RETADDR

kernel/trace/trace_events_filter.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ static __always_inline char *test_string(char *str)
808808
kstr = ubuf->buffer;
809809

810810
/* For safety, do not trust the string pointer */
811-
if (!strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE))
811+
if (strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE) < 0)
812812
return NULL;
813813
return kstr;
814814
}
@@ -827,7 +827,7 @@ static __always_inline char *test_ustring(char *str)
827827

828828
/* user space address? */
829829
ustr = (char __user *)str;
830-
if (!strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE))
830+
if (strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE) < 0)
831831
return NULL;
832832

833833
return kstr;

tools/testing/selftests/ftrace/test.d/filter/event-filter-function.tc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,26 @@ if [ $misscnt -gt 0 ]; then
8080
exit_fail
8181
fi
8282

83+
# Check strings too
84+
if [ -f events/syscalls/sys_enter_openat/filter ]; then
85+
DIRNAME=`basename $TMPDIR`
86+
echo "filename.ustring ~ \"*$DIRNAME*\"" > events/syscalls/sys_enter_openat/filter
87+
echo 1 > events/syscalls/sys_enter_openat/enable
88+
echo 1 > tracing_on
89+
ls /bin/sh
90+
nocnt=`grep openat trace | wc -l`
91+
ls $TMPDIR
92+
echo 0 > tracing_on
93+
hitcnt=`grep openat trace | wc -l`;
94+
echo 0 > events/syscalls/sys_enter_openat/enable
95+
if [ $nocnt -gt 0 ]; then
96+
exit_fail
97+
fi
98+
if [ $hitcnt -eq 0 ]; then
99+
exit_fail
100+
fi
101+
fi
102+
83103
reset_events_filter
84104

85105
exit 0

0 commit comments

Comments
 (0)