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".
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?
Thanx, Paul
