[PATCH] xen/sched: rt: fix NULL cpupool dereference in move_repl_timer()

Oleksii Moisieiev posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/38cc09ca49bb020d4b13ead36e4c9f04da2d00e5.1784211528.git.oleksii._5Fmoisieiev@epam.com
There is a newer version of this series
xen/common/sched/rt.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] xen/sched: rt: fix NULL cpupool dereference in move_repl_timer()
Posted by Oleksii Moisieiev 1 week ago
schedule_cpu_rm() clears the cpupool pointer of the scheduling
resource before calling sched_deinit_pdata():

    sr->cpupool = NULL;
    ...
    sched_deinit_pdata(data->old_ops, data->ppriv_old, cpu);

For RTDS, rt_deinit_pdata() calls move_repl_timer() when the
replenishment timer lives on the cpu being removed, and
move_repl_timer() dereferences get_sched_res(old_cpu)->cpupool
without checking it for NULL. Removing the last pCPU from an RTDS
cpupool therefore dereferences NULL + 0x10 (the res_valid member)
and panics:

    (XEN) Data Abort Trap. Syndrome=0x1c28005
    (XEN) Walking Hypervisor VA 0x10 on CPU0 via TTBR ...
    (XEN) Xen call trace:
    (XEN)    [<...>] find_next_bit+0x74/0xa8 (PC)
    (XEN)    [<...>] rt.c#move_repl_timer+0xb8/0xec (LR)
    (XEN)
    (XEN) Panic on CPU 0:
    (XEN) CPU0: Unexpected Trap: Data Abort

Reproducer, on any host with at least 2 pCPUs and RTDS compiled in
(observed on arm64, but the path is common code):

    xl cpupool-create name="test" sched="rtds"
    xl cpupool-cpu-remove Pool-0 1
    xl cpupool-cpu-add test 1
    xl cpupool-cpu-remove test 1

The last command moves the RTDS replenishment timer to cpu1 (first
and only cpu of the pool) and then removes cpu1, hitting the NULL
dereference in the sched_deinit_pdata() callback.

Treat a NULL cpupool like a cpupool with no remaining scheduling
resources and kill the timer, which is the intent of the existing
comment in move_repl_timer().

Fixes: b6f5334aeaca ("sched: fix cpu offlining with core scheduling")
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---

 xen/common/sched/rt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
