Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net-next] tun: no longer rely on RTNL in tun_fill_info()
@ 2026-07-06 16:35 Eric Dumazet
  0 siblings, 0 replies; only message in thread
From: Eric Dumazet @ 2026-07-06 16:35 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Andrew Lunn, Kuniyuki Iwashima, netdev,
	eric.dumazet, Eric Dumazet, Willem de Bruijn

Update tun_fill_info() to read device configuration fields (flags, owner,
group, numqueues, numdisabled) locklessly using READ_ONCE().

Annotate all writes to these fields in the control paths with WRITE_ONCE()
to prevent data races, as these fields can be modified concurrently via
ioctls (TUNSETPERSIST, TUNSETOWNER, TUNSETGROUP, TUNSETIFF) or queue
attaching/detaching.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
v2: addressed Kuniyuki and sashiko feedback.
v1: https://lore.kernel.org/netdev/20260701125112.3652880-1-edumazet@google.com/

 drivers/net/tun.c      | 60 ++++++++++++++++++++++--------------------
 drivers/net/tun_vnet.h |  8 +++---
 2 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ffbe6f13fb1fa2333227a6b085806c82a362c0a4..9e2761887896955a243649b75d35a3e2a4c67529 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -532,7 +532,7 @@ static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
 {
 	tfile->detached = tun;
 	list_add_tail(&tfile->next, &tun->disabled);
-	++tun->numdisabled;
+	WRITE_ONCE(tun->numdisabled, tun->numdisabled + 1);
 }
 
 static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
@@ -541,7 +541,7 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
 
 	tfile->detached = NULL;
 	list_del_init(&tfile->next);
-	--tun->numdisabled;
+	WRITE_ONCE(tun->numdisabled, tun->numdisabled - 1);
 	return tun;
 }
 
@@ -600,7 +600,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 		rcu_assign_pointer(tun->tfiles[tun->numqueues - 1],
 				   NULL);
 
-		--tun->numqueues;
+		WRITE_ONCE(tun->numqueues, tun->numqueues - 1);
 		if (clean) {
 			RCU_INIT_POINTER(tfile->tun, NULL);
 			sock_put(&tfile->sk);
@@ -663,7 +663,7 @@ static void tun_detach_all(struct net_device *dev)
 		tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
 		tfile->socket.sk->sk_data_ready(tfile->socket.sk);
 		RCU_INIT_POINTER(tfile->tun, NULL);
-		--tun->numqueues;
+		WRITE_ONCE(tun->numqueues, tun->numqueues - 1);
 	}
 	list_for_each_entry(tfile, &tun->disabled, next) {
 		tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
@@ -786,7 +786,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
 	if (publish_tun)
 		rcu_assign_pointer(tfile->tun, tun);
 	rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
-	tun->numqueues++;
+	WRITE_ONCE(tun->numqueues, tun->numqueues + 1);
 	tun_set_real_num_queues(tun);
 out:
 	return err;
@@ -2370,32 +2370,36 @@ static size_t tun_get_size(const struct net_device *dev)
 
 static int tun_fill_info(struct sk_buff *skb, const struct net_device *dev)
 {
-	struct tun_struct *tun = netdev_priv(dev);
+	const struct tun_struct *tun = netdev_priv(dev);
+	unsigned int flags = READ_ONCE(tun->flags);
+	kuid_t owner = READ_ONCE(tun->owner);
+	kgid_t group = READ_ONCE(tun->group);
 
-	if (nla_put_u8(skb, IFLA_TUN_TYPE, tun->flags & TUN_TYPE_MASK))
+	if (nla_put_u8(skb, IFLA_TUN_TYPE, flags & TUN_TYPE_MASK))
 		goto nla_put_failure;
-	if (uid_valid(tun->owner) &&
+	if (uid_valid(owner) &&
 	    nla_put_u32(skb, IFLA_TUN_OWNER,
-			from_kuid_munged(current_user_ns(), tun->owner)))
+			from_kuid_munged(current_user_ns(), owner)))
 		goto nla_put_failure;
-	if (gid_valid(tun->group) &&
+	if (gid_valid(group) &&
 	    nla_put_u32(skb, IFLA_TUN_GROUP,
-			from_kgid_munged(current_user_ns(), tun->group)))
+			from_kgid_munged(current_user_ns(), group)))
 		goto nla_put_failure;
-	if (nla_put_u8(skb, IFLA_TUN_PI, !(tun->flags & IFF_NO_PI)))
+	if (nla_put_u8(skb, IFLA_TUN_PI, !(flags & IFF_NO_PI)))
 		goto nla_put_failure;
-	if (nla_put_u8(skb, IFLA_TUN_VNET_HDR, !!(tun->flags & IFF_VNET_HDR)))
+	if (nla_put_u8(skb, IFLA_TUN_VNET_HDR, !!(flags & IFF_VNET_HDR)))
 		goto nla_put_failure;
