@paulmckrcu, regarding https://lore.kernel.org/kvm/08ee7eb2-8d08-4f1f-9c46-495a544b8c0e@paulmck-laptop/, we went with option 3 and implemented rcu_kvfree_barrier() in #grsecurity, mainly in need for AUTOSLAB which converts every kmalloc() into a dedicated slab cache, making the issue much more likely to trigger. Placing a call to rcu_kvfree_barrier() at a fitting place in free_module() fixes the leak/uaf issue.

@minipli Thank you for the information! What are your thoughts on the additional options called out here? https://docs.google.com/document/d/1v0rcZLvvjVGejT3523W0rDy_sLFu2LWc_NR3fQItZaA/edit?usp=sharing

@paulmckrcu Either kmem_cache_free_barrier() or kmem_cache_destroy_wait() sound sensible to me. Unloading modules should be a rare enough event, more so ones with slab caches making use kfree_rcu() for these. But I also like the optimization of batching the waiting phase in case of multiple RCU slab caches via kmem_cache_destroy_rcu()/_barrier(). It could be as easy as implementing kmem_cache_destroy_rcu() as call_rcu() and in the callback call kfree_rcu_barrier() once to wait for all in-flight kfree_rcu()s and kmem_cache_destroy() afterwards — possibly for all queued caches. kmem_cache_destroy_barrier() then just has to wait for the previous call_rcu() getting processed.

@minipli Thank you for looking into this! Any thoughts on Uladzislau's prototype? https://lore.kernel.org/all/Zmw5FTX752g0vtlD@pc638.lan/

@paulmckrcu I don’t like it because:
1/ It has no explicit synchronization with module unloading. But you want that to have a point in time where to decide if a cache leaks memory or got flushed fully, i.e. all pending kfree_rcu() callbacks possibly targeting that cache have been processed and no further may come.
2/ The once per second polling has no end to it, so a leaking cache will cause unnecessary wakeups indefinitely — without any related warning at all.

IMHO, it really needs the explicit (and blocking) synchronization primitive to flush all pending deferred cache frees. Otherwise it’s an endless wait with no outcome nor debug support (e.g. no way to detecting the leaks).

@minipli Good point, and a later patch from Vlastimil Babka adds warnings if the slab does not fully free up in reasonable time.