Skip to contentSkip to conversation

sockopt: Make SO_BINDTODEVICE readable

PATCH Applied Pavel Emelyanov 5 participants 15 messages Oct 19, 2012 – Oct 23, 2012

Subsystems (3)
Networking Network drivers Sockets
lore
Share & tools
lore

Pavel Emelyanov

author
#0 lore raw

PATCH 1/1 sockopt: Make SO_BINDTODEVICE readable

The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but cannot be get via sockopt API. The only way we can find the device id a socket is bound to is via sock-diag interface. But the diag works only on hashed sockets, while the opt in question can be set for yet unhashed one.

That said, in order to know what device a socket is bound to (we do want to know this in checkpoint-restore project) I propose to make this option getsockopt-able and report the respective device index.

Another solution to the problem might be to teach the sock-diag reporting info on unhashed sockets. Should I go this way instead?

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/core/sock.c +3 -0
index 8a146cf..c49412c 100644
@@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 	case SO_NOFCS:
 		v.val = sock_flag(sk, SOCK_NOFCS);
 		break;
+	case SO_BINDTODEVICE:
+		v.val = sk->sk_bound_dev_if;
+		break;
 	default:
 		return -ENOPROTOOPT;
 	}
Acked-by on v1 1/1
#1 lore raw
Quoted 21 lines · Pavel Emelyanov

The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but cannot be get via sockopt API. The only way we can find the device id a socket is bound to is via sock-diag interface. But the diag works only on hashed sockets, while the opt in question can be set for yet unhashed one.

That said, in order to know what device a socket is bound to (we do want to know this in checkpoint-restore project) I propose to make this option getsockopt-able and report the respective device index.

Another solution to the problem might be to teach the sock-diag reporting info on unhashed sockets. Should I go this way instead?

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/core/sock.c +0 -0
index 8a146cf..c49412c 100644

net/core/sock.c int sock_getsockopt(struct socket *sock, int level, int optname, · line 1082

Quoted 9 lines
net/core/sock.c +3 -0
@@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 	case SO_NOFCS:
 		v.val = sock_flag(sk, SOCK_NOFCS);
 		break;
+	case SO_BINDTODEVICE:
+		v.val = sk->sk_bound_dev_if;
+		break;
 	default:
 		return -ENOPROTOOPT;
 	}

This looks quite reasonable to me.

Acked-by Eric Dumazet <[email protected]>
maintainer Applied
#2 lore raw
In reply to Pavel Emelyanov

From: Pavel Emelyanov <[email protected]>
Date: Fri, 19 Oct 2012 13:55:56 +0400

Quoted 5 lines

The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but cannot be get via sockopt API. The only way we can find the device id a socket is bound to is via sock-diag interface. But the diag works only on hashed sockets, while the opt in question can be set for yet unhashed one.

Quoted 8 lines

That said, in order to know what device a socket is bound to (we do want to know this in checkpoint-restore project) I propose to make this option getsockopt-able and report the respective device index.

Another solution to the problem might be to teach the sock-diag reporting info on unhashed sockets. Should I go this way instead?

Signed-off-by Pavel Emelyanov <[email protected]>

Applied. I can't believe we didn't support this when the feature was added.

I guess we figured that if the application made this setting, then it knows and doesn't need to query it.

author
#3 lore raw
Quoted 15 lines · David Miller

From: Pavel Emelyanov <[email protected]>
Date: Fri, 19 Oct 2012 13:55:56 +0400

Quoted 12 lines

The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but cannot be get via sockopt API. The only way we can find the device id a socket is bound to is via sock-diag interface. But the diag works only on hashed sockets, while the opt in question can be set for yet unhashed one.

That said, in order to know what device a socket is bound to (we do want to know this in checkpoint-restore project) I propose to make this option getsockopt-able and report the respective device index.

Another solution to the problem might be to teach the sock-diag reporting info on unhashed sockets. Should I go this way instead?

