Re: [PATCH v6 5/9] rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable
From: sashiko-bot@kernel.org
Date: 2026-08-20 16:59:21
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Non-synchronous timer cancellation combined with `synchronize_rcu()` or `kfree_rcu()` leads to a Use-After-Free during monitor teardown. --
--- Patch [5]: [PATCH v6 5/9] rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable ---commit 693c5f5465506e090a4b8b44d5e3c72769ec0685 Author: Wen Yang [off-list ref] rv: Make da_monitor_reset_hook and EVENT_NONE_LBL overridable This commit wraps da_monitor_reset_hook and EVENT_NONE_LBL with ifndef guards to allow HA-based monitors to provide their own implementations. It enables monitors like tlob to define custom reset hooks for tasks such as cancelling per-task hrtimers.
quoted hunk ↗ jump to hunk
diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h index e1738d199b289..807b981eb548a 100644 --- a/include/rv/ha_monitor.h +++ b/include/rv/ha_monitor.h@@ -36,8 +36,14 @@ static bool ha_monitor_handle_constraint(struct da_monitor *da_mon, da_id_type id); #define da_monitor_event_hook ha_monitor_handle_constraint #define da_monitor_init_hook ha_monitor_init_env + +/* Overrides must still call ha_monitor_reset_env() or cancel the timer. */ +#ifndef da_monitor_reset_hook #define da_monitor_reset_hook ha_monitor_reset_env +#endif +#ifndef da_monitor_sync_hook #define da_monitor_sync_hook() synchronize_rcu() +#endif
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to rely on
synchronize_rcu() to wait for cancelled timers during monitor teardown?
Looking at the global teardown path:
include/rv/da_monitor.h:da_monitor_destroy() {
...
da_monitor_reset_all();
da_monitor_sync_hook();
...
}
And per-object teardown:
include/rv/da_monitor.h:da_destroy_storage() {
...
da_monitor_reset_hook(&mon_storage->rv.da_mon);
hash_del_rcu(&mon_storage->node);
if (DA_MON_ALLOCATION_STRATEGY == DA_ALLOC_POOL)
call_rcu(&mon_storage->rcu, da_pool_return_cb);
else
kfree_rcu(mon_storage, rcu);
...
}
Both paths cancel timers non-synchronously in ha_monitor_reset_env():
include/rv/ha_monitor.h:ha_monitor_reset_env() {
...
ha_cancel_timer(ha_mon);
...
}
They then rely on RCU grace periods to wait for executing callbacks,
assuming the callbacks execute within an RCU read-side critical section:
include/rv/ha_monitor.h:__ha_monitor_timer_callback() {
...
guard(rcu)();
...
}
If the timer callback is dispatched but preempted or delayed before
entering its RCU read-side critical section, synchronize_rcu() will not
wait for it. When the callback resumes, couldn't it access the freed
monitor storage and cause a use-after-free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787243842.git.wen.yang@linux.dev?part=5