Re: [PATCH net-next v3 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jiri Pirko <jiri@resnulli.us>
Date: 2019-02-24 11:10:11
Fri, Feb 22, 2019 at 11:07:56PM CET, jakub.kicinski@netronome.com wrote:
quoted hunk ↗ jump to hunk
When ethtool is calling into devlink compat code make sure we have a reference on the netdevice on which the operation was invoked. v3: move the hold/lock logic into devlink_compat_* functions (Florian) Signed-off-by: Jakub Kicinski <redacted> --- net/core/devlink.c | 34 +++++++++++++++++++++++----------- net/core/ethtool.c | 13 ++----------- 2 files changed, 25 insertions(+), 22 deletions(-)diff --git a/net/core/devlink.c b/net/core/devlink.c index a13055160be0..78c6ea1870ca 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c@@ -6430,27 +6430,39 @@ void devlink_compat_running_version(struct net_device *dev,{ struct devlink *devlink; + dev_hold(dev); + rtnl_unlock();
Ha, I got it now. You rely on dev_hold to make sure the devlink instance does not dissappear. But until this patch, that is not guaranteed (or I'm missing it).
quoted hunk ↗ jump to hunk
+ devlink = netdev_to_devlink(dev); - if (!devlink || !devlink->ops || !devlink->ops->info_get) - return; + if (devlink && devlink->ops && devlink->ops->info_get) { + mutex_lock(&devlink->lock); + __devlink_compat_running_version(devlink, buf, len); + mutex_unlock(&devlink->lock); + } - mutex_lock(&devlink->lock); - __devlink_compat_running_version(devlink, buf, len); - mutex_unlock(&devlink->lock); + rtnl_lock(); + dev_put(dev); } int devlink_compat_flash_update(struct net_device *dev, const char *file_name) { struct devlink *devlink; - int ret; + int ret = -EOPNOTSUPP; + + dev_hold(dev); + rtnl_unlock(); devlink = netdev_to_devlink(dev); - if (!devlink || !devlink->ops || !devlink->ops->flash_update) - return -EOPNOTSUPP; + if (devlink && devlink->ops && devlink->ops->flash_update) { + mutex_lock(&devlink->lock); + ret = devlink->ops->flash_update(devlink, file_name, + NULL, NULL); + mutex_unlock(&devlink->lock); + } + + rtnl_lock(); + dev_put(dev); - mutex_lock(&devlink->lock); - ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL); - mutex_unlock(&devlink->lock); return ret; }diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 1320e8dce559..47558ffae423 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c@@ -805,11 +805,9 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,if (ops->get_eeprom_len) info.eedump_len = ops->get_eeprom_len(dev); - rtnl_unlock(); if (!info.fw_version[0]) devlink_compat_running_version(dev, info.fw_version, sizeof(info.fw_version)); - rtnl_lock(); if (copy_to_user(useraddr, &info, sizeof(info))) return -EFAULT;@@ -2040,15 +2038,8 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,return -EFAULT; efl.data[ETHTOOL_FLASH_MAX_FILENAME - 1] = 0; - if (!dev->ethtool_ops->flash_device) { - int ret; - - rtnl_unlock(); - ret = devlink_compat_flash_update(dev, efl.data); - rtnl_lock(); - - return ret; - } + if (!dev->ethtool_ops->flash_device) + return devlink_compat_flash_update(dev, efl.data); return dev->ethtool_ops->flash_device(dev, &efl); } -- 2.19.2