-	if (nla_put_u8(skb, IFLA_TUN_PERSIST, !!(tun->flags & IFF_PERSIST)))
+	if (nla_put_u8(skb, IFLA_TUN_PERSIST, !!(flags & IFF_PERSIST)))
 		goto nla_put_failure;
 	if (nla_put_u8(skb, IFLA_TUN_MULTI_QUEUE,
-		       !!(tun->flags & IFF_MULTI_QUEUE)))
+		       !!(flags & IFF_MULTI_QUEUE)))
 		goto nla_put_failure;
-	if (tun->flags & IFF_MULTI_QUEUE) {
-		if (nla_put_u32(skb, IFLA_TUN_NUM_QUEUES, tun->numqueues))
+	if (flags & IFF_MULTI_QUEUE) {
+		if (nla_put_u32(skb, IFLA_TUN_NUM_QUEUES,
+				READ_ONCE(tun->numqueues)))
 			goto nla_put_failure;
 		if (nla_put_u32(skb, IFLA_TUN_NUM_DISABLED_QUEUES,
-				tun->numdisabled))
+				READ_ONCE(tun->numdisabled)))
 			goto nla_put_failure;
 	}
 
@@ -2814,8 +2818,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 			return 0;
 		}
 
-		tun->flags = (tun->flags & ~TUN_FEATURES) |
-			      (ifr->ifr_flags & TUN_FEATURES);
+		WRITE_ONCE(tun->flags, (tun->flags & ~TUN_FEATURES) |
+				       (ifr->ifr_flags & TUN_FEATURES));
 
 		netdev_state_change(dev);
 	} else {
@@ -3213,13 +3217,13 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		/* Disable/Enable persist mode. Keep an extra reference to the
 		 * module to prevent the module being unprobed.
 		 */
-		if (arg && !(tun->flags & IFF_PERSIST)) {
-			tun->flags |= IFF_PERSIST;
+		if (arg && !(READ_ONCE(tun->flags) & IFF_PERSIST)) {
+			WRITE_ONCE(tun->flags, READ_ONCE(tun->flags) | IFF_PERSIST);
 			__module_get(THIS_MODULE);
 			do_notify = true;
 		}
-		if (!arg && (tun->flags & IFF_PERSIST)) {
-			tun->flags &= ~IFF_PERSIST;
+		if (!arg && (READ_ONCE(tun->flags) & IFF_PERSIST)) {
+			WRITE_ONCE(tun->flags, READ_ONCE(tun->flags) & ~IFF_PERSIST);
 			module_put(THIS_MODULE);
 			do_notify = true;
 		}
@@ -3235,10 +3239,10 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 			ret = -EINVAL;
 			break;
 		}
-		tun->owner = owner;
+		WRITE_ONCE(tun->owner, owner);
 		do_notify = true;
 		netif_info(tun, drv, tun->dev, "owner set to %u\n",
-			   from_kuid(&init_user_ns, tun->owner));
+			   from_kuid(&init_user_ns, owner));
 		break;
 
 	case TUNSETGROUP:
@@ -3248,10 +3252,10 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 			ret = -EINVAL;
 			break;
 		}
-		tun->group = group;
+		WRITE_ONCE(tun->group, group);
 		do_notify = true;
 		netif_info(tun, drv, tun->dev, "group set to %u\n",
-			   from_kgid(&init_user_ns, tun->group));
+			   from_kgid(&init_user_ns, group));
 		break;
 
 	case TUNSETLINK:
diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index fa5cab9d3e55c5a6e70afe4c76582c8affe0b73c..f4c652b1fa44d82ae23243857809619bfb5290d2 100644
--- a/drivers/net/tun_vnet.h
+++ b/drivers/net/tun_vnet.h
@@ -40,9 +40,9 @@ static inline long tun_set_vnet_be(unsigned int *flags, int __user *argp)
 		return -EFAULT;
 
 	if (be)
-		*flags |= TUN_VNET_BE;
+		WRITE_ONCE(*flags, *flags | TUN_VNET_BE);
 	else
-		*flags &= ~TUN_VNET_BE;
+		WRITE_ONCE(*flags, *flags & ~TUN_VNET_BE);
 
 	return 0;
 }
@@ -93,9 +93,9 @@ static inline long tun_vnet_ioctl(int *vnet_hdr_sz, unsigned int *flags,
 		if (get_user(s, sp))
 			return -EFAULT;
 		if (s)
-			*flags |= TUN_VNET_LE;
+			WRITE_ONCE(*flags, *flags | TUN_VNET_LE);
 		else
-			*flags &= ~TUN_VNET_LE;
+			WRITE_ONCE(*flags, *flags & ~TUN_VNET_LE);
 		return 0;
 
 	case TUNGETVNETBE:
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-07-06 16:35 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06 16:35 [PATCH v2 net-next] tun: no longer rely on RTNL in tun_fill_info() Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox