[PATCH] netfilter: nf_tables: fix pernet use-after-free on UNREGISTER rebroadcast

[Henry Martin <bsdhenrymartin@gmail.com>]

A device unregistered outside its netns teardown can be stuck in
netdev_wait_allrefs_any() while the pernet exit handlers of its
netns run to completion.  The nftables pernet storage is freed on
that path.  The rebroadcasts then hit the nf_tables netdev
notifiers (base chain, flowtable, offload), which read the freed
storage via nft_pernet() and take commit_mutex on it, e.g. on
mainline v7.4-rc1 (KASAN build):

    BUG: KASAN: slab-use-after-free in nf_tables_netdev_event+0x17c
    Read of size 8 at addr ffff88811061c980 by task ip/422
    Call Trace:
     nf_tables_netdev_event+0x17c/0x1c0
    Freed by task 62:
     kfree+0x161/0x450
     net_passive_dec+0xe5/0x180
     cleanup_net+0x45a/0x780

Skip the rebroadcasts in all three notifiers, which are the only
NETDEV_UNREGISTER events delivered with reg_state ==
NETREG_UNREGISTERED.  This mirrors commit fd0909e963ed8
("netdev-genl: ignore NETDEV_UNREGISTER rebroadcasts").

This issue was discovered by Tencent CodeBuddy Security.

Fixes: 8a4fc54b07d75 ("net: get rid of rtnl_lock_unregistering()")
Cc: stable@vger.kernel.org
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
 net/netfilter/nf_tables_api.c     | 9 +++++++++
 net/netfilter/nf_tables_offload.c | 8 ++++++++
 net/netfilter/nft_chain_filter.c  | 9 +++++++++
 3 files changed, 26 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 59ac623e6805f..e43a1b9fefc36 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9902,6 +9902,15 @@ static int nf_tables_flowtable_event(struct notifier_block *this,
 	    event != NETDEV_CHANGENAME)
 		return NOTIFY_DONE;
 
+	/* netdev_wait_allrefs_any() rebroadcasts NETDEV_UNREGISTER
+	 * while waiting for the last references to be dropped; the
+	 * netns and its nftables pernet storage may already be torn
+	 * down by then.
+	 */
+	if (event == NETDEV_UNREGISTER &&
+	    READ_ONCE(dev->reg_state) == NETREG_UNREGISTERED)
+		return NOTIFY_DONE;
+
 	net = dev_net(dev);
 	nft_net = nft_pernet(net);
 	mutex_lock(&nft_net->commit_mutex);
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 0ac3c26dfb3d1..d360e233d0366 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -672,6 +672,14 @@ static int nft_offload_netdev_event(struct notifier_block *this,
 	if (event != NETDEV_UNREGISTER)
 		return NOTIFY_DONE;
 
+	/* netdev_wait_allrefs_any() rebroadcasts NETDEV_UNREGISTER
+	 * while waiting for the last references to be dropped; the
+	 * netns and its nftables pernet storage may already be torn
+	 * down by then.
+	 */
+	if (READ_ONCE(dev->reg_state) == NETREG_UNREGISTERED)
+		return NOTIFY_DONE;
+
 	nft_net = nft_pernet(net);
 	mutex_lock(&nft_net->commit_mutex);
 	chain = __nft_offload_get_chain(nft_net, dev);
diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index e04e689f22284..b3cfe8d426753 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -412,6 +412,15 @@ static int nf_tables_netdev_event(struct notifier_block *this,
 	    event != NETDEV_CHANGENAME)
 		return NOTIFY_DONE;
 
+	/* netdev_wait_allrefs_any() rebroadcasts NETDEV_UNREGISTER
+	 * while waiting for the last references to be dropped; the
+	 * netns and its nftables pernet storage may already be torn
+	 * down by then.
+	 */
+	if (event == NETDEV_UNREGISTER &&
+	    READ_ONCE(dev->reg_state) == NETREG_UNREGISTERED)
+		return NOTIFY_DONE;
+
 	nft_net = nft_pernet(dev_net(dev));
 	mutex_lock(&nft_net->commit_mutex);
 
-- 
2.43.7