[PATCH] netfilter: nf_log: clear per-net loggers on unregister

[PATCH] netfilter: nf_log: clear per-net loggers on unregister

[Chengfeng Ye]

Logger backends unregister their per-network namespace operations before
unregistering their global logger. This leaves a window where a sysctl or
netlink writer can rebind a per-net logger after the old per-net
selection was cleared, but before the global logger registration is
removed.

The race looks like this:

  CPU 0                                 CPU 1
  ----                                  ----
  unregister_pernet_subsys()
    nf_log_unset(net, logger)
      net->nf.nf_loggers[pf] = NULL

                                        lock nf_log_mutex
                                        find logger in loggers[][]
                                        net->nf.nf_loggers[pf] = logger
                                        unlock nf_log_mutex

  nf_log_unregister(logger)
    lock nf_log_mutex
    loggers[pf][type] = NULL
    unlock nf_log_mutex
    synchronize_rcu()
  module exit returns
  module core frees backend memory

Later, a sysctl read or nf_log_packet() reads net->nf.nf_loggers[pf]
and dereferences the stale logger.

nf_log_unregister() only removes the backend from the global logger
table. It does not clear matching net->nf.nf_loggers[] entries that were
rebound by CPU 1 after per-net teardown. Once module unload completes,
those per-net pointers can still reference static data from the unloaded
logger backend, and later readers can dereference freed module memory.

The kernel reported:

  BUG: unable to handle page fault for address: fffffbfff806d2f4
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  Oops: Oops: 0000 [#1] SMP KASAN NOPTI
  RIP: 0010:nf_log_proc_dostring+0x2aa/0x4d0
  Call Trace:
   proc_sys_call_handler+0x325/0x540
   vfs_read+0x6e1/0xa20
   ksys_read+0xf7/0x1c0
   do_syscall_64+0xf9/0x520
  Modules linked in: [last unloaded: nf_log_syslog]

Fix this by clearing matching per-net logger selections in every live
network namespace while holding nf_log_mutex after removing the global
registrations. Writers that run before this pass are covered by the
per-net clears, and writes that run later can no longer find the logger
in loggers[][]. The existing synchronize_rcu() then covers readers of
both the global and per-net pointers before module memory is released.

Fixes: 5b023fc8d8e0 ("netfilter: enable per netns support for nf_loggers")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/netfilter/nf_log.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index f4d80654dfe6..442c69f544a1 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -42,6 +42,18 @@ static struct nf_logger *__find_logger(int pf, const char *str_logger)
 	return NULL;
 }
 
+static void __nf_log_unset(struct net *net, const struct nf_logger *logger)
+{
+	const struct nf_logger *log;
+	int i;
+
+	for (i = 0; i < NFPROTO_NUMPROTO; i++) {
+		log = nft_log_dereference(net->nf.nf_loggers[i]);
+		if (log == logger)
+			RCU_INIT_POINTER(net->nf.nf_loggers[i], NULL);
+	}
+}
+
 int nf_log_set(struct net *net, u_int8_t pf, const struct nf_logger *logger)
 {
 	const struct nf_logger *log;
@@ -62,15 +74,8 @@ EXPORT_SYMBOL(nf_log_set);
 
 void nf_log_unset(struct net *net, const struct nf_logger *logger)
 {
-	int i;
-	const struct nf_logger *log;
-
 	mutex_lock(&nf_log_mutex);
-	for (i = 0; i < NFPROTO_NUMPROTO; i++) {
-		log = nft_log_dereference(net->nf.nf_loggers[i]);
-		if (log == logger)
-			RCU_INIT_POINTER(net->nf.nf_loggers[i], NULL);
-	}
+	__nf_log_unset(net, logger);
 	mutex_unlock(&nf_log_mutex);
 }
 EXPORT_SYMBOL(nf_log_unset);
@@ -112,6 +117,7 @@ EXPORT_SYMBOL(nf_log_register);
 void nf_log_unregister(struct nf_logger *logger)
 {
 	const struct nf_logger *log;
+	struct net *net;
 	int i;
 
 	mutex_lock(&nf_log_mutex);
@@ -120,6 +126,10 @@ void nf_log_unregister(struct nf_logger *logger)
 		if (log == logger)
 			RCU_INIT_POINTER(loggers[i][logger->type], NULL);
 	}
+	rcu_read_lock();
+	for_each_net_rcu(net)
+		__nf_log_unset(net, logger);
+	rcu_read_unlock();
 	mutex_unlock(&nf_log_mutex);
 	synchronize_rcu();
 }
-- 
2.43.0

Re: [PATCH] netfilter: nf_log: clear per-net loggers on unregister

[Florian Westphal]

Chengfeng Ye <nicoyip.dev@gmail.com> wrote:
> Logger backends unregister their per-network namespace operations before
> unregistering their global logger. This leaves a window where a sysctl or
> netlink writer can rebind a per-net logger after the old per-net
> selection was cleared, but before the global logger registration is
> removed.
> 
> The race looks like this:
> 
>   CPU 0                                 CPU 1
>   ----                                  ----
>   unregister_pernet_subsys()
>     nf_log_unset(net, logger)
>       net->nf.nf_loggers[pf] = NULL
> 
>                                         lock nf_log_mutex
>                                         find logger in loggers[][]
>                                         net->nf.nf_loggers[pf] = logger
>                                         unlock nf_log_mutex
> Later, a sysctl read or nf_log_packet() reads net->nf.nf_loggers[pf]
> and dereferences the stale logger.
> 
> nf_log_unregister() only removes the backend from the global logger
> table. It does not clear matching net->nf.nf_loggers[] entries that were
> rebound by CPU 1 after per-net teardown. Once module unload completes,
> those per-net pointers can still reference static data from the unloaded
> logger backend, and later readers can dereference freed module memory.

Why can't this be fixed by unregistering in reverse order?

Re: [PATCH] netfilter: nf_log: clear per-net loggers on unregister

[Chengfeng Ye]

On Sun, Aug 23, 2026 at 11:51 PM Florian Westphal <fw@strlen.de> wrote:
>
> Chengfeng Ye <nicoyip.dev@gmail.com> wrote:
> > Logger backends unregister their per-network namespace operations before
> > unregistering their global logger. This leaves a window where a sysctl or
> > netlink writer can rebind a per-net logger after the old per-net
> > selection was cleared, but before the global logger registration is
> > removed.
> >
> > The race looks like this:
> >
> >   CPU 0                                 CPU 1
> >   ----                                  ----
> >   unregister_pernet_subsys()
> >     nf_log_unset(net, logger)
> >       net->nf.nf_loggers[pf] = NULL
> >
> >                                         lock nf_log_mutex
> >                                         find logger in loggers[][]
> >                                         net->nf.nf_loggers[pf] = logger
> >                                         unlock nf_log_mutex
> > Later, a sysctl read or nf_log_packet() reads net->nf.nf_loggers[pf]
> > and dereferences the stale logger.
> >
> > nf_log_unregister() only removes the backend from the global logger
> > table. It does not clear matching net->nf.nf_loggers[] entries that were
> > rebound by CPU 1 after per-net teardown. Once module unload completes,
> > those per-net pointers can still reference static data from the unloaded
> > logger backend, and later readers can dereference freed module memory.
>
> Why can't this be fixed by unregistering in reverse order?

Right, that would be much more clean, I will send the v2 right away.

Best,
Chengfeng