Quoted 8 lines
Quoted 1 line
Signed-off-by Pavel Emelyanov <[email protected]>

Applied. I can't believe we didn't support this when the feature was added.

I guess we figured that if the application made this setting, then it knows and doesn't need to query it.
.

AFAIS such decision (application that configures something knows what it is) was made for many things. The same is true at least for the SO_ATTACH_FILTER option and for the shutdown syscall. Both add something to a socket that cannot be queried back. And I'm now thinking which way for getting one would be better.

Thanks,
Pavel

#4 lore raw
In reply to Pavel Emelyanov
Quoted 21 lines · Pavel Emelyanov

The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but cannot be get via sockopt API. The only way we can find the device id a socket is bound to is via sock-diag interface. But the diag works only on hashed sockets, while the opt in question can be set for yet unhashed one.

That said, in order to know what device a socket is bound to (we do want to know this in checkpoint-restore project) I propose to make this option getsockopt-able and report the respective device index.

Another solution to the problem might be to teach the sock-diag reporting info on unhashed sockets. Should I go this way instead?

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/core/sock.c +0 -0
index 8a146cf..c49412c 100644

net/core/sock.c int sock_getsockopt(struct socket *sock, int level, int optname, · line 1082

Quoted 9 lines
net/core/sock.c +3 -0
@@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 	case SO_NOFCS:
 		v.val = sock_flag(sk, SOCK_NOFCS);
 		break;
+	case SO_BINDTODEVICE:
+		v.val = sk->sk_bound_dev_if;
+		break;
 	default:
 		return -ENOPROTOOPT;
 	}

Doesn't this make the set and get non-symmetrical? For example, setsockopt() would take "eth0", but getsockopt() would return 2. The following patch would return a string, or -ENODEV if not set.

-Brian

---

Change getsockopt(SO_BINDTODEVICE) to be symmetrical with setsockopt() by returning the interface name as a string.

Signed-off-by Brian Haley <[email protected]>
net/core/sock.c +46 -2
index c49412c..69b9d92 100644
@@ -505,7 +505,8 @@ struct dst_entry *sk_dst_check(struct sock *sk, u32 cookie)
 }
 EXPORT_SYMBOL(sk_dst_check);
 
-static int sock_bindtodevice(struct sock *sk, char __user *optval, int optlen)
+static int sock_setbindtodevice(struct sock *sk, char __user *optval,
+				int optlen)
 {
 	int ret = -ENOPROTOOPT;
 #ifdef CONFIG_NETDEVICES

@@ -562,6 +563,49 @@ out:
 	return ret;
 }
 
