I won't apply this version, please send v2 with the changes below.
On Mon, 9 Nov 2026 20:06:56 +0100, Denis V. Lunev wrote:
> The destructor never stops enospc_timer. ploop_presuspend() deletes it
> with timer_delete_sync_try(), which does not wait for a callback that
> runs on another CPU, and ploop_try_delay_enospc() tests wants_suspend
> without deferred_lock, so a write that fails with ENOSPC around the
> presuspend can arm the timer again. The callback dereferences the
> struct ploop that ploop_dtr() frees.
- Item 2 of the cover letter reply: the race is mod_timer() after the
test, and the message should say what the user gets.
The lockless test is not the cause. In 6ddad5a4d226 the test was under
deferred_lock, but mod_timer() ran after the unlock, so the timer could
be armed after presuspend's timer_delete_sync_try() from the start.
That is also why the Fixes tag is right.
"does not wait for a callback that runs on another CPU" can go: a
callback that runs during presuspend is done long before dtr.
Say what happens then: the timer expires on freed memory, and with
patch 2 it also calls wake_up_process() on a freed task.
The paragraph is one sentence chained with commas; something like:
The destructor never stops enospc_timer. ploop_try_delay_enospc()
arms it after testing wants_suspend, and nothing orders that against
timer_delete_sync_try() in ploop_presuspend(). A write that fails
with ENOSPC around the presuspend leaves the timer pending after the
suspend, and it expires on the struct ploop that ploop_dtr() freed.
> if (ploop->kt_worker) {
> + timer_shutdown_sync(&ploop->enospc_timer);
> ploop->force_md_writeback = 1;
- Item 1 of the cover letter reply: cancel event_work here too.
ploop_try_delay_enospc() calls schedule_work(&ploop->event_work) right
after mod_timer(), so the same race queues event_work after
presuspend's flush_work(). Nothing in ploop_destroy() waits for it,
and do_ploop_event_work() reads ploop->ti after the free. The window
is much shorter than the timer's, but it is the same race:
timer_shutdown_sync(&ploop->enospc_timer);
cancel_work_sync(&ploop->event_work);
INIT_WORK() runs before kt_worker is created, so it is valid under the
same check.
--
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
