Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] rv: missing rcu_barrier() for non-pool allocations causes UAF on module unload --
commit f8013d6f70621f02511ed8a820d487338e72eed7 Author: Wen Yang <[email protected]> rv: Add per-object teardown hook da_extra_cleanup da_monitor_destroy() frees each per-object storage, but a monitor whose target is allocated separately has no point to free it: the target is still reachable from the storage until hash_del_rcu(). Add an optional da_extra_cleanup() hook, called for each storage after da_monitor_free_hook() has disarmed its timer and before the storage is unlinked. It defaults to a no-op. tlob, added later in this series, uses it to retire and free its per-window record. > diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h > index 7d1026e9edbc6..04caaf529034b 100644 > --- a/include/rv/da_monitor.h > +++ b/include/rv/da_monitor.h [ ... ] > @@ -715,7 +725,9 @@ static inline void da_monitor_destroy(void) > * pending, we can safely assume no concurrent user. > */ > hash_for_each_safe(da_monitor_ht, bkt, tmp, mon_storage, node) { > + /* Before da_extra_cleanup(): it frees what the timer reads. */ > da_monitor_free_hook(&mon_storage->rv.da_mon); > + da_extra_cleanup(&mon_storage->rv.da_mon); > hash_del_rcu(&mon_storage->node); > if (DA_MON_ALLOCATION_STRATEGY == DA_ALLOC_POOL) > mempool_free(mon_storage, &da_monitor_pool); [Severity: High] Does this missing rcu_barrier() for non-pool allocations cause a use-after-free on module unload? Further down in da_monitor_destroy(), rcu_barrier() is only called for DA_ALLOC_POOL allocations: include/rv/da_monitor.h:da_monitor_destroy() { ... if (DA_MON_ALLOCATION_STRATEGY == DA_ALLOC_POOL) { rcu_barrier(); mempool_exit(&da_monitor_pool); } } When a monitor using DA_ALLOC_AUTO or DA_ALLOC_MANUAL is active and an event calls da_destroy_storage(), it queues the da_kfree_cb() callback: include/rv/da_monitor.h:da_destroy_storage() { ... if (DA_MON_ALLOCATION_STRATEGY == DA_ALLOC_POOL) call_rcu(&mon_storage->rcu, da_pool_return_cb); else call_rcu(&mon_storage->rcu, da_kfree_cb); } Because da_kfree_cb() is a static function in this header, its code resides in the loadable module of the specific monitor. If the user unloads the monitor module immediately after a monitored object is destroyed, da_monitor_destroy() is called during the module exit path. Since there is no rcu_barrier() for non-pool allocations, the module's text segment is unmapped while da_kfree_cb() is still pending in the RCU queues. This leads to a kernel panic when the RCU core attempts to execute the unmapped callback. Should the rcu_barrier() be called unconditionally so that all pending RCU callbacks finish executing before the module exits and its memory is unmapped? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