+static int sock_getbindtodevice(struct sock *sk, char __user *optval,
+				int __user *optlen, int len)
+{
+	int ret = -ENOPROTOOPT;
+#ifdef CONFIG_NETDEVICES
+	struct net *net = sock_net(sk);
+	struct net_device *dev;
+	char devname[IFNAMSIZ];
+
+	ret = 0;
+	if (sk->sk_bound_dev_if == 0)
+		goto out;
+
+	ret = -EINVAL;
+	if (len < IFNAMSIZ)
+		goto out;
+	if (len > IFNAMSIZ)
+		len = IFNAMSIZ;
+
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
+	if (dev)
+		strcpy(dev->name, devname);
+	rcu_read_unlock();
+	ret = -ENODEV;
+	if (!dev)
+		goto out;
+
+	ret = -EFAULT;
+	if (copy_to_user(optval, devname, len))
+		goto out;
+
+	if (put_user(len, optlen))
+		goto out;
+
+	ret = 0;
+
+out:
+#endif
+
+	return ret;
+}
+
 static inline void sock_valbool_flag(struct sock *sk, int bit, int valbool)
 {
 	if (valbool)

@@ -589,7 +633,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 	 */
 
 	if (optname == SO_BINDTODEVICE)
-		return sock_bindtodevice(sk, optval, optlen);
+		return sock_setbindtodevice(sk, optval, optlen);
 
 	if (optlen < sizeof(int))
 		return -EINVAL;

@@ -1074,9 +1118,10 @@ int sock_getsockopt(struct socket *sock, int level, int

optname,
case SO_NOFCS:
v.val = sock_flag(sk, SOCK_NOFCS); break;
+
case SO_BINDTODEVICE:
- v.val = sk->sk_bound_dev_if;
- break;
+ return sock_getbindtodevice(sk, optval, optlen, len);
+
default:
return -ENOPROTOOPT;
}

author
#5 lore raw
Quoted 26 lines · Brian Haley
Quoted 25 lines · Pavel Emelyanov

The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but cannot be get via sockopt API. The only way we can find the device id a socket is bound to is via sock-diag interface. But the diag works only on hashed sockets, while the opt in question can be set for yet unhashed one.

That said, in order to know what device a socket is bound to (we do want to know this in checkpoint-restore project) I propose to make this option getsockopt-able and report the respective device index.

Another solution to the problem might be to teach the sock-diag reporting info on unhashed sockets. Should I go this way instead?

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/core/sock.c +1 -0
index 8a146cf..c49412c 100644
@@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 	case SO_NOFCS:
 		v.val = sock_flag(sk, SOCK_NOFCS);
 		break;
+	case SO_BINDTODEVICE:
Quoted 8 lines

net/core/sock.c int sock_getsockopt(struct socket *sock, int level, int optname, · line 1082

Quoted 5 lines
net/core/sock.c +2 -0
+		v.val = sk->sk_bound_dev_if;
+		break;
 	default:
 		return -ENOPROTOOPT;
 	}

Doesn't this make the set and get non-symmetrical? For example, setsockopt() would take "eth0", but getsockopt() would return 2.

It will, but since device name and index are two equal device "IDs" I assumed it would be OK.

However, some comments inline.

Quoted 31 lines

The following patch would return a string, or -ENODEV if not set.

-Brian

---

Change getsockopt(SO_BINDTODEVICE) to be symmetrical with setsockopt() by returning the interface name as a string.

Signed-off-by Brian Haley <[email protected]>
net/core/sock.c +4 -1
index c49412c..69b9d92 100644
@@ -505,7 +505,8 @@ struct dst_entry *sk_dst_check(struct sock *sk, u32 cookie)
 }
 EXPORT_SYMBOL(sk_dst_check);
 
-static int sock_bindtodevice(struct sock *sk, char __user *optval, int optlen)
+static int sock_setbindtodevice(struct sock *sk, char __user *optval,
+				int optlen)
 {
 	int ret = -ENOPROTOOPT;
 #ifdef CONFIG_NETDEVICES

@@ -562,6 +563,49 @@ out:
 	return ret;
 }
 
+static int sock_getbindtodevice(struct sock *sk, char __user *optval,
+				int __user *optlen, int len)

net/core/sock.c out: · line 577

