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

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