fs/dlm/lowcomms.c | 1 + fs/dlm/midcomms.c | 1 + 2 files changed, 2 insertions(+)
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.
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
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 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. 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. - 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 >
> > 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 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 > > >
Hi, On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote: > > > > > 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 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. - Alex
On Wed, Sep 02, 2026 at 10:30:15AM -0400, Alexander Aring wrote: > Hi, > > On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote: > > > > > > > > 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 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
Hi, On Wed, Sep 2, 2026 at 12:50 PM Paul E. McKenney <paulmck@kernel.org> 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 <qiang.zhang@linux.dev> wrote: > > > > > > > > > > > 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 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
On Wed, Sep 02, 2026 at 01:13:50PM -0400, Alexander Aring wrote:
> Hi,
>
> On Wed, Sep 2, 2026 at 12:50 PM Paul E. McKenney <paulmck@kernel.org> 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 <qiang.zhang@linux.dev> wrote:
> > > >
> > > > >
> > > > > 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 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.
Fair point! How about this?
Now I am wondering if the various DEFINE_SRCU() macros need kernel-doc
headers...
Thanx, Paul
------------------------------------------------------------------------
commit 2a20fcf8bd8ffffd0e7cdd0adf80e41d1efdfc2b
Author: Paul E. McKenney <paulmck@kernel.org>
Date: Wed Sep 2 12:58:24 2026 -0700
srcutree: Explicitly note DEFINE_SRCU() needs for srcu_barrier()
In the core kernel, an srcu_struct structure created by DEFINE_SRCU()
or friends lives as long as the kernel does, so there are no particular
requirements surrounding the end of that structure's life. In contrast,
when DEFINE_SRCU() and friends are used within a module, the corresponding
srcu_struct structures' lifetimes end when that module exits. This in
turn means that if such a structure was passed to call_srcu(), then
srcu_barrier() must be invoked after the last call_srcu() invocation
but before the module exits.
This commit therefore adds a comment stating this.
Reported-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index ad9d9658b0a2f5..93b76e54389432 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -214,6 +214,12 @@ struct srcu_struct {
* instead of smp_mb(), and given that the first (for example)
* srcu_read_lock_fast() might race with the first synchronize_srcu(),
* this different must be specified at initialization time.
+ *
+ * If you use any of the DEFINE_SRCU() functions within a module, the
+ * module-entry code will invoke init_srcu_struct() and the module-exit
+ * code will invoke cleanup_srcu_struct(). This means that if your module
+ * passes the resulting srcu_struct structure to call_srcu(), you will
+ * need to also pass this structure to srcu_barrier() prior to module exit.
*/
#ifdef MODULE
# define __DEFINE_SRCU(name, fast, is_static) \
Hi, On Wed, Sep 2, 2026 at 10:30 AM Alexander Aring <aahringo@redhat.com> wrote: > > Hi, > > On Wed, Sep 2, 2026 at 10:16 AM Zqiang <qiang.zhang@linux.dev> wrote: > > > > > > > > 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 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. > okay, it can be the case for "nodes_srcu" but with the argumentation it can release memory being used by kmem_cache that is being destroyed later. - Alex
>
> >
> > 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
> >
>
Hi, On Wed, Sep 2, 2026 at 10:30 AM Zqiang <qiang.zhang@linux.dev> wrote: > > > > > > > > > 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() Then there need to be a MUST call of srcu_barrier() before cleanup_srcu_struct() every time. I am confused here. - Alex
> > Hi, > > On Wed, Sep 2, 2026 at 10:30 AM Zqiang <qiang.zhang@linux.dev> wrote: > > > > > > > > > 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() > > > Then there need to be a MUST call of srcu_barrier() before > cleanup_srcu_struct() every time. If there are some call_srcu() call, the srcu_barrier() should be called before cleanup_srcu_struct(). usually, when the cleanup_srcu_struct() run, we should ensure that there are no pending SRCU callbacks or ongoing SRCU grace periods at this time. there are another reason: The midcomms_node_release() srcu callback can call call_rcu(), we must wait the all call_rcu() to complete, because rcu_barrier() which in dlm_memory_exit() is required to successfully intercept these RCU callbacks. Thanks Zqiang > > I am confused here. > > - Alex >
Hi, On Wed, Sep 2, 2026 at 10:49 AM Zqiang <qiang.zhang@linux.dev> wrote: > > > > > Hi, > > > > On Wed, Sep 2, 2026 at 10:30 AM Zqiang <qiang.zhang@linux.dev> wrote: > > > > > > > > > > > > > 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() > > > > > Then there need to be a MUST call of srcu_barrier() before > > cleanup_srcu_struct() every time. > > If there are some call_srcu() call, the srcu_barrier() should be called before cleanup_srcu_struct(). > usually, when the cleanup_srcu_struct() run, we should ensure that there are no pending SRCU callbacks > or ongoing SRCU grace periods at this time. > For me, this is a MUST because who is maintaining something to know if it was ever called? > there are another reason: > > The midcomms_node_release() srcu callback can call call_rcu(), we must wait the all > call_rcu() to complete, because rcu_barrier() which in dlm_memory_exit() is required > to successfully intercept these RCU callbacks. > Yes, I can see why "nodes_srcu" is necessary as this has something to do with kmem_cache is being involed. The "connections_srcu" is necessary for the above reason that the srcu API requires you to call srcu_barrier() before cleanup_srcu_struct() because for whatever reason we don't do that in cleanup_srcu_struct() as I understand it now. - Alex
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. > > 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> Acked-by: Alexander Aring <aahringo@redhat.com> - Alex
© 2016 - 2026 Red Hat, Inc.