ftrace: Avoid potential division by zero in function_stat_show()
authorNikolay Kuratov <kniv@yandex-team.ru>
Thu, 6 Feb 2025 09:01:56 +0000 (12:01 +0300)
committerSteven Rostedt (Google) <rostedt@goodmis.org>
Fri, 28 Feb 2025 02:02:10 +0000 (21:02 -0500)
Check whether denominator expression x * (x - 1) * 1000 mod {2^32, 2^64}
produce zero and skip stddev computation in that case.

For now don't care about rec->counter * rec->counter overflow because
rec->time * rec->time overflow will likely happen earlier.

Cc: stable@vger.kernel.org
Cc: Wen Yang <wenyang@linux.alibaba.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/20250206090156.1561783-1-kniv@yandex-team.ru
Fixes: e31f7939c1c27 ("ftrace: Avoid potential division by zero in function profiler")
Signed-off-by: Nikolay Kuratov <kniv@yandex-team.ru>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
kernel/trace/ftrace.c

index 6b0c25761ccb1e5ef002f0f388019310d7de2655..fc88e0688daf097c3369a56a6be53d89e51d024f 100644 (file)
@@ -540,6 +540,7 @@ static int function_stat_show(struct seq_file *m, void *v)
        static struct trace_seq s;
        unsigned long long avg;
        unsigned long long stddev;
+       unsigned long long stddev_denom;
 #endif
        guard(mutex)(&ftrace_profile_lock);
 
@@ -559,23 +560,19 @@ static int function_stat_show(struct seq_file *m, void *v)
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
        seq_puts(m, "    ");
 
-       /* Sample standard deviation (s^2) */
-       if (rec->counter <= 1)
-               stddev = 0;
-       else {
-               /*
-                * Apply Welford's method:
-                * s^2 = 1 / (n * (n-1)) * (n * \Sum (x_i)^2 - (\Sum x_i)^2)
-                */
+       /*
+        * Variance formula:
+        * s^2 = 1 / (n * (n-1)) * (n * \Sum (x_i)^2 - (\Sum x_i)^2)
+        * Maybe Welford's method is better here?
+        * Divide only by 1000 for ns^2 -> us^2 conversion.
+        * trace_print_graph_duration will divide by 1000 again.
+        */
+       stddev = 0;
+       stddev_denom = rec->counter * (rec->counter - 1) * 1000;
+       if (stddev_denom) {
                stddev = rec->counter * rec->time_squared -
                         rec->time * rec->time;
-
-               /*
-                * Divide only 1000 for ns^2 -> us^2 conversion.
-                * trace_print_graph_duration will divide 1000 again.
-                */
-               stddev = div64_ul(stddev,
-                                 rec->counter * (rec->counter - 1) * 1000);
+               stddev = div64_ul(stddev, stddev_denom);
        }
 
        trace_seq_init(&s);