linux-kernel - [PATCH] hrtimer: reset .hres_active and .online at appropriate points

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>] [day] [month] [year] [list]

Message-ID: <20241220134421.3809834-1-koichiro.den@canonical.com>
Date: Fri, 20 Dec 2024 22:44:21 +0900
From: Koichiro Den <koichiro.den@...onical.com>
To: linux-kernel@...r.kernel.org
Cc: anna-maria@...utronix.de,
	frederic@...nel.org,
	tglx@...utronix.de,
	peterz@...radead.org
Subject: [PATCH] hrtimer: reset .hres_active and .online at appropriate points

Consider a scenario where a CPU transitions from CPUHP_ONLINE to halfway
through a cpu hotunplug down to CPUHP_HRTIMERS_PREPARE, and then back to
CPUHP_ONLINE. Since hrtimers_prepare_cpu() does not run, .hres_active
remains set to 1 throughout. However, during the cpuhotplug down, the
tick and clockevents are shut down at CPUHP_AP_TICK_DYING. On the return
to the online state, for instance CFS incorrectly assumes that hrtick is
already active, and the chance of clockevent device to transition to
oneshot mode is also lost forever for the cpu, unless it goes back to a
lower state than CPUHP_HRTIMERS_PREPARE once.

This round-trip reveals another issue; .online is not reset to 1 after
the transition, which appears as a WARN_ON_ONCE in enqueue_hrtimer().

Reset these fields at the appropriate points. Ideally, .hres_active
would be reset in tick_cpu_dying() as underlying clockevent is shut down
there. However, since these operations occur in the atomic AP section
with interrupts disabled in any case, this patch resets .hres_active to
0 in hrtimers_cpu_dying() for simplicity.

Signed-off-by: Koichiro Den <koichiro.den@...onical.com>
---
 include/linux/hrtimer.h | 1 +
 kernel/cpu.c            | 2 +-
 kernel/time/hrtimer.c   | 9 +++++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 7ef5f7ef31a9..a5fdd305cabd 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -387,6 +387,7 @@ extern void sysrq_timer_list_show(void);

 int hrtimers_prepare_cpu(unsigned int cpu);
 #ifdef CONFIG_HOTPLUG_CPU
+int hrtimers_cpu_starting(unsigned int cpu);
 int hrtimers_cpu_dying(unsigned int cpu);
 #else
 #define hrtimers_cpu_dying	NULL
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 85fd7ac4561e..34f1a09349fc 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2180,7 +2180,7 @@ static struct cpuhp_step cpuhp_hp_states[] = {
 	},
 	[CPUHP_AP_HRTIMERS_DYING] = {
 		.name			= "hrtimers:dying",
-		.startup.single		= NULL,
+		.startup.single		= hrtimers_cpu_starting,
 		.teardown.single	= hrtimers_cpu_dying,
 	},
 	[CPUHP_AP_TICK_DYING] = {
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 80fe3749d2db..98f23c9341f5 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -2246,6 +2246,14 @@ static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
 	}
 }

+int hrtimers_cpu_starting(unsigned int cpu)
+{
+	struct hrtimer_cpu_base *cpu_base = &per_cpu(hrtimer_bases, cpu);
+
+	cpu_base->online = 1;
+	return 0;
+}
+
 int hrtimers_cpu_dying(unsigned int dying_cpu)
 {
 	int i, ncpu = cpumask_any_and(cpu_active_mask, housekeeping_cpumask(HK_TYPE_TIMER));
@@ -2275,6 +2283,7 @@ int hrtimers_cpu_dying(unsigned int dying_cpu)
 	smp_call_function_single(ncpu, retrigger_next_event, NULL, 0);

 	raw_spin_unlock(&new_base->lock);
+	old_base->hres_active = 0;
 	old_base->online = 0;
 	raw_spin_unlock(&old_base->lock);

-- 
2.43.0