2020-01-03 03:04:09

by Wen Yang

Subject: [PATCH v2] ftrace: avoid potential division by zero

The ftrace_profile->counter is unsigned long and
do_div truncates it to 32 bits, which means it can test
non-zero and be truncated to zero for division.
Fix this issue by using div64_ul() instead.

Signed-off-by: Wen Yang <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: [email protected]
---
v2: get rid of the pre-assigning of avg.

kernel/trace/ftrace.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ac99a35..9bf1f2c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -526,8 +526,7 @@ static int function_stat_show(struct seq_file *m, void *v)
}

#ifdef CONFIG_FUNCTION_GRAPH_TRACER
- avg = rec->time;
- do_div(avg, rec->counter);
+ avg = div64_ul(rec->time, rec->counter);
if (tracing_thresh && (avg < tracing_thresh))
goto out;
#endif
@@ -553,7 +552,8 @@ static int function_stat_show(struct seq_file *m, void *v)
* Divide only 1000 for ns^2 -> us^2 conversion.
* trace_print_graph_duration will divide 1000 again.
*/
- do_div(stddev, rec->counter * (rec->counter - 1) * 1000);
+ stddev = div64_ul(stddev,
+ rec->counter * (rec->counter - 1) * 1000);
}

trace_seq_init(&s);
--
1.8.3.1

2020-01-03 08:01:41

by Justin Capella

Subject: Re: [PATCH v2] ftrace: avoid potential division by zero

> - do_div(stddev, rec->counter * (rec->counter - 1) * 1000);
>
> + stddev = div64_ul(stddev,
> + rec->counter * (rec->counter - 1) * 1000);


Is a rec->counter > 1 assertion needed here?

2020-01-03 11:32:47

by Wen Yang

Subject: Re: [PATCH v2] ftrace: avoid potential division by zero

On 2020/1/3 4:00 下午, Justin Capella wrote:
>> - do_div(stddev, rec->counter * (rec->counter - 1) * 1000);
>>
>> + stddev = div64_ul(stddev,
>> + rec->counter * (rec->counter - 1) * 1000);
>
>
> Is a rec->counter > 1 assertion needed here?
>

Hello, the above lines deal with this situation:

if (rec->counter <= 1)
stddev = 0;
else {


--
Regards,
Wen