Commit fcc017e3 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-deduplicate-netdev-name-allocation' 

Jakub Kicinski says:

====================
net: deduplicate netdev name allocation

After recent fixes we have even more duplicated code in netdev name
allocation helpers. There are two complications in this code.
First, __dev_alloc_name() clobbers its output arg even if allocation
fails, forcing callers to do extra copies. Second as our experience in
commit 55a5ec9b ("Revert "net: core: dev_get_valid_name is now the same as dev_alloc_name_ns"") and
commit 029b6d14 ("Revert "net: core: maybe return -EEXIST in __dev_alloc_name"")
taught us, user space is very sensitive to the exact error codes.

Align the callers of __dev_alloc_name(), and remove some of its
complexity.

v1: https://lore.kernel.org/all/20231020011856.3244410-1-kuba@kernel.org/
====================

Link: https://lore.kernel.org/r/20231023152346.3639749-1-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents fb1c535b ce4cfa23

net/core/dev.c

+45 −75
Original line number Diff line number Diff line
@@ -1057,7 +1057,7 @@ EXPORT_SYMBOL(dev_valid_name); 
 *	__dev_alloc_name - allocate a name for a device

 *	@net: network namespace to allocate the device name in

 *	@name: name format string

 *	@buf:  scratch buffer and result name string

 *	@res: result name string

 *

 *	Passed a format string - eg "lt%d" it will try and find a suitable

 *	id. It scans list of devices to build up a free map, then chooses
@@ -1068,25 +1068,20 @@ EXPORT_SYMBOL(dev_valid_name); 
 *	Returns the number of the unit assigned or a negative errno code.

 */



static int __dev_alloc_name(struct net *net, const char *name, char *buf)

static int __dev_alloc_name(struct net *net, const char *name, char *res)

{

	int i = 0;

	const char *p;

	const int max_netdevices = 8*PAGE_SIZE;

	unsigned long *inuse;

	struct net_device *d;

	char buf[IFNAMSIZ];



	if (!dev_valid_name(name))

		return -EINVAL;



	p = strchr(name, '%');

	if (p) {

		/*

		 * Verify the string as this thing may have come from

		 * the user.  There must be either one "%d" and no other "%"

		 * characters.

	/* Verify the string as this thing may have come from the user.

	 * There must be one "%d" and no other "%" characters.

	 */

		if (p[1] != 'd' || strchr(p + 2, '%'))

	p = strchr(name, '%');

	if (!p || p[1] != 'd' || strchr(p + 2, '%'))

		return -EINVAL;



	/* Use one page as a bit array of possible slots */
@@ -1121,53 +1116,31 @@ static int __dev_alloc_name(struct net *net, const char *name, char *buf) 


	i = find_first_zero_bit(inuse, max_netdevices);

	bitmap_free(inuse);

	}

	if (i == max_netdevices)

		return -ENFILE;



	snprintf(buf, IFNAMSIZ, name, i);

	if (!netdev_name_in_use(net, buf))

	snprintf(res, IFNAMSIZ, name, i);

	return i;



	/* It is possible to run out of possible slots

	 * when the name is long and there isn't enough space left

	 * for the digits, or if all bits are used.

	 */

	return -ENFILE;

}



/* Returns negative errno or allocated unit id (see __dev_alloc_name()) */

static int dev_prep_valid_name(struct net *net, struct net_device *dev,

			       const char *want_name, char *out_name)

			       const char *want_name, char *out_name,

			       int dup_errno)

{

	int ret;



	if (!dev_valid_name(want_name))

		return -EINVAL;



	if (strchr(want_name, '%')) {

		ret = __dev_alloc_name(net, want_name, out_name);

		return ret < 0 ? ret : 0;

	} else if (netdev_name_in_use(net, want_name)) {

		return -EEXIST;

	} else if (out_name != want_name) {

		strscpy(out_name, want_name, IFNAMSIZ);

	}

	if (strchr(want_name, '%'))

		return __dev_alloc_name(net, want_name, out_name);



	if (netdev_name_in_use(net, want_name))

		return -dup_errno;

	if (out_name != want_name)

		strscpy(out_name, want_name, IFNAMSIZ);

	return 0;

}



static int dev_alloc_name_ns(struct net *net,

			     struct net_device *dev,

			     const char *name)

{

	char buf[IFNAMSIZ];

	int ret;



	BUG_ON(!net);

	ret = __dev_alloc_name(net, name, buf);

	if (ret >= 0)

		strscpy(dev->name, buf, IFNAMSIZ);

	return ret;

}



/**

 *	dev_alloc_name - allocate a name for a device

 *	@dev: device
@@ -1184,20 +1157,17 @@ static int dev_alloc_name_ns(struct net *net, 


int dev_alloc_name(struct net_device *dev, const char *name)

{

	return dev_alloc_name_ns(dev_net(dev), dev, name);

	return dev_prep_valid_name(dev_net(dev), dev, name, dev->name, ENFILE);

}

EXPORT_SYMBOL(dev_alloc_name);



static int dev_get_valid_name(struct net *net, struct net_device *dev,

			      const char *name)

{

	char buf[IFNAMSIZ];

	int ret;



	ret = dev_prep_valid_name(net, dev, name, buf);

	if (ret >= 0)

		strscpy(dev->name, buf, IFNAMSIZ);

	return ret;

	ret = dev_prep_valid_name(net, dev, name, dev->name, EEXIST);

	return ret < 0 ? ret : 0;

}



/**
@@ -11135,7 +11105,7 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net, 
		/* We get here if we can't use the current device name */

		if (!pat)

			goto out;

		err = dev_prep_valid_name(net, dev, pat, new_name);

		err = dev_prep_valid_name(net, dev, pat, new_name, EEXIST);

		if (err < 0)

			goto out;

	}