[PATCH net-next v2 09/11] leds: trigger: netdev: don't recurse on the netdev ops lock

[Jakub Kicinski <kuba@kernel.org>]

get_device_state() calls __ethtool_get_link_ksettings() on the trigger's
netdev, which will soon take the dev's ops lock. Three of its callers
already hold that lock and one doesn't, so the function would either
deadlock or run unprotected depending on the path.

Make get_device_state() expect the dev's ops lock held and switch to
netif_get_link_ksettings():

  * netdev_trig_notify() NETDEV_UP / NETDEV_CHANGE / NETDEV_CHANGENAME
    arrive with the dev's ops lock held (per netdevices.rst).
  * set_device_name() does not hold the lock, take it explicitly.

Due to lock ordering we need to reshuffle the code in set_device_name()
a little bit. We need to find the device earlier on, so that we can
lock it before we take trigger_data->lock.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/leds/trigger/ledtrig-netdev.c | 37 +++++++++++++++------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 12cb3311ea22..64c078e997f2 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -26,6 +26,7 @@
 #include <linux/phy.h>
 #include <linux/rtnetlink.h>
 #include <linux/timer.h>
+#include <net/netdev_lock.h>
 #include "../leds.h"
 
 #define NETDEV_LED_DEFAULT_INTERVAL	50
@@ -228,7 +229,7 @@ static void get_device_state(struct led_netdev_data *trigger_data)
 
 	trigger_data->carrier_link_up = netif_carrier_ok(trigger_data->net_dev);
 
-	if (__ethtool_get_link_ksettings(trigger_data->net_dev, &cmd))
+	if (netif_get_link_ksettings(trigger_data->net_dev, &cmd))
 		return;
 
 	if (trigger_data->carrier_link_up) {
@@ -259,31 +260,33 @@ static ssize_t device_name_show(struct device *dev,
 static int set_device_name(struct led_netdev_data *trigger_data,
 			   const char *name, size_t size)
 {
+	struct net_device *new_dev = NULL;
+	char device_name[IFNAMSIZ];
+
 	if (size >= IFNAMSIZ)
 		return -EINVAL;
 
 	cancel_delayed_work_sync(&trigger_data->work);
 
+	memcpy(device_name, name, size);
+	device_name[size] = 0;
+	if (size > 0 && device_name[size - 1] == '\n')
+		device_name[size - 1] = 0;
+
 	/*
-	 * Take RTNL lock before trigger_data lock to prevent potential
-	 * deadlock with netdev notifier registration.
+	 * Lock order: rtnl_lock -> netdev instance lock -> trigger_data lock.
 	 */
 	rtnl_lock();
+	if (device_name[0]) {
+		new_dev = dev_get_by_name(&init_net, device_name);
+		if (new_dev)
+			netdev_lock_ops(new_dev);
+	}
 	mutex_lock(&trigger_data->lock);
 
-	if (trigger_data->net_dev) {
-		dev_put(trigger_data->net_dev);
-		trigger_data->net_dev = NULL;
-	}
-
-	memcpy(trigger_data->device_name, name, size);
-	trigger_data->device_name[size] = 0;
-	if (size > 0 && trigger_data->device_name[size - 1] == '\n')
-		trigger_data->device_name[size - 1] = 0;
-
-	if (trigger_data->device_name[0] != 0)
-		trigger_data->net_dev =
-		    dev_get_by_name(&init_net, trigger_data->device_name);
+	dev_put(trigger_data->net_dev);
+	trigger_data->net_dev = new_dev;
+	strscpy(trigger_data->device_name, device_name);
 
 	trigger_data->carrier_link_up = false;
 	trigger_data->link_speed = SPEED_UNKNOWN;
@@ -298,6 +301,8 @@ static int set_device_name(struct led_netdev_data *trigger_data,
 		set_baseline_state(trigger_data);
 
 	mutex_unlock(&trigger_data->lock);
+	if (new_dev)
+		netdev_unlock_ops(new_dev);
 	rtnl_unlock();
 
 	return 0;
-- 
2.54.0