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

Chengfeng Ye <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel,org.kernel.vger.netfilter-devel,org.kernel.vger.stable
Message-ID <[email protected]>
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: [email protected]
Signed-off-by: Chengfeng Ye <[email protected]>
---
 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
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.