Hi, On Wed, Sep 2, 2026 at 12:50 PM Paul E. McKenney <[email protected]> wrote: > > On Wed, Sep 02, 2026 at 10:30:15AM -0400, Alexander Aring wrote: > > Hi, > > > > On Wed, Sep 2, 2026 at 10:16 AM Zqiang <[email protected]> wrote: > > > > > > > > > > > Hi, > > > > > > > > On Tue, Sep 1, 2026 at 5:19 AM Zqiang <[email protected]> wrote: > > > > > > > > > > > > > > The dlm_lowcomms_exit() and dlm_midcomms_exit() iterate over the > > > > > srcu protected connection and node hash tables and hand each > > > > > element to call_srcu() for deferred freeing (connection_release() > > > > > and midcomms_node_release()). call_srcu() is asynchronous: the > > > > > callbacks are invoked only after an SRCU grace period, which may > > > > > happen after the exit function has already returned. > > > > > > > > > > These exit functions are reached from exit_dlm() on module unload. > > > > > Once they return, module teardown continues and the module text > > > > > may be unloaded while call_srcu() callbacks are still pending. When > > > > > such a callback finally runs, it executes freed module code and > > > > > touches the static SRCU domains that are being torn down, resulting > > > > > in a use-after-free. > > > > > > > > > I thought again about this and in my opinion this is not possible as > > > > it is already being handled by DEFINE_STATIC_SRCU() with a cleanup > > > > handling when the module is unloaded. > > > > > > When the moudle unload, the srcu_module_going() will call > > > cleanup_srcu_struct() > > > and free_percpu(ssp->sda) to release resource. but we not call > > > srcu_barrier(), > > > the srcu_barrier() should be called before cleanup_srcu_struct(). > > > > > > > > > > I know that srcu subsystem does a lot of magic with modules init/exit > > > > functionality to call init_srcu_struct() and cleanup_srcu_struct(). > > > > See > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v7.3-rc1#n2711 > > > > > > > > so this patch should be reverted. If they don't use a barrier there, > > > > the fix should be in the SRCU subsystem, but I believe the current > > > > SRCU implementation already handles this. > > > > > > The srcu_barrier() need to be called, there are some description > > > from Documentation/RCU/rcubarrier.rst: > > > > > > rcu_barrier() > > > ------------- > > > > > > This situation can be handled by the rcu_barrier() primitive. Rather > > > than waiting for a grace period to elapse, rcu_barrier() waits for all > > > outstanding RCU callbacks to complete. Please note that rcu_barrier() > > > does **not** imply synchronize_rcu(), in particular, if there are no RCU > > > callbacks queued anywhere, rcu_barrier() is within its rights to return > > > immediately, without waiting for anything, let alone a grace period. > > > > > > Pseudo-code using rcu_barrier() is as follows: > > > > > > 1. Prevent any new RCU callbacks from being posted. > > > 2. Execute rcu_barrier(). > > > 3. Allow the module to be unloaded. > > > > > > There is also an srcu_barrier() function for SRCU, and you of course > > > must match the flavor of srcu_barrier() with that of call_srcu(). > > > If your module uses multiple srcu_struct structures, then it must also > > > use multiple invocations of srcu_barrier() when unloading that module. > > > For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and > > > call_srcu() on srcu_struct_2, then the following three lines of code > > > will be required when unloading:: > > > > > > 1 rcu_barrier(); > > > 2 srcu_barrier(&srcu_struct_1); > > > 3 srcu_barrier(&srcu_struct_2); > > > .... > > > > yes, I can see this makes problems when there is kmem_cache involved > > and you need to be sure it's being done before you destroy the > > kmem_cache. > > > > This is not the case here. > > > > I looked more into srcu functionality "cleanup_srcu_struct()" > > (srcutiny) and it does "flush_work(&ssp->srcu_work)", workfn is > > "srcu_drive_gp()" and the comment states "Workqueue handler to drive > > one grace period and invoke any callbacks that become ready as a > > result." > > > > In my opinion "cleanup_srcu_struct()" should be sure there are no > > pending operations. > > If the user never passed this srcu_struct structure to call_srcu(), > then there would be nothing to clean up. Now, maybe all SRCU use cases > are OK with the extra srcu_barrier() overhead at cleanup_srcu_struct() > time, but as far as I know, that is still a "maybe". >
Okay, I agree with the "maybe" argument, then maybe we add a comment to "cleanup_srcu_struct()" that there must be a "srcu_barrier()" called before. Except "call_srcu()" is never being used. > And of course, cleanup_srcu_struct() has no ability to prevent the > user from doing a (buggy!) concurrent call to synchronize_srcu() or > call_srcu(), which would defeat any attempt by cleanup_srcu_struct() > to do this pending-operation cleanup. > > Or am I missing your point? I use "DEFINE_STATIC_SRCU()" that does the srcu cleanup for me and I was confused I need to care about "srcu_barrier()" before module exit. Then "DEFINE_STATIC_SRCU()" needs a comment as well to call "srcu_barrier()" before module exit functionality as my initial thought would be it would do that for me as I am even not a caller of "cleanup_srcu_struct()". All of this, of course, should happen in a safe context. Wheras for "DEFINE_STATIC_SRCU()" I would assume after users module exit function there is nothing going on anymore. - Alex
