| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 30 Oct 2021 10:18:51 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: davem@...emloft.net
Cc: netdev@...r.kernel.org, jiri@...nulli.us, leon@...nel.org,
mkubecek@...e.cz, andrew@...n.ch, f.fainelli@...il.com,
Jakub Kicinski <kuba@...nel.org>
Subject: [PATCH net-next v2 4/4] 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@...nel.org>
---
include/net/devlink.h | 8 ++++----
net/core/devlink.c | 45 +++++++------------------------------------
net/ethtool/ioctl.c | 36 ++++++++++++++++++++++++++++++----
3 files changed, 43 insertions(+), 46 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 991ce48f77ca..aab3d007c577 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1729,9 +1729,9 @@ devlink_trap_policers_unregister(struct devlink *devlink,
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 devlink *devlink)
}
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;
}
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 100d87fd3f65..6b5ee862429e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -11283,55 +11283,28 @@ static struct devlink_port *netdev_to_devlink_port(struct net_device *dev)
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(¶ms.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 net_device *dev, const char *file_name)
release_firmware(params.fw);
-out:
- rtnl_lock();
- dev_put(dev);
-
return ret;
}
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;
}
--
2.31.1
Powered by blists - more mailing lists