[PATCH net] net: avoid theoretical races with ref drain

Jakub Kicinski <[email protected]>
Newsgroups org.kernel.vger.netdev
Message-ID <[email protected]>
Technically, it's illegal to take a ref on a netdev just because
we have a pointer on which we already hold a ref, with no other
protection. This is because our simple per-cpu refcount
implementation cannot atomically read the count.

Let's make sure we cancel outstanding work and never queue more
work for a device we know is dead. This way taking a ref on
a dev we know is on the netdev_work_list is always going to be safe.

Jiangshan Yi reports that the issues is caught by ref tracker infra
leading to a warning:
  WARNING: lib/ref_tracker.c:322 at ref_tracker_free
  WARNING: lib/ref_tracker.c:246 at ref_tracker_dir_exit

Reported-by: Jiangshan Yi <[email protected]>
Link: https://lore.kernel.org/[email protected]
Fixes: 12c765be84d2 ("net: turn the rx_mode work into a generic netdev_work facility")
Signed-off-by: Jakub Kicinski <[email protected]>
---
 net/core/dev.h         |  1 +
 net/core/dev.c         |  1 +
 net/core/netdev_work.c | 16 ++++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/net/core/dev.h b/net/core/dev.h
index 5d0b0305d3ba..b757faead4d1 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -179,6 +179,7 @@ enum netdev_work_core {
 void __netdev_work_core_sched(struct net_device *dev, unsigned long event);
 unsigned long
 __netdev_work_core_cancel(struct net_device *dev, unsigned long mask);
+void netdev_work_cancel_all(struct net_device *dev);
 
 void __dev_notify_flags(struct net_device *dev, unsigned int old_flags,
 			unsigned int gchanges, u32 portid,
diff --git a/net/core/dev.c b/net/core/dev.c
index e50ed677de72..fca25797eeec 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -12478,6 +12478,7 @@ void unregister_netdevice_many_notify(struct list_head *head,
 		dev_tcx_uninstall(dev);
 		dev_xdp_uninstall(dev);
 		dev_memory_provider_uninstall(dev);
+		netdev_work_cancel_all(dev);
 		netdev_unlock_ops(dev);
 		bpf_dev_bound_netdev_unregister(dev);
 
diff --git a/net/core/netdev_work.c b/net/core/netdev_work.c
index 3109fae132ad..e721a06d58df 100644
--- a/net/core/netdev_work.c
+++ b/net/core/netdev_work.c
@@ -31,6 +31,10 @@ static void netdev_work_enqueue(struct net_device *dev, unsigned long events,
 		return;
 
 	spin_lock_bh(&netdev_work_lock);
+	if (!dev_isalive(dev)) {
+		spin_unlock_bh(&netdev_work_lock);
+		return;
+	}
 	if (list_empty(&dev->work_node)) {
 		list_add_tail(&dev->work_node, &netdev_work_list);
 		netdev_hold(dev, &dev->work_tracker, GFP_ATOMIC);
@@ -61,6 +65,18 @@ netdev_work_dequeue(struct net_device *dev, unsigned long *pending,
 	return events;
 }
 
+void netdev_work_cancel_all(struct net_device *dev)
+{
+	spin_lock_bh(&netdev_work_lock);
+	dev->work_pending = 0;
+	dev->work_core_pending = 0;
+	if (!list_empty(&dev->work_node)) {
+		list_del_init(&dev->work_node);
+		netdev_put(dev, &dev->work_tracker);
+	}
+	spin_unlock_bh(&netdev_work_lock);
+}
+
 void netdev_work_sched(struct net_device *dev, unsigned long events)
 {
 	netdev_work_enqueue(dev, events, 0);
-- 
2.55.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.