index f1feb4384e..49a9b76b89 100644
--- a/xen/common/sched/rt.c
+++ b/xen/common/sched/rt.c
@@ -766,8 +766,9 @@ rt_switch_sched(struct scheduler *new_ops, unsigned int cpu,
 
 static void move_repl_timer(struct rt_private *prv, unsigned int old_cpu)
 {
-    cpumask_t *online = get_sched_res(old_cpu)->cpupool->res_valid;
-    unsigned int new_cpu = cpumask_cycle(old_cpu, online);
+    const struct cpupool *c = get_sched_res(old_cpu)->cpupool;
+    unsigned int new_cpu = c ? cpumask_cycle(old_cpu, c->res_valid)
+                             : nr_cpu_ids;
 
     /*
      * Make sure the timer run on one of the cpus that are still available
-- 
2.43.0

base-commit: 959fa8800470e3d757b5a0bcd9394954ed7b923a
branch: amoi_sched_6
Re: [PATCH] xen/sched: rt: fix NULL cpupool dereference in move_repl_timer()
Posted by Juergen Gross 1 week ago
On 16.07.26 16:20, Oleksii Moisieiev wrote:
> schedule_cpu_rm() clears the cpupool pointer of the scheduling
> resource before calling sched_deinit_pdata():
> 
>      sr->cpupool = NULL;
>      ...
>      sched_deinit_pdata(data->old_ops, data->ppriv_old, cpu);
> 
> For RTDS, rt_deinit_pdata() calls move_repl_timer() when the
> replenishment timer lives on the cpu being removed, and
> move_repl_timer() dereferences get_sched_res(old_cpu)->cpupool
> without checking it for NULL. Removing the last pCPU from an RTDS
> cpupool therefore dereferences NULL + 0x10 (the res_valid member)
> and panics:
> 
>      (XEN) Data Abort Trap. Syndrome=0x1c28005
>      (XEN) Walking Hypervisor VA 0x10 on CPU0 via TTBR ...
>      (XEN) Xen call trace:
>      (XEN)    [<...>] find_next_bit+0x74/0xa8 (PC)
>      (XEN)    [<...>] rt.c#move_repl_timer+0xb8/0xec (LR)
>      (XEN)
>      (XEN) Panic on CPU 0:
>      (XEN) CPU0: Unexpected Trap: Data Abort
> 
> Reproducer, on any host with at least 2 pCPUs and RTDS compiled in
> (observed on arm64, but the path is common code):
> 
>      xl cpupool-create name="test" sched="rtds"
>      xl cpupool-cpu-remove Pool-0 1
>      xl cpupool-cpu-add test 1
>      xl cpupool-cpu-remove test 1
> 
> The last command moves the RTDS replenishment timer to cpu1 (first
> and only cpu of the pool) and then removes cpu1, hitting the NULL
> dereference in the sched_deinit_pdata() callback.
> 
> Treat a NULL cpupool like a cpupool with no remaining scheduling
> resources and kill the timer, which is the intent of the existing
> comment in move_repl_timer().

Hmm, is this correct?

What if there are 2 cpus in the cpupool and you are just removing the
one owning the timer? This would kill the timer instead of moving it
to the other cpu of the cpupool.

I guess the correct fix would be to add a cpupool pointer to struct
rt_private and use that in move_repl_timer().


Juergen
Re: [PATCH] xen/sched: rt: fix NULL cpupool dereference in move_repl_timer()
Posted by Oleksii Moisieiev 1 week ago
Hi Juergen,

Thanks for quick response.

On 16/07/2026 17:44, Juergen Gross wrote:
> On 16.07.26 16:20, Oleksii Moisieiev wrote:
>> schedule_cpu_rm() clears the cpupool pointer of the scheduling
>> resource before calling sched_deinit_pdata():
>>
>>      sr->cpupool = NULL;
>>      ...
>>      sched_deinit_pdata(data->old_ops, data->ppriv_old, cpu);
>>
>> For RTDS, rt_deinit_pdata() calls move_repl_timer() when the
>> replenishment timer lives on the cpu being removed, and
>> move_repl_timer() dereferences get_sched_res(old_cpu)->cpupool
>> without checking it for NULL. Removing the last pCPU from an RTDS
>> cpupool therefore dereferences NULL + 0x10 (the res_valid member)
>> and panics:
>>
>>      (XEN) Data Abort Trap. Syndrome=0x1c28005
>>      (XEN) Walking Hypervisor VA 0x10 on CPU0 via TTBR ...
>>      (XEN) Xen call trace:
>>      (XEN)    [<...>] find_next_bit+0x74/0xa8 (PC)
>>      (XEN)    [<...>] rt.c#move_repl_timer+0xb8/0xec (LR)
>>      (XEN)
>>      (XEN) Panic on CPU 0:
>>      (XEN) CPU0: Unexpected Trap: Data Abort
>>
>> Reproducer, on any host with at least 2 pCPUs and RTDS compiled in
>> (observed on arm64, but the path is common code):
>>
>>      xl cpupool-create name="test" sched="rtds"
>>      xl cpupool-cpu-remove Pool-0 1
>>      xl cpupool-cpu-add test 1
>>      xl cpupool-cpu-remove test 1
>>
>> The last command moves the RTDS replenishment timer to cpu1 (first
>> and only cpu of the pool) and then removes cpu1, hitting the NULL
>> dereference in the sched_deinit_pdata() callback.
>>
>> Treat a NULL cpupool like a cpupool with no remaining scheduling
>> resources and kill the timer, which is the intent of the existing
>> comment in move_repl_timer().
>
> Hmm, is this correct?
>
> What if there are 2 cpus in the cpupool and you are just removing the
> one owning the timer? This would kill the timer instead of moving it
> to the other cpu of the cpupool.
>
> I guess the correct fix would be to add a cpupool pointer to struct
> rt_private and use that in move_repl_timer().
>
Makes sense... I'll rethink. Thanks
>
> Juergen