[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250617200523.1261231-16-joelagnelf@nvidia.com>
Date: Tue, 17 Jun 2025 16:05:18 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: linux-kernel@...r.kernel.org,
Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>,
Ben Segall <bsegall@...gle.com>,
Mel Gorman <mgorman@...e.de>,
Valentin Schneider <vschneid@...hat.com>
Cc: Joel Fernandes <joelagnelf@...dia.com>,
Tejun Heo <tj@...nel.org>,
David Vernet <void@...ifault.com>,
Andrea Righi <arighi@...dia.com>,
Changwoo Min <changwoo@...lia.com>
Subject: [PATCH v4 15/15] sched/deadline: Fix DL server crash in inactive_timer callback
When sched_ext is rapidly disabled/enabled (the reload_loop selftest),
the following crash is observed. This happens because the timer handler
could not be cancelled and still fires even though the dl_server
bandwidth may have been removed via dl_server_remove_params().
hrtimer_try_to_cancel() does not guarantee timer cancellation. This
results in a NULL pointer dereference as 'p' is bogus for a dl_se.
I think this happens because the timer may be about to run, but its
softirq has not executed yet. Because of that hrtimer_try_to_cancel()
cannot prevent the timer from being canceled, however dl_server is still
set to 0 by dl_server_apply_params(). When the timer handler eventually
runs, it crashes.
[ 24.771835] BUG: kernel NULL pointer dereference, address: 000000000000006c
[ 24.772097] #PF: supervisor read access in kernel mode
[ 24.772248] #PF: error_code(0x0000) - not-present page
[ 24.772404] PGD 0 P4D 0
[ 24.772499] Oops: Oops: 0000 [#1] SMP PTI
[ 24.772614] CPU: 9 UID: 0 PID: 0 Comm: swapper/9 [..] #74 PREEMPT(voluntary)
[ 24.772932] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), [...]
[ 24.773149] Sched_ext: maximal (disabling)
[ 24.773944] RSP: 0018:ffffb162c0348ee0 EFLAGS: 00010046
[ 24.774100] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88d4412f1800
[ 24.774302] RDX: 0000000000000001 RSI: 0000000000000010 RDI: ffffffffac939240
[ 24.774498] RBP: ffff88d47e65b940 R08: 0000000000000010 R09: 00000008bad3370a
[ 24.774742] R10: 0000000000000000 R11: ffffffffa9f159d0 R12: ffff88d47e65b900
[ 24.774962] R13: ffff88d47e65b960 R14: ffff88d47e66a340 R15: ffff88d47e66aed0
[ 24.775182] FS: 0000000000000000(0000) GS:ffff88d4d1d56000(0000) knlGS:[...]
[ 24.775392] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 24.775579] CR2: 000000000000006c CR3: 0000000002bb0003 CR4: 0000000000770ef0
[ 24.775810] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 24.776023] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 24.776225] PKRU: 55555554
[ 24.776292] Call Trace:
[ 24.776373] <IRQ>
[ 24.776453] ? __pfx_inactive_task_timer+0x10/0x10
[ 24.776591] __hrtimer_run_queues+0xf1/0x270
[ 24.776744] hrtimer_interrupt+0xfa/0x220
[ 24.776847] __sysvec_apic_timer_interrupt+0x4d/0x190
[ 24.776988] sysvec_apic_timer_interrupt+0x69/0x80
[ 24.777132] </IRQ>
[ 24.777194] <TASK>
[ 24.777256] asm_sysvec_apic_timer_interrupt+0x1a/0x20
Fix, by also checking the DL server's has_task pointer which only exists
for server tasks. This fixes the crash.
Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>
---
kernel/sched/deadline.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 50d938856b3d..f7ca022463dc 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1820,7 +1820,13 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)
struct rq_flags rf;
struct rq *rq;
- if (!dl_server(dl_se)) {
+ /*
+ * It is possible that after dl_server_apply_params(), the dl_se->dl_server == 0,
+ * but the inactive timer is still queued and could not get canceled. Double check
+ * by looking at ->server_has_tasks to make sure we're dealing with a non-server
+ * here. Otherwise p may be bogus and we'll crash.
+ */
+ if (!dl_server(dl_se) && !dl_se->server_has_tasks) {
p = dl_task_of(dl_se);
rq = task_rq_lock(p, &rf);
} else {
@@ -1831,7 +1837,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)
sched_clock_tick();
update_rq_clock(rq);
- if (dl_server(dl_se))
+ if (dl_server(dl_se) || dl_se->server_has_tasks)
goto no_task;
if (!dl_task(p) || READ_ONCE(p->__state) == TASK_DEAD) {
@@ -1859,7 +1865,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)
dl_se->dl_non_contending = 0;
unlock:
- if (!dl_server(dl_se)) {
+ if (!dl_server(dl_se) && !dl_se->server_has_tasks) {
task_rq_unlock(rq, p, &rf);
put_task_struct(p);
} else {
--
2.43.0
Powered by blists - more mailing lists