Hi Sven, You're right. I re-examined __cancel_work_sync() and confirmed that it internally disables the work before flushing, so the re-queue race I described does not exist. I'll withdraw this patch.
Thanks for the review. Soowan Park 2026년 5월 26일 (화) 오후 4:30, Sven Eckelmann <[email protected]>님이 작성: > > On Tuesday, 26 May 2026 08:48:35 CEST Soowan Park wrote: > > batadv_dat_purge() is a periodic delayed work that re-queues itself via > > batadv_dat_start_timer() at the end of each run. When the mesh interface > > is torn down, batadv_dat_free() calls cancel_delayed_work_sync() to stop > > the purge work before freeing the DAT hash table. > > > > However, cancel_delayed_work_sync() leaves the work in an enabled state. > > If the purge work is currently executing and re-queues itself before > > cancel_delayed_work_sync() internally marks it for cancellation, the > > newly queued work escapes cancellation. This re-queued work then fires > > after batadv_dat_hash_free() has already freed the hash table but before > > the pointer is set to NULL, causing __batadv_dat_purge() to operate on a > > dangling pointer that passes the NULL check, and spin indefinitely on a > > spinlock in freed memory. > > > You are talking about a re-queue by batadv_dat_start_timer(). This only > happens when the DAT gets initialized or via the worker (batadv_dat_purge) > itself. How can the worker which is cancelled (with sync) re-queue itself? > Isn't this breaking a guarantee of cancel_delayed_work_sync() or did I > misunderstand this part of the documentation? > > > "This is cancel_work_sync() for delayed works." [1] > > "Cancel work and wait for its execution to finish. This function can be used > even if the work re-queues itself or migrates to another workqueue. On return > from this function, work is guaranteed to be not pending or executing on any > CPU as long as there aren’t racing enqueues." [2] > > (the part "This function can be used even if the work re-queues itself" is > the important part here). > > > > Replace cancel_delayed_work_sync() with disable_delayed_work_sync(), > > which additionally disables the work so that any concurrent > > queue_delayed_work() call from the running batadv_dat_purge() is > > silently rejected. This guarantees no re-queued work can fire after > > disable_delayed_work_sync() returns. > > I have no problem with using "disabled_*" everywhere (I even have a pending > patchset to use it - just to avoid problems with code changes in the future). > But since this is a fix which I don't get in the moment, I would like to > understand the problem you are describing better before applying it. > > Regards, > Sven > > > [1] > https://www.kernel.org/doc/html/v7.0/core-api/workqueue.html#c.cancel_delayed_work_sync > [2] > https://www.kernel.org/doc/html/v7.0/core-api/workqueue.html#c.cancel_work_sync
