Commit 1af0a094 authored by Jakub Kicinski's avatar Jakub Kicinski Committed by David S. Miller
Browse files

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: default avatarJakub Kicinski <kuba@kernel.org>
Reviewed-by: default avatarLeon Romanovsky <leonro@nvidia.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 46db1b77

include/net/devlink.h

+4 −4
Original line number Diff line number Diff line
@@ -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;

}

net/core/devlink.c

+7 −38
Original line number Diff line number Diff line
@@ -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(&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 net_device *dev, const char *file_name) 


	release_firmware(params.fw);



out:

	rtnl_lock();

	dev_put(dev);



	return ret;

}

net/ethtool/ioctl.c

+32 −4
Original line number Diff line number Diff line
@@ -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;

}