diff mbox series

[net-next,v2,05/10] wwan: core: remove all netdevs on ops unregistering

Message ID 20210621225100.21005-6-ryazanov.s.a@gmail.com
State New
Headers show
Series net: WWAN link creation improvements | expand

Commit Message

Sergey Ryazanov June 21, 2021, 10:50 p.m. UTC
We use the ops owner module hold to protect against ops memory
disappearing. But this approach does not protect us from a driver that
unregisters ops but forgets to remove netdev(s) that were created using
this ops. In such case, we are left with netdev(s), which can not be
removed since ops is gone. Moreover, batch netdevs removing on
deinitialization is a desireable option for WWAN drivers as it is a
quite common task.

Implement deletion of all created links on WWAN netdev ops unregistering
in the same way that RTNL removes all links on RTNL ops unregistering.
Simply remove all child netdevs of a device whose WWAN netdev ops is
unregistering. This way we protecting the kernel from buggy drivers and
make it easier to write a driver deinitialization code.

Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---

v1 -> v2:
 * no changes

 drivers/net/wwan/wwan_core.c | 40 ++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 9 deletions(-)

Comments

Loic Poulain June 22, 2021, 1:50 p.m. UTC | #1
On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>

> We use the ops owner module hold to protect against ops memory

> disappearing. But this approach does not protect us from a driver that

> unregisters ops but forgets to remove netdev(s) that were created using

> this ops. In such case, we are left with netdev(s), which can not be

> removed since ops is gone. Moreover, batch netdevs removing on

> deinitialization is a desireable option for WWAN drivers as it is a

> quite common task.

>

> Implement deletion of all created links on WWAN netdev ops unregistering

> in the same way that RTNL removes all links on RTNL ops unregistering.

> Simply remove all child netdevs of a device whose WWAN netdev ops is

> unregistering. This way we protecting the kernel from buggy drivers and

> make it easier to write a driver deinitialization code.

>

> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>


Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
diff mbox series

Patch

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index b6b9c52f617c..ec6a69b23dd1 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -941,6 +941,17 @@  int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
 }
 EXPORT_SYMBOL_GPL(wwan_register_ops);
 
+/* Enqueue child netdev deletion */
+static int wwan_child_dellink(struct device *dev, void *data)
+{
+	struct list_head *kill_list = data;
+
+	if (dev->type == &wwan_type)
+		wwan_rtnl_dellink(to_net_dev(dev), kill_list);
+
+	return 0;
+}
+
 /**
  * wwan_unregister_ops - remove WWAN device ops
  * @parent: Device to use as parent and shared by all WWAN ports and
@@ -949,26 +960,37 @@  EXPORT_SYMBOL_GPL(wwan_register_ops);
 void wwan_unregister_ops(struct device *parent)
 {
 	struct wwan_device *wwandev = wwan_dev_get_by_parent(parent);
-	bool has_ops;
+	struct module *owner;
+	LIST_HEAD(kill_list);
 
 	if (WARN_ON(IS_ERR(wwandev)))
 		return;
-
-	has_ops = wwandev->ops;
+	if (WARN_ON(!wwandev->ops)) {
+		put_device(&wwandev->dev);
+		return;
+	}
 
 	/* put the reference obtained by wwan_dev_get_by_parent(),
 	 * we should still have one (that the owner is giving back
-	 * now) due to the ops being assigned, check that below
-	 * and return if not.
+	 * now) due to the ops being assigned.
 	 */
 	put_device(&wwandev->dev);
 
-	if (WARN_ON(!has_ops))
-		return;
+	owner = wwandev->ops->owner;	/* Preserve ops owner */
+
+	rtnl_lock();	/* Prevent concurent netdev(s) creation/destroying */
+
+	/* Remove all child netdev(s), using batch removing */
+	device_for_each_child(&wwandev->dev, &kill_list,
+			      wwan_child_dellink);
+	unregister_netdevice_many(&kill_list);
+
+	wwandev->ops = NULL;	/* Finally remove ops */
+
+	rtnl_unlock();
 
-	module_put(wwandev->ops->owner);
+	module_put(owner);
 
-	wwandev->ops = NULL;
 	wwandev->ops_ctxt = NULL;
 	wwan_remove_dev(wwandev);
 }