Re: [PATCH net v2] ppp: defer channel free to an RCU grace period to fix pppol2tp RX UAF

Sebastian Andrzej Siewior <[email protected]>
Newsgroups org.kernel.vger.linux-modules,org.kernel.vger.linux-kernel,org.kernel.vger.linux-ppp,org.kernel.vger.netdev
Message-ID <[email protected]>
+ MODULE maintainer

On 2026-07-05 10:57:44 [+0800], Qingfang Deng wrote:
> On 7/4/2026 at 12:32 AM, Breno Leitao wrote:
> > On Fri, Jul 03, 2026 at 03:27:00PM +0800, Qingfang Deng wrote:
> > > AI-review found an issue: https://sashiko.dev/#/patchset/D9C0245B-608B-4884-8A09-F55BA4A9F948%40doyensec.com
> > > 
> > > An rcu_barrier() call is needed at the end of ppp_cleanup().
> > 
> > I was initially unclear why rcu_barrier() would be necessary on a kfree path,
> > but it appears to be required during module unload to ensure that
> > ppp_release_channel_free() completes before the module's struct rcu_head is
> > destroyed. Is that the correct understanding?
> 
> It's required to ensure that all ppp_release_channel_free() callback
> complete before the text segment of the module is unloaded.

So either a rcu_barrier() in ppp's module_exit() callback or a
synchronize_rcu() instead of the call_rcu(). And all this because the
module RCU callbacks pending which can be invoked after the module has
been removed. There is a synchronize_rcu() during module exit but this
is after the module code is gone.

I'm curious how many modules have a call_rcu() within their code but
don't have anything to enforce its completion before module removal is
complete? Wouldn't something like


diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a6058..8eae1ea2d6eb4 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -858,6 +858,9 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		goto out;
 
 	mutex_unlock(&module_mutex);
+
+	/* Ensure all rcu callbacks issued by the module have completed */
+	rcu_barrier();
 	/* Final destruction now no one is using it. */
 	if (mod->exit != NULL)
 		mod->exit();

make sense?

Sebastian
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.