{"id":2233357,"url":"http://patchwork.ozlabs.org/api/1.1/patches/2233357/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/patch/20260506100728.2664-3-fw@strlen.de/","project":{"id":26,"url":"http://patchwork.ozlabs.org/api/1.1/projects/26/?format=json","name":"Netfilter Development","link_name":"netfilter-devel","list_id":"netfilter-devel.vger.kernel.org","list_email":"netfilter-devel@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null},"msgid":"<20260506100728.2664-3-fw@strlen.de>","date":"2026-05-06T10:07:14","name":"[v3,nf,2/8] netfilter: xtables: allocate hook ops while under mutex","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"a786e8c8365056ef6a7c701e1acbbd7b649f9f7e","submitter":{"id":1025,"url":"http://patchwork.ozlabs.org/api/1.1/people/1025/?format=json","name":"Florian Westphal","email":"fw@strlen.de"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/netfilter-devel/patch/20260506100728.2664-3-fw@strlen.de/mbox/","series":[{"id":502948,"url":"http://patchwork.ozlabs.org/api/1.1/series/502948/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/list/?series=502948","date":"2026-05-06T10:07:14","name":"netfilter: xtables: fix module load and teardown races","version":3,"mbox":"http://patchwork.ozlabs.org/series/502948/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/2233357/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/2233357/checks/","tags":{},"headers":{"Return-Path":"\n <netfilter-devel+bounces-12453-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","netfilter-devel@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.234.253.10; helo=sea.lore.kernel.org;\n envelope-from=netfilter-devel+bounces-12453-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=91.216.245.30","smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=Chamillionaire.breakpoint.cc"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org [172.234.253.10])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g9WKS5rbkz1yJx\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 06 May 2026 20:07:56 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id B36DD30097C0\n\tfor <incoming@patchwork.ozlabs.org>; Wed,  6 May 2026 10:07:51 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id A74BA3F23AE;\n\tWed,  6 May 2026 10:07:50 +0000 (UTC)","from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc\n [91.216.245.30])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 46A983FADFF\n\tfor <netfilter-devel@vger.kernel.org>; Wed,  6 May 2026 10:07:45 +0000 (UTC)","by Chamillionaire.breakpoint.cc (Postfix, from userid 1003)\n\tid C5EB7605F3; Wed, 06 May 2026 12:07:42 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778062068; cv=none;\n b=DDpy+WAGIWI233gPmMAVzFp7ommBuMAdIEaqwYweKRubTgBZvM+Dle2Z/rzR0rrzujh4akbYDfqbYejIhuSW6sLIvvGcm9k4EyHVjpgoTx9WhE8iUIYcKjkki8kvYS0QFGIuY6tt2VG9wUg+zK8UF8bgM7cEx3ILeOOztxxkksA=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778062068; c=relaxed/simple;\n\tbh=rxc8ooKPLwPzXXPqPXVpfUl8clP8rQwtQN1iyUc5mis=;\n\th=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:\n\t MIME-Version;\n b=LQy3a/1DfewY2gbjxtMBd9q+TClgHwGQt/Q+HVL47AnOHKQ/FglJ8j04Dr+2RQ8RGKOdD+aBhdYvCb8sUrxTcOKx4sc4VVE/7Y2nSoAAnlS67+xPVKBGl9NMFmB6oJoYPMkXWgB3Dtw1QaqJUgSrIR3Th2Q72HOebkL6Fj1wN6M=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de;\n spf=pass smtp.mailfrom=Chamillionaire.breakpoint.cc;\n arc=none smtp.client-ip=91.216.245.30","From":"Florian Westphal <fw@strlen.de>","To":"<netfilter-devel@vger.kernel.org>","Cc":"tristan@talencesecurity.com,\n\tFlorian Westphal <fw@strlen.de>","Subject":"[PATCH v3 nf 2/8] netfilter: xtables: allocate hook ops while under\n mutex","Date":"Wed,  6 May 2026 12:07:14 +0200","Message-ID":"<20260506100728.2664-3-fw@strlen.de>","X-Mailer":"git-send-email 2.53.0","In-Reply-To":"<20260506100728.2664-1-fw@strlen.de>","References":"<20260506100728.2664-1-fw@strlen.de>","Precedence":"bulk","X-Mailing-List":"netfilter-devel@vger.kernel.org","List-Id":"<netfilter-devel.vger.kernel.org>","List-Subscribe":"<mailto:netfilter-devel+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:netfilter-devel+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit"},"content":"arp/ip(6)t_register_table() add the table to the per-netns list via\nxt_register_table() before allocating the per-netns hook ops copy\nvia kmemdup_array().  This leaves a window where the table is\nvisible in the list with ops=NULL.\n\nIf the pernet exit happens runs concurrently the pre_exit callback finds\nthe table via xt_find_table() and passes the NULL ops pointer to\nnf_unregister_net_hooks(), causing a NULL dereference:\n\n  general protection fault in nf_unregister_net_hooks+0xbc/0x150\n  RIP: nf_unregister_net_hooks (net/netfilter/core.c:613)\n  Call Trace:\n    ipt_unregister_table_pre_exit\n    iptable_mangle_net_pre_exit\n    ops_pre_exit_list\n    cleanup_net\n\nFix by moving the ops allocation into the xtables core so the table is\nnever in the list without valid ops.  Also ensure the table is no longer\nprocessing packets before its torn down on error unwind.\nnf_register_net_hooks might have published at least one hook; call\nsynchronize_rcu() if there was an error.\n\naudit log register message gets deferred until all operations have\npassed, this avoids need to emit another ureg message in case of\nerror unwinding.\n\nBased on earlier patch by Tristan Madani.\n\nFixes: f9006acc8dfe5 (\"netfilter: arp_tables: pass table pointer via nf_hook_ops\")\nFixes: ee177a54413a (\"netfilter: ip6_tables: pass table pointer via nf_hook_ops\")\nFixes: ae689334225f (\"netfilter: ip_tables: pass table pointer via nf_hook_ops\")\nLink: https://lore.kernel.org/netfilter-devel/20260429175613.1459342-1-tristmd@gmail.com/\nSigned-off-by: Tristan Madani <tristan@talencesecurity.com>\nSigned-off-by: Florian Westphal <fw@strlen.de>\n---\n v3: no changes.\n include/linux/netfilter/x_tables.h |  1 +\n net/ipv4/netfilter/arp_tables.c    | 35 +++------------------\n net/ipv4/netfilter/ip_tables.c     | 41 +++---------------------\n net/ipv6/netfilter/ip6_tables.c    | 38 +++--------------------\n net/netfilter/x_tables.c           | 50 +++++++++++++++++++++++++-----\n 5 files changed, 55 insertions(+), 110 deletions(-)","diff":"diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h\nindex a81b46af5118..cb4b694dd9e4 100644\n--- a/include/linux/netfilter/x_tables.h\n+++ b/include/linux/netfilter/x_tables.h\n@@ -305,6 +305,7 @@ struct xt_counters *xt_counters_alloc(unsigned int counters);\n \n struct xt_table *xt_register_table(struct net *net,\n \t\t\t\t   const struct xt_table *table,\n+\t\t\t\t   const struct nf_hook_ops *template_ops,\n \t\t\t\t   struct xt_table_info *bootstrap,\n \t\t\t\t   struct xt_table_info *newinfo);\n void *xt_unregister_table(struct xt_table *table);\ndiff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c\nindex 97ead883e4a1..c02e46a0271a 100644\n--- a/net/ipv4/netfilter/arp_tables.c\n+++ b/net/ipv4/netfilter/arp_tables.c\n@@ -1522,13 +1522,11 @@ int arpt_register_table(struct net *net,\n \t\t\tconst struct arpt_replace *repl,\n \t\t\tconst struct nf_hook_ops *template_ops)\n {\n-\tstruct nf_hook_ops *ops;\n-\tunsigned int num_ops;\n-\tint ret, i;\n-\tstruct xt_table_info *newinfo;\n \tstruct xt_table_info bootstrap = {0};\n-\tvoid *loc_cpu_entry;\n+\tstruct xt_table_info *newinfo;\n \tstruct xt_table *new_table;\n+\tvoid *loc_cpu_entry;\n+\tint ret;\n \n \tnewinfo = xt_alloc_table_info(repl->size);\n \tif (!newinfo)\n@@ -1543,7 +1541,7 @@ int arpt_register_table(struct net *net,\n \t\treturn ret;\n \t}\n \n-\tnew_table = xt_register_table(net, table, &bootstrap, newinfo);\n+\tnew_table = xt_register_table(net, table, template_ops, &bootstrap, newinfo);\n \tif (IS_ERR(new_table)) {\n \t\tstruct arpt_entry *iter;\n \n@@ -1553,31 +1551,6 @@ int arpt_register_table(struct net *net,\n \t\treturn PTR_ERR(new_table);\n \t}\n \n-\tnum_ops = hweight32(table->valid_hooks);\n-\tif (num_ops == 0) {\n-\t\tret = -EINVAL;\n-\t\tgoto out_free;\n-\t}\n-\n-\tops = kmemdup_array(template_ops, num_ops, sizeof(*ops), GFP_KERNEL);\n-\tif (!ops) {\n-\t\tret = -ENOMEM;\n-\t\tgoto out_free;\n-\t}\n-\n-\tfor (i = 0; i < num_ops; i++)\n-\t\tops[i].priv = new_table;\n-\n-\tnew_table->ops = ops;\n-\n-\tret = nf_register_net_hooks(net, ops, num_ops);\n-\tif (ret != 0)\n-\t\tgoto out_free;\n-\n-\treturn ret;\n-\n-out_free:\n-\t__arpt_unregister_table(net, new_table);\n \treturn ret;\n }\n \ndiff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c\nindex 23c8deff8095..488c5945ebb2 100644\n--- a/net/ipv4/netfilter/ip_tables.c\n+++ b/net/ipv4/netfilter/ip_tables.c\n@@ -1724,13 +1724,11 @@ int ipt_register_table(struct net *net, const struct xt_table *table,\n \t\t       const struct ipt_replace *repl,\n \t\t       const struct nf_hook_ops *template_ops)\n {\n-\tstruct nf_hook_ops *ops;\n-\tunsigned int num_ops;\n-\tint ret, i;\n-\tstruct xt_table_info *newinfo;\n \tstruct xt_table_info bootstrap = {0};\n-\tvoid *loc_cpu_entry;\n+\tstruct xt_table_info *newinfo;\n \tstruct xt_table *new_table;\n+\tvoid *loc_cpu_entry;\n+\tint ret;\n \n \tnewinfo = xt_alloc_table_info(repl->size);\n \tif (!newinfo)\n@@ -1745,7 +1743,7 @@ int ipt_register_table(struct net *net, const struct xt_table *table,\n \t\treturn ret;\n \t}\n \n-\tnew_table = xt_register_table(net, table, &bootstrap, newinfo);\n+\tnew_table = xt_register_table(net, table, template_ops, &bootstrap, newinfo);\n \tif (IS_ERR(new_table)) {\n \t\tstruct ipt_entry *iter;\n \n@@ -1755,37 +1753,6 @@ int ipt_register_table(struct net *net, const struct xt_table *table,\n \t\treturn PTR_ERR(new_table);\n \t}\n \n-\t/* No template? No need to do anything. This is used by 'nat' table, it registers\n-\t * with the nat core instead of the netfilter core.\n-\t */\n-\tif (!template_ops)\n-\t\treturn 0;\n-\n-\tnum_ops = hweight32(table->valid_hooks);\n-\tif (num_ops == 0) {\n-\t\tret = -EINVAL;\n-\t\tgoto out_free;\n-\t}\n-\n-\tops = kmemdup_array(template_ops, num_ops, sizeof(*ops), GFP_KERNEL);\n-\tif (!ops) {\n-\t\tret = -ENOMEM;\n-\t\tgoto out_free;\n-\t}\n-\n-\tfor (i = 0; i < num_ops; i++)\n-\t\tops[i].priv = new_table;\n-\n-\tnew_table->ops = ops;\n-\n-\tret = nf_register_net_hooks(net, ops, num_ops);\n-\tif (ret != 0)\n-\t\tgoto out_free;\n-\n-\treturn ret;\n-\n-out_free:\n-\t__ipt_unregister_table(net, new_table);\n \treturn ret;\n }\n \ndiff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c\nindex d585ac3c1113..dbe7c7acd702 100644\n--- a/net/ipv6/netfilter/ip6_tables.c\n+++ b/net/ipv6/netfilter/ip6_tables.c\n@@ -1733,13 +1733,11 @@ int ip6t_register_table(struct net *net, const struct xt_table *table,\n \t\t\tconst struct ip6t_replace *repl,\n \t\t\tconst struct nf_hook_ops *template_ops)\n {\n-\tstruct nf_hook_ops *ops;\n-\tunsigned int num_ops;\n-\tint ret, i;\n-\tstruct xt_table_info *newinfo;\n \tstruct xt_table_info bootstrap = {0};\n-\tvoid *loc_cpu_entry;\n+\tstruct xt_table_info *newinfo;\n \tstruct xt_table *new_table;\n+\tvoid *loc_cpu_entry;\n+\tint ret;\n \n \tnewinfo = xt_alloc_table_info(repl->size);\n \tif (!newinfo)\n@@ -1754,7 +1752,7 @@ int ip6t_register_table(struct net *net, const struct xt_table *table,\n \t\treturn ret;\n \t}\n \n-\tnew_table = xt_register_table(net, table, &bootstrap, newinfo);\n+\tnew_table = xt_register_table(net, table, template_ops, &bootstrap, newinfo);\n \tif (IS_ERR(new_table)) {\n \t\tstruct ip6t_entry *iter;\n \n@@ -1764,34 +1762,6 @@ int ip6t_register_table(struct net *net, const struct xt_table *table,\n \t\treturn PTR_ERR(new_table);\n \t}\n \n-\tif (!template_ops)\n-\t\treturn 0;\n-\n-\tnum_ops = hweight32(table->valid_hooks);\n-\tif (num_ops == 0) {\n-\t\tret = -EINVAL;\n-\t\tgoto out_free;\n-\t}\n-\n-\tops = kmemdup_array(template_ops, num_ops, sizeof(*ops), GFP_KERNEL);\n-\tif (!ops) {\n-\t\tret = -ENOMEM;\n-\t\tgoto out_free;\n-\t}\n-\n-\tfor (i = 0; i < num_ops; i++)\n-\t\tops[i].priv = new_table;\n-\n-\tnew_table->ops = ops;\n-\n-\tret = nf_register_net_hooks(net, ops, num_ops);\n-\tif (ret != 0)\n-\t\tgoto out_free;\n-\n-\treturn ret;\n-\n-out_free:\n-\t__ip6t_unregister_table(net, new_table);\n \treturn ret;\n }\n \ndiff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c\nindex bb0cb3959551..06f27bea9eed 100644\n--- a/net/netfilter/x_tables.c\n+++ b/net/netfilter/x_tables.c\n@@ -1542,7 +1542,6 @@ xt_replace_table(struct xt_table *table, unsigned int num_counters,\n \tprivate = do_replace_table(table, num_counters, newinfo, error);\n \tif (private)\n \t\taudit_log_nfcfg(table->name, table->af, private->number,\n-\t\t\t\t!private->number ? AUDIT_XT_OP_REGISTER :\n \t\t\t\tAUDIT_XT_OP_REPLACE,\n \t\t\t\tGFP_KERNEL);\n \n@@ -1552,20 +1551,32 @@ EXPORT_SYMBOL_GPL(xt_replace_table);\n \n struct xt_table *xt_register_table(struct net *net,\n \t\t\t\t   const struct xt_table *input_table,\n+\t\t\t\t   const struct nf_hook_ops *template_ops,\n \t\t\t\t   struct xt_table_info *bootstrap,\n \t\t\t\t   struct xt_table_info *newinfo)\n {\n \tstruct xt_pernet *xt_net = net_generic(net, xt_pernet_id);\n+\tstruct xt_table *t, *table = NULL;\n+\tstruct nf_hook_ops *ops = NULL;\n \tstruct xt_table_info *private;\n-\tstruct xt_table *t, *table;\n-\tint ret;\n+\tunsigned int num_ops;\n+\tint ret = -EINVAL;\n+\n+\tnum_ops = hweight32(input_table->valid_hooks);\n+\tif (num_ops == 0)\n+\t\tgoto out;\n+\n+\tret = -ENOMEM;\n+\tif (template_ops) {\n+\t\tops = kmemdup_array(template_ops, num_ops, sizeof(*ops), GFP_KERNEL);\n+\t\tif (!ops)\n+\t\t\tgoto out;\n+\t}\n \n \t/* Don't add one object to multiple lists. */\n \ttable = kmemdup(input_table, sizeof(struct xt_table), GFP_KERNEL);\n-\tif (!table) {\n-\t\tret = -ENOMEM;\n+\tif (!table)\n \t\tgoto out;\n-\t}\n \n \tmutex_lock(&xt[table->af].mutex);\n \t/* Don't autoload: we'd eat our tail... */\n@@ -1579,7 +1590,7 @@ struct xt_table *xt_register_table(struct net *net,\n \t/* Simplifies replace_table code. */\n \ttable->private = bootstrap;\n \n-\tif (!xt_replace_table(table, 0, newinfo, &ret))\n+\tif (!do_replace_table(table, 0, newinfo, &ret))\n \t\tgoto unlock;\n \n \tprivate = table->private;\n@@ -1588,14 +1599,37 @@ struct xt_table *xt_register_table(struct net *net,\n \t/* save number of initial entries */\n \tprivate->initial_entries = private->number;\n \n+\tif (ops) {\n+\t\tint i;\n+\n+\t\tfor (i = 0; i < num_ops; i++)\n+\t\t\tops[i].priv = table;\n+\n+\t\tret = nf_register_net_hooks(net, ops, num_ops);\n+\t\tif (ret != 0) {\n+\t\t\tmutex_unlock(&xt[table->af].mutex);\n+\t\t\t/* nf_register_net_hooks() might have published a\n+\t\t\t * base chain before internal error unwind.\n+\t\t\t */\n+\t\t\tsynchronize_rcu();\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\ttable->ops = ops;\n+\t}\n+\n+\taudit_log_nfcfg(table->name, table->af, private->number,\n+\t\t\tAUDIT_XT_OP_REGISTER, GFP_KERNEL);\n+\n \tlist_add(&table->list, &xt_net->tables[table->af]);\n \tmutex_unlock(&xt[table->af].mutex);\n \treturn table;\n \n unlock:\n \tmutex_unlock(&xt[table->af].mutex);\n-\tkfree(table);\n out:\n+\tkfree(table);\n+\tkfree(ops);\n \treturn ERR_PTR(ret);\n }\n EXPORT_SYMBOL_GPL(xt_register_table);\n","prefixes":["v3","nf","2/8"]}