Re: [PATCH] dlm: wait for outstanding SRCU callbacks to complete in exit paths

["Zqiang" <qiang.zhang@linux.dev>]

> 
> > 
> > Hi,
> >  
> >  On Tue, Sep 1, 2026 at 5:19 AM Zqiang <qiang.zhang@linux.dev> 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 should rephrase it:

     Due to missed call to the srcu_barrier(), when invoke cleanup_srcu_struct()
     and find there are still pending srcu callback, the WARN_ON() will trigger.

     from kernel/rcu/srcutree.c in cleanup_srcu_struct():


                // Call srcu_barrier() before this cleanup_srcu_struct()
                // to avoid triggering this WARN_ON().
                if (WARN_ON(timer_delete_sync(&sdp->delay_work) &&
                            rcu_segcblist_n_cbs(&sdp->srcu_cblist)) &&
                    rcu_cpu_beenfullyonline(sdp->cpu))
                        queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
                flush_work(&sdp->work);

Thanks
Zqiang

> >  
> >  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);
> ....
> 
> Thanks
> Zqiang
> 
> > 
> > - Alex
> >  
> >  
> >  Add an srcu_barrier() after the call_srcu() loop in each exit function
> >  to wait for all outstanding callbacks of the respective SRCU domain to
> >  complete before returning. In dlm_midcomms_exit() the barrier is issued
> >  before dlm_lowcomms_exit() so that node callbacks are drained prior to
> >  tearing down the lower layer.
> >  
> >  Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> >  ---
> >  fs/dlm/lowcomms.c | 1 +
> >  fs/dlm/midcomms.c | 1 +
> >  2 files changed, 2 insertions(+)
> >  
> >  diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> >  index 2aff1c7c17de..ea8353c4638d 100644
> >  --- a/fs/dlm/lowcomms.c
> >  +++ b/fs/dlm/lowcomms.c
> >  @@ -1984,4 +1984,5 @@ void dlm_lowcomms_exit(void)
> >  }
> >  }
> >  srcu_read_unlock(&connections_srcu, idx);
> >  + srcu_barrier(&connections_srcu);
> >  }
> >  diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
> >  index 8964164600d2..045431524494 100644
> >  --- a/fs/dlm/midcomms.c
> >  +++ b/fs/dlm/midcomms.c
> >  @@ -1178,6 +1178,7 @@ void dlm_midcomms_exit(void)
> >  }
> >  }
> >  srcu_read_unlock(&nodes_srcu, idx);
> >  + srcu_barrier(&nodes_srcu);
> >  
> >  dlm_lowcomms_exit();
> >  }
> >  --
> >  2.17.1
> >
>