From: Jakub Kicinski <kuba@kernel.org>

Date: Sat, 30 Oct 2021 10:18:51 -0700

Subject: ethtool: don't drop the rtnl_lock half way thru the ioctl

Patch-mainline: v5.16-rc1

Git-commit: 1af0a0948e28d83bcfa9d48cd0f992f616c5d62e

References: jsc#PED-1495

devlink compat code needs to drop rtnl_lock to take

devlink->lock to ensure correct lock ordering.

This is problematic because we're not strictly guaranteed

that the netdev will not disappear after we re-lock.

It may open a possibility of nested ->begin / ->complete

calls.

Instead of calling into devlink under rtnl_lock take

a ref on the devlink instance and make the call after

we've dropped rtnl_lock.

We (continue to) assume that netdevs have an implicit

reference on the devlink returned from ndo_get_devlink_port

Note that ndo_get_devlink_port will now get called

under rtnl_lock. That should be fine since none of

the drivers seem to be taking serious locks inside

ndo_get_devlink_port.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Signed-off-by: David S. Miller <davem@davemloft.net>

Acked-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>

---

 include/net/devlink.h |    8 ++++----

 net/core/devlink.c    |   45 +++++++--------------------------------------

 net/ethtool/ioctl.c   |   36 ++++++++++++++++++++++++++++++++----

 3 files changed, 43 insertions(+), 46 deletions(-)

--- a/include/net/devlink.h

+++ b/include/net/devlink.h

@@ -1729,9 +1729,9 @@ devlink_trap_policers_unregister(struct

 struct devlink *__must_check devlink_try_get(struct devlink *devlink);

 void devlink_put(struct devlink *devlink);

-void devlink_compat_running_version(struct net_device *dev,

+void devlink_compat_running_version(struct devlink *devlink,

 				    char *buf, size_t len);

-int devlink_compat_flash_update(struct net_device *dev, const char *file_name);

+int devlink_compat_flash_update(struct devlink *devlink, const char *file_name);

 int devlink_compat_phys_port_name_get(struct net_device *dev,

 				      char *name, size_t len);

 int devlink_compat_switch_id_get(struct net_device *dev,

@@ -1749,12 +1749,12 @@ static inline void devlink_put(struct de

 }

 static inline void

-devlink_compat_running_version(struct net_device *dev, char *buf, size_t len)

+devlink_compat_running_version(struct devlink *devlink, char *buf, size_t len)

 {

 }

 static inline int

-devlink_compat_flash_update(struct net_device *dev, const char *file_name)

+devlink_compat_flash_update(struct devlink *devlink, const char *file_name)

 {

 	return -EOPNOTSUPP;

 }

--- a/net/core/devlink.c

+++ b/net/core/devlink.c

@@ -11283,55 +11283,28 @@ static struct devlink_port *netdev_to_de

 	return dev->netdev_ops->ndo_get_devlink_port(dev);

 }

-static struct devlink *netdev_to_devlink(struct net_device *dev)

-{

-	struct devlink_port *devlink_port = netdev_to_devlink_port(dev);

-

-	if (!devlink_port)

-		return NULL;

-

-	return devlink_port->devlink;

-}

-

-void devlink_compat_running_version(struct net_device *dev,

+void devlink_compat_running_version(struct devlink *devlink,

 				    char *buf, size_t len)

 {

-	struct devlink *devlink;

-

-	dev_hold(dev);

-	rtnl_unlock();

-

-	devlink = netdev_to_devlink(dev);

-	if (!devlink || !devlink->ops->info_get)

-		goto out;

+	if (!devlink->ops->info_get)

+		return;

 	mutex_lock(&devlink->lock);

 	__devlink_compat_running_version(devlink, buf, len);

 	mutex_unlock(&devlink->lock);

-

-out:

-	rtnl_lock();

-	dev_put(dev);

 }

-int devlink_compat_flash_update(struct net_device *dev, const char *file_name)

+int devlink_compat_flash_update(struct devlink *devlink, const char *file_name)

 {

 	struct devlink_flash_update_params params = {};

-	struct devlink *devlink;

 	int ret;

-	dev_hold(dev);

-	rtnl_unlock();

-

-	devlink = netdev_to_devlink(dev);

-	if (!devlink || !devlink->ops->flash_update) {

-		ret = -EOPNOTSUPP;

-		goto out;

-	}

+	if (!devlink->ops->flash_update)

+		return -EOPNOTSUPP;

 	ret = request_firmware(&params.fw, file_name, devlink->dev);

 	if (ret)

-		goto out;

+		return ret;

 	mutex_lock(&devlink->lock);

 	devlink_flash_update_begin_notify(devlink);

@@ -11341,10 +11314,6 @@ int devlink_compat_flash_update(struct n

 	release_firmware(params.fw);

-out:

-	rtnl_lock();

-	dev_put(dev);

-

 	return ret;

 }

--- a/net/ethtool/ioctl.c

+++ b/net/ethtool/ioctl.c

@@ -34,12 +34,27 @@

 /* State held across locks and calls for commands which have devlink fallback */

 struct ethtool_devlink_compat {

+	struct devlink *devlink;

 	union {

 		struct ethtool_flash efl;

 		struct ethtool_drvinfo info;

 	};

 };

+static struct devlink *netdev_to_devlink_get(struct net_device *dev)

+{

+	struct devlink_port *devlink_port;

+

+	if (!dev->netdev_ops->ndo_get_devlink_port)

+		return NULL;

+

+	devlink_port = dev->netdev_ops->ndo_get_devlink_port(dev);

+	if (!devlink_port)

+		return NULL;

+

+	return devlink_try_get(devlink_port->devlink);

+}

+

 /*

  * Some useful ethtool_ops methods that're device independent.

  * If we find that all drivers want to do the same thing here,

@@ -752,8 +767,8 @@ ethtool_get_drvinfo(struct net_device *d

 		rsp->info.eedump_len = ops->get_eeprom_len(dev);

 	if (!rsp->info.fw_version[0])

-		devlink_compat_running_version(dev, rsp->info.fw_version,

-					       sizeof(rsp->info.fw_version));

+		rsp->devlink = netdev_to_devlink_get(dev);

+

 	return 0;

 }

@@ -2185,8 +2200,10 @@ static int ethtool_set_value(struct net_

 static int

 ethtool_flash_device(struct net_device *dev, struct ethtool_devlink_compat *req)

 {

-	if (!dev->ethtool_ops->flash_device)

-		return devlink_compat_flash_update(dev, req->efl.data);

+	if (!dev->ethtool_ops->flash_device) {

+		req->devlink = netdev_to_devlink_get(dev);

+		return 0;

+	}

 	return dev->ethtool_ops->flash_device(dev, &req->efl);

 }

@@ -3028,7 +3045,16 @@ int dev_ethtool(struct net *net, struct

 		goto exit_free;

 	switch (ethcmd) {

+	case ETHTOOL_FLASHDEV:

+		if (state->devlink)

+			rc = devlink_compat_flash_update(state->devlink,

+							 state->efl.data);

+		break;

 	case ETHTOOL_GDRVINFO:

+		if (state->devlink)

+			devlink_compat_running_version(state->devlink,

+						       state->info.fw_version,

+						       sizeof(state->info.fw_version));

 		if (copy_to_user(useraddr, &state->info, sizeof(state->info))) {

 			rc = -EFAULT;

 			goto exit_free;

@@ -3037,6 +3063,8 @@ int dev_ethtool(struct net *net, struct

 	}

 exit_free:

+	if (state->devlink)

+		devlink_put(state->devlink);

 	kfree(state);

 	return rc;

 }