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

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>

1 files changed, 32 insertions, 4 deletions

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 1980e37b6472..65e9bc1058b5 100644
--- 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,
@@ -751,8 +766,8 @@ ethtool_get_drvinfo(struct net_device *dev, struct ethtool_devlink_compat *rsp)
 		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;
 }
 
@@ -2184,8 +2199,10 @@ static int ethtool_set_value(struct net_device *dev, char __user *useraddr,
 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);
 }
@@ -3027,7 +3044,16 @@ int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr)
 		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;
@@ -3036,6 +3062,8 @@ int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr)
 	}
 
 exit_free:
+	if (state->devlink)
+		devlink_put(state->devlink);
 	kfree(state);
 	return rc;
 }