linux-kernel - [PATCH stable 1/1] x86: aperfmperf: fix overflow problem in the concurrency scenario

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: <20221021021740.137196-2-zouyipeng@huawei.com>
Date:   Fri, 21 Oct 2022 10:17:40 +0800
From:   Yipeng Zou <zouyipeng@...wei.com>
To:     <tglx@...utronix.de>, <bp@...en8.de>, <mingo@...hat.com>,
        <x86@...nel.org>, <hpa@...or.com>, <rafael.j.wysocki@...el.com>,
        <linux-kernel@...r.kernel.org>
CC:     Yipeng Zou <zouyipeng@...wei.com>
Subject: [PATCH stable 1/1] x86: aperfmperf: fix overflow problem in the concurrency scenario

Recently i was doing some work about calculating CPU frequency on x86 on
stable branch linux-5.10.y, and there are the details of the problem.
I was test on platform Intel Xeon CPU @ 2.10Ghz. In most cases the cpu
freq field cat from /proc/cpuinfo was 2099.800 ~ 2100.300 MHz. At a very
low probability, the values of cpu freq was very small, such as 105.000
MHz.

After debug I found that there is an integer overflow problem in the
concurrency scenario during the calculation of cpu freq. The key code
was the function aperfmperf_snapshot_khz(),

In function aperfmperf_snapshot_khz(), the aperf_delta may be large(a
long time since the last execution). This has the potential to cause
integer overflow when multiplying with it.

And Then to avoid this the stable branch will calculate the cpu freq
twice and sleep 10ms when it is found that this update is a long time
since the last time to ensure calculated the cpu freq correctly.

Consider the following:

Task 0                   Task 1

arch_freq_perpare_all    ....

sleep 10ms               ....

....                     arch_freq_perpare_all    // Within 10ms

....                     aperfmperf_snapshot_cpu  // Within 10ms

aperfmperf_snapshot_cpu  ....

step 0: task0 : arch_freq_perpare_all go through all cpus and update
their time and freq. If this time is a long time since the last update,
then the saved CPU freq is abnormal(integer overflow).

step 1: Then task1 has also come to get the same CPU freq. But since
within 10ms of last task 0 calculation, it cannot update the current
CPU freq, which exception value it will gets.

I've noticed that it has been abandoned on the mainline. On the mainline
it becomes to update [a,m]cnt in timer code with HZ frequency, and it is
actually calculated at the time of reading the cpu freqency. This solves
the problem above.

Fixes: 7d5905dc14a8 ("x86 / CPU: Always show current CPU frequency in /proc/cpuinfo")
Signed-off-by: Yipeng Zou <zouyipeng@...wei.com>
---
 arch/x86/kernel/cpu/aperfmperf.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/aperfmperf.c b/arch/x86/kernel/cpu/aperfmperf.c
index e2f319dc992d..d3f417c06d5f 100644
--- a/arch/x86/kernel/cpu/aperfmperf.c
+++ b/arch/x86/kernel/cpu/aperfmperf.c
@@ -68,10 +68,6 @@ static bool aperfmperf_snapshot_cpu(int cpu, ktime_t now, bool wait)
 {
 	s64 time_delta = ktime_ms_delta(now, per_cpu(samples.time, cpu));

-	/* Don't bother re-computing within the cache threshold time. */
-	if (time_delta < APERFMPERF_CACHE_THRESHOLD_MS)
-		return true;
-
 	smp_call_function_single(cpu, aperfmperf_snapshot_khz, NULL, wait);

 	/* Return false if the previous iteration was too long ago. */
-- 
2.17.1