linux-kernel - Re: [PATCH] ftrace: Avoid potential division by zero in function_stat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250204074301.1427497-1-kniv@yandex-team.ru>
Date: Tue,  4 Feb 2025 10:43:02 +0300
From: Nikolay Kuratov <kniv@...dex-team.ru>
To: linux-kernel@...r.kernel.org
Cc: linux-trace-kernel@...r.kernel.org,
	Wen Yang <wenyang@...ux.alibaba.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Mark Rutland <mark.rutland@....com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	kniv@...dex-team.ru
Subject: Re: [PATCH] ftrace: Avoid potential division by zero in function_stat_show()

Thank you for the review!

`counter > rec->counter` check does not protect us from overflows,
so this could mislead especially with the comment included.

I think we should either eliminate overflows or only protect from zero
division. To prevent overflows it is wise to split the division into
three and forget about it. But in 32-bit case overflows will still occur.

As for zero division protection, maybe this variant even more concise?

#ifdef CONFIG_FUNCTION_GRAPH_TRACER
	seq_puts(m, "    ");

	stddev = 0;
	unsigned long denom = rec->counter * (rec->counter - 1) * 1000;
	if (denom) {
		stddev = rec->counter * rec->time_squared -
			 rec->time * rec->time;
		stddev = div64_ul(stddev, denom);
	}

	trace_seq_init(&s);
	trace_print_graph_duration(rec->time, &s);

--
My last attempt to get this email into lore with git-send-email, since
no luck with mutt. Sorry if you see this message twice or thrice in your
inbox. It won't happen again.