Re: [PATCH] tick/broadcast: Plug clockevents replacement race
From: Frederic Weisbecker
Date: Thu Oct 17 2024 - 12:17:10 EST
Le Mon, Aug 12, 2024 at 04:19:48PM +0200, Thomas Gleixner a écrit :
> 朱恺乾 reported and decoded the following race condition when a broadcast
> device is replaced:
>
> CPUA CPUB
> __tick_broadcast_oneshot_control()
> bc = tick_broadcast_device.evtdev;
> tick_install_broadcast_device(dev)
> clockevents_exchange_device(cur, dev)
> shutdown(cur);
> detach(cur);
> cur->handler = noop;
> tick_broadcast_device.evtdev = dev;
>
> tick_broadcast_set_event(bc, next_event); <- FAIL: arms a detached device.
>
> If the original broadcast device has a restricted interrupt affinity mask
> and the last CPU in that mask goes offline then the BUG() in
> tick_cleanup_dead_cpu() triggers because the clockevent device is not in
> detached state.
>
> The reason for this is that tick_install_broadcast_device() is not
> serialized vs. tick broadcast operations.
>
> The obvious cure is to serialize tick_install_broadcast_device() with
> tick_broadcast_lock against a concurrent tick broadcast operation.
>
> That requires to split clockevents_exchange_device() into two parts, one
> which does the exchange, shutdown and detach operation and the other which
> drops the module reference count. This is required because the module
> reference cannot be dropped while holding tick_broadcast_lock.
The reason why the module reference can not be dropped while holding
tick_broadcast_lock is not obvious though. What can go wrong?
Thanks.