Skip to content

Commit 5c44dda

Browse files
committed
Merge tag 'trace-v6.14-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
Pull tracing fixes from Steven Rostedt: - Fix crash from bad histogram entry An error path in the histogram creation could leave an entry in a link list that gets freed. Then when a new entry is added it can cause a u-a-f bug. This is fixed by restructuring the code so that the histogram is consistent on failure and everything is cleaned up appropriately. - Fix fprobe self test The fprobe self test relies on no function being attached by ftrace. BPF programs can attach to functions via ftrace and systemd now does so. This causes those functions to appear in the enabled_functions list which holds all functions attached by ftrace. The selftest also uses that file to see if functions are being connected correctly. It counts the functions in the file, but if there's already functions in the file, it fails. Instead, add the number of functions in the file at the start of the test to all the calculations during the test. - Fix potential division by zero of the function profiler stddev The calculated divisor that calculates the standard deviation of the function times can overflow. If the overflow happens to land on zero, that can cause a division by zero. Check for zero from the calculation before doing the division. TODO: Catch when it ever overflows and report it accordingly. For now, just prevent the system from crashing. * tag 'trace-v6.14-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace: ftrace: Avoid potential division by zero in function_stat_show() selftests/ftrace: Let fprobe test consider already enabled functions tracing: Fix bad hist from corrupting named_triggers list
2 parents 3d7dc86 + a1a7eb8 commit 5c44dda

File tree

3 files changed

+38
-37
lines changed

3 files changed

+38
-37
lines changed

kernel/trace/ftrace.c

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,7 @@ static int function_stat_show(struct seq_file *m, void *v)
540540
static struct trace_seq s;
541541
unsigned long long avg;
542542
unsigned long long stddev;
543+
unsigned long long stddev_denom;
543544
#endif
544545
guard(mutex)(&ftrace_profile_lock);
545546

@@ -559,23 +560,19 @@ static int function_stat_show(struct seq_file *m, void *v)
559560
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
560561
seq_puts(m, " ");
561562

562-
/* Sample standard deviation (s^2) */
563-
if (rec->counter <= 1)
564-
stddev = 0;
565-
else {
566-
/*
567-
* Apply Welford's method:
568-
* s^2 = 1 / (n * (n-1)) * (n * \Sum (x_i)^2 - (\Sum x_i)^2)
569-
*/
563+
/*
564+
* Variance formula:
565+
* s^2 = 1 / (n * (n-1)) * (n * \Sum (x_i)^2 - (\Sum x_i)^2)
566+
* Maybe Welford's method is better here?
567+
* Divide only by 1000 for ns^2 -> us^2 conversion.
568+
* trace_print_graph_duration will divide by 1000 again.
569+
*/
570+
stddev = 0;
571+
stddev_denom = rec->counter * (rec->counter - 1) * 1000;
572+
if (stddev_denom) {
570573
stddev = rec->counter * rec->time_squared -
571574
rec->time * rec->time;
572-
573-
/*
574-
* Divide only 1000 for ns^2 -> us^2 conversion.
575-
* trace_print_graph_duration will divide 1000 again.
576-
*/
577-
stddev = div64_ul(stddev,
578-
rec->counter * (rec->counter - 1) * 1000);
575+
stddev = div64_ul(stddev, stddev_denom);
579576
}
580577

581578
trace_seq_init(&s);

kernel/trace/trace_events_hist.c

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6724,27 +6724,27 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
67246724
if (existing_hist_update_only(glob, trigger_data, file))
67256725
goto out_free;
67266726

6727-
ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
6728-
if (ret < 0)
6729-
goto out_free;
6727+
if (!get_named_trigger_data(trigger_data)) {
67306728

6731-
if (get_named_trigger_data(trigger_data))
6732-
goto enable;
6729+
ret = create_actions(hist_data);
6730+
if (ret)
6731+
goto out_free;
67336732

6734-
ret = create_actions(hist_data);
6735-
if (ret)
6736-
goto out_unreg;
6733+
if (has_hist_vars(hist_data) || hist_data->n_var_refs) {
6734+
ret = save_hist_vars(hist_data);
6735+
if (ret)
6736+
goto out_free;
6737+
}
67376738

6738-
if (has_hist_vars(hist_data) || hist_data->n_var_refs) {
6739-
ret = save_hist_vars(hist_data);
6739+
ret = tracing_map_init(hist_data->map);
67406740
if (ret)
6741-
goto out_unreg;
6741+
goto out_free;
67426742
}
67436743

6744-
ret = tracing_map_init(hist_data->map);
6745-
if (ret)
6746-
goto out_unreg;
6747-
enable:
6744+
ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
6745+
if (ret < 0)
6746+
goto out_free;
6747+
67486748
ret = hist_trigger_enable(trigger_data, file);
67496749
if (ret)
67506750
goto out_unreg;

tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,24 @@ PLACE=$FUNCTION_FORK
1010
PLACE2="kmem_cache_free"
1111
PLACE3="schedule_timeout"
1212

13+
# Some functions may have BPF programs attached, therefore
14+
# count already enabled_functions before tests start
15+
ocnt=`cat enabled_functions | wc -l`
16+
1317
echo "f:myevent1 $PLACE" >> dynamic_events
1418

1519
# Make sure the event is attached and is the only one
1620
grep -q $PLACE enabled_functions
1721
cnt=`cat enabled_functions | wc -l`
18-
if [ $cnt -ne 1 ]; then
22+
if [ $cnt -ne $((ocnt + 1)) ]; then
1923
exit_fail
2024
fi
2125

2226
echo "f:myevent2 $PLACE%return" >> dynamic_events
2327

2428
# It should till be the only attached function
2529
cnt=`cat enabled_functions | wc -l`
26-
if [ $cnt -ne 1 ]; then
30+
if [ $cnt -ne $((ocnt + 1)) ]; then
2731
exit_fail
2832
fi
2933

@@ -32,7 +36,7 @@ echo "f:myevent3 $PLACE2" >> dynamic_events
3236

3337
grep -q $PLACE2 enabled_functions
3438
cnt=`cat enabled_functions | wc -l`
35-
if [ $cnt -ne 2 ]; then
39+
if [ $cnt -ne $((ocnt + 2)) ]; then
3640
exit_fail
3741
fi
3842

@@ -49,31 +53,31 @@ grep -q myevent1 dynamic_events
4953

5054
# should still have 2 left
5155
cnt=`cat enabled_functions | wc -l`
52-
if [ $cnt -ne 2 ]; then
56+
if [ $cnt -ne $((ocnt + 2)) ]; then
5357
exit_fail
5458
fi
5559

5660
echo > dynamic_events
5761

5862
# Should have none left
5963
cnt=`cat enabled_functions | wc -l`
60-
if [ $cnt -ne 0 ]; then
64+
if [ $cnt -ne $ocnt ]; then
6165
exit_fail
6266
fi
6367

6468
echo "f:myevent4 $PLACE" >> dynamic_events
6569

6670
# Should only have one enabled
6771
cnt=`cat enabled_functions | wc -l`
68-
if [ $cnt -ne 1 ]; then
72+
if [ $cnt -ne $((ocnt + 1)) ]; then
6973
exit_fail
7074
fi
7175

7276
echo > dynamic_events
7377

7478
# Should have none left
7579
cnt=`cat enabled_functions | wc -l`
76-
if [ $cnt -ne 0 ]; then
80+
if [ $cnt -ne $ocnt ]; then
7781
exit_fail
7882
fi
7983

0 commit comments

Comments
 (0)