Messages in this thread | | | Date | Sun, 13 Nov 2022 19:15:18 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v6 4/6] timers: Add timer_shutdown_sync() to be called before freeing timers |
| |
On Mon, 14 Nov 2022 00:18:21 +0100 Thomas Gleixner <tglx@linutronix.de> wrote:
> > @@ -1285,11 +1281,25 @@ int try_to_del_timer_sync(struct timer_list *timer) > > > > if (base->running_timer != timer) > > ret = detach_if_pending(timer, base, true); > > + if (free) > > + timer->function = NULL; > > Same problem as in the timer_shutdown() case just more subtle: > > CPU0 CPU1 > > lock_timer(timer); > base->running_timer = timer; > fn = timer->function; > unlock_timer(timer); > fn(timer) { > > __try_to_del_timer_sync(timer, free=true) > lock_timer(timer); > if (base->running_timer != timer) > // Not taken > if (free) mod_timer(timer); > if (WARN_ON_ONCE(!timer->function)) > return; // not taken > timer->function = NULL; > unlock_timer(timer); > lock_timer(timer); > enqueue_timer(timer); > unlock_timer(timer); > } > > //timer expires > lock_timer(timer); > fn = timer->function; > unlock_timer(timer); > fn(timer); <--- NULL pointer dereference > > You surely have spent a massive amount of analysis on this! > > Can you please explain how you came up with the brilliant idea of asking > Linus to pull this post -rc4 without a review from the timer maintainers > or anyone else who understands concurrency?
I trusted the source of this code:
https://lore.kernel.org/all/87pmlrkgi3.ffs@tglx/
-- Steve
| |