Quoted 10 lines
net/core/sock.c +10 -0
+{
+	int ret = -ENOPROTOOPT;
+#ifdef CONFIG_NETDEVICES
+	struct net *net = sock_net(sk);
+	struct net_device *dev;
+	char devname[IFNAMSIZ];
+
+	ret = 0;
+	if (sk->sk_bound_dev_if == 0)
+		goto out;

It will return 0 if device is not set, thus making it impossible to detect this situation.

Quoted 11 lines
net/core/sock.c +11 -0
+	ret = -EINVAL;
+	if (len < IFNAMSIZ)
+		goto out;
+	if (len > IFNAMSIZ)
+		len = IFNAMSIZ;
+
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
+	if (dev)
+		strcpy(dev->name, devname);
+	rcu_read_unlock();

net/core/sock.c out: · line 599

Quoted 10 lines
net/core/sock.c +10 -0
+	ret = -ENODEV;
+	if (!dev)
+		goto out;
+
+	ret = -EFAULT;
+	if (copy_to_user(optval, devname, len))
+		goto out;
+
+	if (put_user(len, optlen))
+		goto out;

What's the point in reporting IFNAMSIZ to the userspace always, taking into account that this constant is exported there anyway?

Quoted 40 lines
net/core/sock.c +12 -3
+	ret = 0;
+
+out:
+#endif
+
+	return ret;
+}
+
 static inline void sock_valbool_flag(struct sock *sk, int bit, int valbool)
 {
 	if (valbool)

@@ -589,7 +633,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 	 */
 
 	if (optname == SO_BINDTODEVICE)
-		return sock_bindtodevice(sk, optval, optlen);
+		return sock_setbindtodevice(sk, optval, optlen);
 
 	if (optlen < sizeof(int))
 		return -EINVAL;

@@ -1074,9 +1118,10 @@ int sock_getsockopt(struct socket *sock, int level, int
 optname,
 	case SO_NOFCS:
 		v.val = sock_flag(sk, SOCK_NOFCS);
 		break;
+
 	case SO_BINDTODEVICE:
-		v.val = sk->sk_bound_dev_if;
-		break;
+		return sock_getbindtodevice(sk, optval, optlen, len);
+
 	default:
 		return -ENOPROTOOPT;

}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
.

#6 lore raw
Quoted 36 lines · Pavel Emelyanov
Quoted 26 lines · Brian Haley
Quoted 25 lines · Pavel Emelyanov

The SO_BINDTODEVICE option is the only SOL_SOCKET one that can be set, but cannot be get via sockopt API. The only way we can find the device id a socket is bound to is via sock-diag interface. But the diag works only on hashed sockets, while the opt in question can be set for yet unhashed one.

That said, in order to know what device a socket is bound to (we do want to know this in checkpoint-restore project) I propose to make this option getsockopt-able and report the respective device index.

Another solution to the problem might be to teach the sock-diag reporting info on unhashed sockets. Should I go this way instead?

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/core/sock.c +1 -0
index 8a146cf..c49412c 100644
@@ -1074,6 +1074,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 	case SO_NOFCS:
 		v.val = sock_flag(sk, SOCK_NOFCS);
 		break;
+	case SO_BINDTODEVICE:
Quoted 8 lines

net/core/sock.c int sock_getsockopt(struct socket *sock, int level, int optname, · line 1082

Quoted 5 lines
net/core/sock.c +2 -0
+		v.val = sk->sk_bound_dev_if;
+		break;
 	default:
 		return -ENOPROTOOPT;
 	}

Doesn't this make the set and get non-symmetrical? For example, setsockopt() would take "eth0", but getsockopt() would return 2.

Quoted 6 lines

It will, but since device name and index are two equal device "IDs" I assumed it would be OK.

However, some comments inline.

Quoted 1 line

The following patch would return a string, or -ENODEV if not set.

Sorry, my description is not quite right, this should return something like this to be correct:

0 on success
optlen zero if interface not set
optlen > zero if set and optval filled-in

-errno on failure

Quoted 7 lines
Quoted 7 lines
net/core/sock.c +7 -0
+static int sock_getbindtodevice(struct sock *sk, char __user *optval,
+				int __user *optlen, int len)
+{
+	int ret = -ENOPROTOOPT;
+#ifdef CONFIG_NETDEVICES
+	struct net *net = sock_net(sk);
+	struct net_device *dev;
Quoted 8 lines

net/core/sock.c out: · line 577

Quoted 5 lines
net/core/sock.c +5 -0
+	char devname[IFNAMSIZ];
+
+	ret = 0;
+	if (sk->sk_bound_dev_if == 0)
+		goto out;

It will return 0 if device is not set, thus making it impossible to detect this situation.

See below.

Quoted 16 lines
Quoted 16 lines
net/core/sock.c +16 -0
+	ret = -EINVAL;
+	if (len < IFNAMSIZ)
+		goto out;
+	if (len > IFNAMSIZ)
+		len = IFNAMSIZ;
+
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
+	if (dev)
+		strcpy(dev->name, devname);
+	rcu_read_unlock();
+	ret = -ENODEV;
+	if (!dev)
+		goto out;
+
+	ret = -EFAULT;
Quoted 8 lines

net/core/sock.c out: · line 599

Quoted 5 lines
net/core/sock.c +5 -0
+	if (copy_to_user(optval, devname, len))
+		goto out;
+
+	if (put_user(len, optlen))
+		goto out;

What's the point in reporting IFNAMSIZ to the userspace always, taking into account that this constant is exported there anyway?

I should have put an RFC on the patch :)

In the case that there is no interface, the length returned would be zero, indicating nothing was there.

I can post another version.

-Brian

#7 lore raw
In reply to Brian Haley
Quoted 8 lines · Brian Haley
net/core/sock.c +8 -0
+	char devname[IFNAMSIZ];
+
+	ret = 0;
+	if (sk->sk_bound_dev_if == 0)
+		goto out;
+
+	ret = -EINVAL;
+	if (len < IFNAMSIZ)

net/core/sock.c out: · line 590

Quoted 10 lines
net/core/sock.c +10 -0
+		goto out;
+	if (len > IFNAMSIZ)
+		len = IFNAMSIZ;
+
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
+	if (dev)
+		strcpy(dev->name, devname);
+	rcu_read_unlock();
+	ret = -ENODEV;

You probably meant

	strcpy(devname, dev->name)

By the way, this is not really safe in case device is renamed

author
#8 lore raw
Quoted 23 lines · Eric Dumazet
Quoted 8 lines · Brian Haley
net/core/sock.c +8 -0
+	char devname[IFNAMSIZ];
+
+	ret = 0;
+	if (sk->sk_bound_dev_if == 0)
+		goto out;
+
+	ret = -EINVAL;
+	if (len < IFNAMSIZ)

net/core/sock.c out: · line 590

Quoted 10 lines
net/core/sock.c +10 -0
+		goto out;
+	if (len > IFNAMSIZ)
+		len = IFNAMSIZ;
+
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
+	if (dev)
+		strcpy(dev->name, devname);
+	rcu_read_unlock();
+	ret = -ENODEV;

You probably meant

Quoted 3 lines
	strcpy(devname, dev->name)

By the way, this is not really safe in case device is renamed

Good point, actually. Getting a device name may be not very safe in terms of -- once we have the name there's no 100% guarantee, that this name corresponds to the actual device the socket is bound to (it could be renamed after we strcpy-ed its name). This problem doesn't exist when we get device index, as it cannot be changed.

Thanks,
Pavel

#9 lore raw
In reply to Eric Dumazet
Quoted 23 lines · Eric Dumazet
Quoted 8 lines · Brian Haley
net/core/sock.c +8 -0
+	char devname[IFNAMSIZ];
+
+	ret = 0;
+	if (sk->sk_bound_dev_if == 0)
+		goto out;
+
+	ret = -EINVAL;
+	if (len < IFNAMSIZ)

net/core/sock.c out: · line 590

Quoted 10 lines
net/core/sock.c +10 -0
+		goto out;
+	if (len > IFNAMSIZ)
+		len = IFNAMSIZ;
+
+	rcu_read_lock();
+	dev = dev_get_by_index_rcu(net, sk->sk_bound_dev_if);
+	if (dev)
+		strcpy(dev->name, devname);
+	rcu_read_unlock();
+	ret = -ENODEV;

You probably meant

Quoted 1 line
	strcpy(devname, dev->name)

Yes, that was a stupid mistake, I'll fix it.

Quoted 1 line

By the way, this is not really safe in case device is renamed

It's not much different from what's there:

	setsockopt("foo");

	rename foo -> bar

	index = getsockopt();
	if_indextoname(index) -> "bar"

I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *' to getsockopt(), I don't think any other value is non-symmetrical like this.

-Brian

#10 lore raw
Quoted 9 lines · Brian Haley

It's not much different from what's there:

	setsockopt("foo");

	rename foo -> bar

	index = getsockopt();
	if_indextoname(index) -> "bar"
Quoted 4 lines

I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *' to getsockopt(), I don't think any other value is non-symmetrical like this.

-Brian

I meant another cpu can be changing dev->name[] content while the strcpy() is done, and you get a mangled devname, like "for" or "bao" instead of "foo" or "bar"

But yes, I obviously understood your point about "char *" and "int *"

#11 lore raw
Quoted 11 lines · Eric Dumazet
Quoted 9 lines · Brian Haley

It's not much different from what's there:

	setsockopt("foo");

	rename foo -> bar

	index = getsockopt();
	if_indextoname(index) -> "bar"
Quoted 8 lines
Quoted 4 lines

I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *' to getsockopt(), I don't think any other value is non-symmetrical like this.

-Brian

I meant another cpu can be changing dev->name[] content while the strcpy() is done, and you get a mangled devname, like "for" or "bao" instead of "foo" or "bar"

Even when holding the rcu_read_lock()? I'd have to hold the rtnl lock there?

-Brian

#12 lore raw
Quoted 15 lines · Brian Haley
Quoted 14 lines · Eric Dumazet
Quoted 12 lines · Brian Haley

It's not much different from what's there:

	setsockopt("foo");

	rename foo -> bar

	index = getsockopt();
	if_indextoname(index) -> "bar"

I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *' to getsockopt(), I don't think any other value is non-symmetrical like this.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

-Brian

I meant another cpu can be changing dev->name[] content while the strcpy() is done, and you get a mangled devname, like "for" or "bao" instead of "foo" or "bar"

Even when holding the rcu_read_lock()? I'd have to hold the rtnl lock there?

Yes, rcu_read_lock() only makes sure the device doesnt disappear.

But its name can be changed.

You could use a seqcount_t, so that readers dont have to lock rtnl.

But do we really want to return a name here, I am not yet convinced.

#13 lore raw
Quoted 24 lines · Eric Dumazet
Quoted 15 lines · Brian Haley
Quoted 14 lines · Eric Dumazet
Quoted 12 lines · Brian Haley

It's not much different from what's there:

	setsockopt("foo");

	rename foo -> bar

	index = getsockopt();
	if_indextoname(index) -> "bar"

I more raised the issue since you pass a 'char *' to setsockopt() but an 'int *' to getsockopt(), I don't think any other value is non-symmetrical like this.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

-Brian

I meant another cpu can be changing dev->name[] content while the strcpy() is done, and you get a mangled devname, like "for" or "bao" instead of "foo" or "bar"

Even when holding the rcu_read_lock()? I'd have to hold the rtnl lock there?

Quoted 7 lines

Yes, rcu_read_lock() only makes sure the device doesnt disappear.

But its name can be changed.

You could use a seqcount_t, so that readers dont have to lock rtnl.

But do we really want to return a name here, I am not yet convinced.

If setsockopt() takes a name then it makes no sense that getsockopt() would return an index. Perhaps an SO_BINDTOIFINDEX would be useful, but let's not make SO_BINDTODEVICE mean two different things.

Ben.

Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
#14 lore raw
In reply to Eric Dumazet
Quoted 9 lines · Eric Dumazet
Quoted 5 lines
Quoted 3 lines

I meant another cpu can be changing dev->name[] content while the strcpy() is done, and you get a mangled devname, like "for" or "bao" instead of "foo" or "bar"

Even when holding the rcu_read_lock()? I'd have to hold the rtnl lock there?

Yes, rcu_read_lock() only makes sure the device doesnt disappear.

But its name can be changed.

There's a similar bug in the SIOCGIFNAME/dev_ifname() code too then, but I would think this rare enough that it doesn't happen in practice.

-Brian