From 1af0a0948e28d83bcfa9d48cd0f992f616c5d62e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Sat, 30 Oct 2021 10:18:51 -0700 Subject: 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 Reviewed-by: Leon Romanovsky Signed-off-by: David S. Miller --- net/ethtool/ioctl.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) (limited to 'net/ethtool') 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; } -- cgit