[PATCH v1 net 0/8] net: Fix UAF of sk_dst_get(sk)->dev.

[PATCH v1 net 0/8] net: Fix UAF of sk_dst_get(sk)->dev.

[Kuniyuki Iwashima]

syzbot caught use-after-free of sk_dst_get(sk)->dev,
which was not fetched under RCU nor RTNL. [0]

Patch 1 adds 2 helpers to fetch __sk_dst_get(sk)->dev safely
Patch 2 ~ 8 fix UAF in smc, tcp, ktls, mptcp

[0]: https://lore.kernel.org/netdev/68c237c7.050a0220.3c6139.0036.GAE@google.com/


Kuniyuki Iwashima (8):
  net: Add sk_dst_dev_rcu() and sk_dst_dev_get().
  smc: Fix use-after-free in __pnet_find_base_ndev().
  smc: Use sk_dst_dev_rcu() in in smc_clc_prfx_set().
  smc: Use sk_dst_dev_rcu() in smc_clc_prfx_match().
  smc: Use sk_dst_dev_rcu() in smc_vlan_by_tcpsk().
  tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().
  tls: Use sk_dst_dev_rcu() in get_netdev_for_sock().
  mptcp: Use sk_dst_dev_rcu() in mptcp_active_enable().

 include/net/sock.h      | 19 +++++++++++++
 net/ipv4/tcp_fastopen.c |  7 ++---
 net/mptcp/ctrl.c        |  7 +++--
 net/smc/smc_clc.c       | 63 ++++++++++++++++++++---------------------
 net/smc/smc_core.c      | 25 +++++++---------
 net/smc/smc_pnet.c      | 35 ++++++++---------------
 net/tls/tls_device.c    | 16 +++++------
 7 files changed, 88 insertions(+), 84 deletions(-)

-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 1/8] net: Add sk_dst_dev_rcu() and sk_dst_dev_get().

[Kuniyuki Iwashima]

dst->dev is safe under RTNL or RCU.

syzbot demonstrated that an unsafe use of sk_dst_get()->dev
leads to use-after-free.

Let's add two helpers to fix such issues.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/sock.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/net/sock.h b/include/net/sock.h
index fb13322a11fc..e1ae975c1920 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2157,6 +2157,25 @@ sk_dst_get(const struct sock *sk)
 	return dst;
 }
 
+static inline struct net_device *sk_dst_dev_rcu(const struct sock *sk)
+{
+	struct dst_entry *dst = __sk_dst_get(sk);
+
+	return dst ? dst_dev_rcu(dst) : NULL;
+}
+
+static inline struct net_device *sk_dst_dev_get(const struct sock *sk)
+{
+	struct net_device *dev;
+
+	rcu_read_lock();
+	dev = sk_dst_dev_rcu(sk);
+	dev_hold(dev);
+	rcu_read_unlock();
+
+	return dev;
+}
+
 static inline void __dst_negative_advice(struct sock *sk)
 {
 	struct dst_entry *dst = __sk_dst_get(sk);
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 2/8] smc: Fix use-after-free in __pnet_find_base_ndev().

[Kuniyuki Iwashima]

syzbot reported use-after-free of net_device in __pnet_find_base_ndev(),
which was called during connect(). [0]

smc_pnet_find_ism_resource() fetches sk_dst_get(sk)->dev and passes
down to pnet_find_base_ndev(), where RTNL is held.  Then, UAF happened
at __pnet_find_base_ndev() when the dev is first used.

This means dev had already been freed before acquiring RTNL in
pnet_find_base_ndev().

While dev is going away, dst->dev could be swapped with blackhole_netdev,
and the dev's refcnt by dst will be released.

We must hold dev's refcnt before calling smc_pnet_find_ism_resource().

Also, smc_pnet_find_roce_resource() has the same problem.

Let's use sk_dst_dev_get() in the two functions.

[0]:
BUG: KASAN: use-after-free in __pnet_find_base_ndev+0x1b1/0x1c0 net/smc/smc_pnet.c:926
Read of size 1 at addr ffff888036bac33a by task syz.0.3632/18609

CPU: 1 UID: 0 PID: 18609 Comm: syz.0.3632 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
Call Trace:
 <TASK>
 dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0xca/0x240 mm/kasan/report.c:482
 kasan_report+0x118/0x150 mm/kasan/report.c:595
 __pnet_find_base_ndev+0x1b1/0x1c0 net/smc/smc_pnet.c:926
 pnet_find_base_ndev net/smc/smc_pnet.c:946 [inline]
 smc_pnet_find_ism_by_pnetid net/smc/smc_pnet.c:1103 [inline]
 smc_pnet_find_ism_resource+0xef/0x390 net/smc/smc_pnet.c:1154
 smc_find_ism_device net/smc/af_smc.c:1030 [inline]
 smc_find_proposal_devices net/smc/af_smc.c:1115 [inline]
 __smc_connect+0x372/0x1890 net/smc/af_smc.c:1545
 smc_connect+0x877/0xd90 net/smc/af_smc.c:1715
 __sys_connect_file net/socket.c:2086 [inline]
 __sys_connect+0x313/0x440 net/socket.c:2105
 __do_sys_connect net/socket.c:2111 [inline]
 __se_sys_connect net/socket.c:2108 [inline]
 __x64_sys_connect+0x7a/0x90 net/socket.c:2108
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f47cbf8eba9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f47ccdb1038 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 00007f47cc1d5fa0 RCX: 00007f47cbf8eba9
RDX: 0000000000000010 RSI: 0000200000000280 RDI: 000000000000000b
RBP: 00007f47cc011e19 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f47cc1d6038 R14: 00007f47cc1d5fa0 R15: 00007ffc512f8aa8
 </TASK>

The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff888036bacd00 pfn:0x36bac
flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000000000 ffffea0001243d08 ffff8880b863fdc0 0000000000000000
raw: ffff888036bacd00 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as freed
page last allocated via order 2, migratetype Unmovable, gfp_mask 0x446dc0(GFP_KERNEL_ACCOUNT|__GFP_ZERO|__GFP_NOWARN|__GFP_RETRY_MAYFAIL|__GFP_COMP), pid 16741, tgid 16741 (syz-executor), ts 343313197788, free_ts 380670750466
 set_page_owner include/linux/page_owner.h:32 [inline]
 post_alloc_hook+0x240/0x2a0 mm/page_alloc.c:1851
 prep_new_page mm/page_alloc.c:1859 [inline]
 get_page_from_freelist+0x21e4/0x22c0 mm/page_alloc.c:3858
 __alloc_frozen_pages_noprof+0x181/0x370 mm/page_alloc.c:5148
 alloc_pages_mpol+0x232/0x4a0 mm/mempolicy.c:2416
 ___kmalloc_large_node+0x5f/0x1b0 mm/slub.c:4317
 __kmalloc_large_node_noprof+0x18/0x90 mm/slub.c:4348
 __do_kmalloc_node mm/slub.c:4364 [inline]
 __kvmalloc_node_noprof+0x6d/0x5f0 mm/slub.c:5067
 alloc_netdev_mqs+0xa3/0x11b0 net/core/dev.c:11812
 tun_set_iff+0x532/0xef0 drivers/net/tun.c:2775
 __tun_chr_ioctl+0x788/0x1df0 drivers/net/tun.c:3085
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:598 [inline]
 __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:584
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
page last free pid 18610 tgid 18608 stack trace:
 reset_page_owner include/linux/page_owner.h:25 [inline]
 free_pages_prepare mm/page_alloc.c:1395 [inline]
 __free_frozen_pages+0xbc4/0xd30 mm/page_alloc.c:2895
 free_large_kmalloc+0x13a/0x1f0 mm/slub.c:4820
 device_release+0x99/0x1c0 drivers/base/core.c:-1
 kobject_cleanup lib/kobject.c:689 [inline]
 kobject_release lib/kobject.c:720 [inline]
 kref_put include/linux/kref.h:65 [inline]
 kobject_put+0x22b/0x480 lib/kobject.c:737
 netdev_run_todo+0xd2e/0xea0 net/core/dev.c:11513
 rtnl_unlock net/core/rtnetlink.c:157 [inline]
 rtnl_net_unlock include/linux/rtnetlink.h:135 [inline]
 rtnl_dellink+0x537/0x710 net/core/rtnetlink.c:3563
 rtnetlink_rcv_msg+0x7cc/0xb70 net/core/rtnetlink.c:6946
 netlink_rcv_skb+0x208/0x470 net/netlink/af_netlink.c:2552
 netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
 netlink_unicast+0x82f/0x9e0 net/netlink/af_netlink.c:1346
 netlink_sendmsg+0x805/0xb30 net/netlink/af_netlink.c:1896
 sock_sendmsg_nosec net/socket.c:714 [inline]
 __sock_sendmsg+0x219/0x270 net/socket.c:729
 ____sys_sendmsg+0x505/0x830 net/socket.c:2614
 ___sys_sendmsg+0x21f/0x2a0 net/socket.c:2668
 __sys_sendmsg net/socket.c:2700 [inline]
 __do_sys_sendmsg net/socket.c:2705 [inline]
 __se_sys_sendmsg net/socket.c:2703 [inline]
 __x64_sys_sendmsg+0x19b/0x260 net/socket.c:2703
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Memory state around the buggy address:
 ffff888036bac200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff888036bac280: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff888036bac300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
                                        ^
 ffff888036bac380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff888036bac400: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

Fixes: 0afff91c6f5e ("net/smc: add pnetid support")
Fixes: 1619f770589a ("net/smc: add pnetid support for SMC-D and ISM")
Reported-by: syzbot+ea28e9d85be2f327b6c6@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/68c237c7.050a0220.3c6139.0036.GAE@google.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: "D. Wythe" <alibuda@linux.alibaba.com>
Cc: Dust Li <dust.li@linux.alibaba.com>
Cc: Sidraya Jayagond <sidraya@linux.ibm.com>
Cc: Wenjia Zhang <wenjia@linux.ibm.com>
Cc: Mahanta Jambigi <mjambigi@linux.ibm.com>
Cc: Tony Lu <tonylu@linux.alibaba.com>
Cc: Wen Gu <guwen@linux.alibaba.com>
Cc: Ursula Braun <ubraun@linux.ibm.com>
Cc: Hans Wippel <hwippel@linux.ibm.com>
---
 net/smc/smc_pnet.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 76ad29e31d60..e59c3827115e 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -1126,37 +1126,26 @@ static void smc_pnet_find_ism_by_pnetid(struct net_device *ndev,
  */
 void smc_pnet_find_roce_resource(struct sock *sk, struct smc_init_info *ini)
 {
-	struct dst_entry *dst = sk_dst_get(sk);
-
-	if (!dst)
-		goto out;
-	if (!dst->dev)
-		goto out_rel;
-
-	smc_pnet_find_roce_by_pnetid(dst->dev, ini);
+	struct net_device *dev;
 
-out_rel:
-	dst_release(dst);
-out:
-	return;
+	dev = sk_dst_dev_get(sk);
+	if (dev) {
+		smc_pnet_find_roce_by_pnetid(dev, ini);
+		dev_put(dev);
+	}
 }
 
 void smc_pnet_find_ism_resource(struct sock *sk, struct smc_init_info *ini)
 {
-	struct dst_entry *dst = sk_dst_get(sk);
+	struct net_device *dev;
 
 	ini->ism_dev[0] = NULL;
-	if (!dst)
-		goto out;
-	if (!dst->dev)
-		goto out_rel;
-
-	smc_pnet_find_ism_by_pnetid(dst->dev, ini);
 
-out_rel:
-	dst_release(dst);
-out:
-	return;
+	dev = sk_dst_dev_get(sk);
+	if (dev) {
+		smc_pnet_find_ism_by_pnetid(dev, ini);
+		dev_put(dev);
+	}
 }
 
 /* Lookup and apply a pnet table entry to the given ib device.
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 3/8] smc: Use sk_dst_dev_rcu() in in smc_clc_prfx_set().

[Kuniyuki Iwashima]

smc_clc_prfx_set() is called during connect() and not under RCU
nor RTNL.

Using sk_dst_get(sk)->dev could trigger UAF.

Let's use sk_dst_get_rcu() under rcu_read_lock() after
kernel_getsockname().

While at it, we change the 1st arg of smc_clc_prfx_set[46]_rcu()
not to touch dst there.

Fixes: a046d57da19f ("smc: CLC handshake (incl. preparation steps)")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: "D. Wythe" <alibuda@linux.alibaba.com>
Cc: Dust Li <dust.li@linux.alibaba.com>
Cc: Sidraya Jayagond <sidraya@linux.ibm.com>
Cc: Wenjia Zhang <wenjia@linux.ibm.com>
Cc: Mahanta Jambigi <mjambigi@linux.ibm.com>
Cc: Tony Lu <tonylu@linux.alibaba.com>
Cc: Wen Gu <guwen@linux.alibaba.com>
Cc: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_clc.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 08be56dfb3f2..9aa1d75d3079 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -509,10 +509,10 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool check_trl)
 }
 
 /* find ipv4 addr on device and get the prefix len, fill CLC proposal msg */
-static int smc_clc_prfx_set4_rcu(struct dst_entry *dst, __be32 ipv4,
+static int smc_clc_prfx_set4_rcu(struct net_device *dev, __be32 ipv4,
 				 struct smc_clc_msg_proposal_prefix *prop)
 {
-	struct in_device *in_dev = __in_dev_get_rcu(dst->dev);
+	struct in_device *in_dev = __in_dev_get_rcu(dev);
 	const struct in_ifaddr *ifa;
 
 	if (!in_dev)
@@ -530,12 +530,12 @@ static int smc_clc_prfx_set4_rcu(struct dst_entry *dst, __be32 ipv4,
 }
 
 /* fill CLC proposal msg with ipv6 prefixes from device */
-static int smc_clc_prfx_set6_rcu(struct dst_entry *dst,
+static int smc_clc_prfx_set6_rcu(struct net_device *dev,
 				 struct smc_clc_msg_proposal_prefix *prop,
 				 struct smc_clc_ipv6_prefix *ipv6_prfx)
 {
 #if IS_ENABLED(CONFIG_IPV6)
-	struct inet6_dev *in6_dev = __in6_dev_get(dst->dev);
+	struct inet6_dev *in6_dev = __in6_dev_get(dev);
 	struct inet6_ifaddr *ifa;
 	int cnt = 0;
 
@@ -564,41 +564,42 @@ static int smc_clc_prfx_set(struct socket *clcsock,
 			    struct smc_clc_msg_proposal_prefix *prop,
 			    struct smc_clc_ipv6_prefix *ipv6_prfx)
 {
-	struct dst_entry *dst = sk_dst_get(clcsock->sk);
 	struct sockaddr_storage addrs;
 	struct sockaddr_in6 *addr6;
 	struct sockaddr_in *addr;
+	struct net_device *dev;
 	int rc = -ENOENT;
 
-	if (!dst) {
-		rc = -ENOTCONN;
-		goto out;
-	}
-	if (!dst->dev) {
-		rc = -ENODEV;
-		goto out_rel;
-	}
 	/* get address to which the internal TCP socket is bound */
 	if (kernel_getsockname(clcsock, (struct sockaddr *)&addrs) < 0)
-		goto out_rel;
+		goto out;
+
 	/* analyze IP specific data of net_device belonging to TCP socket */
 	addr6 = (struct sockaddr_in6 *)&addrs;
+
 	rcu_read_lock();
+
+	dev = sk_dst_dev_rcu(clcsock->sk);
+	if (!dev) {
+		rc = -ENODEV;
+		goto out_unlock;
+	}
+
 	if (addrs.ss_family == PF_INET) {
 		/* IPv4 */
 		addr = (struct sockaddr_in *)&addrs;
-		rc = smc_clc_prfx_set4_rcu(dst, addr->sin_addr.s_addr, prop);
+		rc = smc_clc_prfx_set4_rcu(dev, addr->sin_addr.s_addr, prop);
 	} else if (ipv6_addr_v4mapped(&addr6->sin6_addr)) {
 		/* mapped IPv4 address - peer is IPv4 only */
-		rc = smc_clc_prfx_set4_rcu(dst, addr6->sin6_addr.s6_addr32[3],
+		rc = smc_clc_prfx_set4_rcu(dev, addr6->sin6_addr.s6_addr32[3],
 					   prop);
 	} else {
 		/* IPv6 */
-		rc = smc_clc_prfx_set6_rcu(dst, prop, ipv6_prfx);
+		rc = smc_clc_prfx_set6_rcu(dev, prop, ipv6_prfx);
 	}
+
+out_unlock:
 	rcu_read_unlock();
-out_rel:
-	dst_release(dst);
 out:
 	return rc;
 }
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 4/8] smc: Use sk_dst_dev_rcu() in smc_clc_prfx_match().

[Kuniyuki Iwashima]

smc_clc_prfx_match() is called from smc_listen_work() and
not under RCU nor RTNL.

Using sk_dst_get(sk)->dev could trigger UAF.

Let's use sk_dst_dev_rcu().

Fixes: a046d57da19f ("smc: CLC handshake (incl. preparation steps)")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: "D. Wythe" <alibuda@linux.alibaba.com>
Cc: Dust Li <dust.li@linux.alibaba.com>
Cc: Sidraya Jayagond <sidraya@linux.ibm.com>
Cc: Wenjia Zhang <wenjia@linux.ibm.com>
Cc: Mahanta Jambigi <mjambigi@linux.ibm.com>
Cc: Tony Lu <tonylu@linux.alibaba.com>
Cc: Wen Gu <guwen@linux.alibaba.com>
Cc: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_clc.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 9aa1d75d3079..5b4fe2c22879 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -655,26 +655,24 @@ static int smc_clc_prfx_match6_rcu(struct net_device *dev,
 int smc_clc_prfx_match(struct socket *clcsock,
 		       struct smc_clc_msg_proposal_prefix *prop)
 {
-	struct dst_entry *dst = sk_dst_get(clcsock->sk);
+	struct net_device *dev;
 	int rc;
 
-	if (!dst) {
-		rc = -ENOTCONN;
-		goto out;
-	}
-	if (!dst->dev) {
+	rcu_read_lock();
+
+	dev = sk_dst_dev_rcu(clcsock->sk);
+	if (!dev) {
 		rc = -ENODEV;
-		goto out_rel;
+		goto out;
 	}
-	rcu_read_lock();
+
 	if (!prop->ipv6_prefixes_cnt)
-		rc = smc_clc_prfx_match4_rcu(dst->dev, prop);
+		rc = smc_clc_prfx_match4_rcu(dev, prop);
 	else
-		rc = smc_clc_prfx_match6_rcu(dst->dev, prop);
-	rcu_read_unlock();
-out_rel:
-	dst_release(dst);
+		rc = smc_clc_prfx_match6_rcu(dev, prop);
 out:
+	rcu_read_unlock();
+
 	return rc;
 }
 
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 5/8] smc: Use sk_dst_dev_rcu() in smc_vlan_by_tcpsk().

[Kuniyuki Iwashima]

smc_vlan_by_tcpsk() fetches sk_dst_get(sk)->dev before RTNL and
passes it to netdev_walk_all_lower_dev(), which is illegal.

Also, smc_vlan_by_tcpsk_walk() does not require RTNL at all.

Let's use sk_dst_dev_rcu() and netdev_walk_all_lower_dev_rcu().

Fixes: 0cfdd8f92cac ("smc: connection and link group creation")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: "D. Wythe" <alibuda@linux.alibaba.com>
Cc: Dust Li <dust.li@linux.alibaba.com>
Cc: Sidraya Jayagond <sidraya@linux.ibm.com>
Cc: Wenjia Zhang <wenjia@linux.ibm.com>
Cc: Mahanta Jambigi <mjambigi@linux.ibm.com>
Cc: Tony Lu <tonylu@linux.alibaba.com>
Cc: Wen Gu <guwen@linux.alibaba.com>
Cc: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/smc_core.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 262746e304dd..feb61018ce66 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1883,35 +1883,30 @@ static int smc_vlan_by_tcpsk_walk(struct net_device *lower_dev,
 /* Determine vlan of internal TCP socket. */
 int smc_vlan_by_tcpsk(struct socket *clcsock, struct smc_init_info *ini)
 {
-	struct dst_entry *dst = sk_dst_get(clcsock->sk);
 	struct netdev_nested_priv priv;
 	struct net_device *ndev;
 	int rc = 0;
 
 	ini->vlan_id = 0;
-	if (!dst) {
-		rc = -ENOTCONN;
-		goto out;
-	}
-	if (!dst->dev) {
+
+	rcu_read_lock();
+
+	ndev = sk_dst_dev_rcu(clcsock->sk);
+	if (!ndev) {
 		rc = -ENODEV;
-		goto out_rel;
+		goto out;
 	}
 
-	ndev = dst->dev;
 	if (is_vlan_dev(ndev)) {
 		ini->vlan_id = vlan_dev_vlan_id(ndev);
-		goto out_rel;
+		goto out;
 	}
 
 	priv.data = (void *)&ini->vlan_id;
-	rtnl_lock();
-	netdev_walk_all_lower_dev(ndev, smc_vlan_by_tcpsk_walk, &priv);
-	rtnl_unlock();
-
-out_rel:
-	dst_release(dst);
+	netdev_walk_all_lower_dev_rcu(ndev, smc_vlan_by_tcpsk_walk, &priv);
 out:
+	rcu_read_unlock();
+
 	return rc;
 }
 
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

[Kuniyuki Iwashima]

tcp_fastopen_active_disable_ofo_check() is called from tcp_disconnect()
or tcp_v4_destroy_sock(), so not under RCU nor RTNL.

Using sk_dst_get(sk)->dev could trigger UAF.

Let's use sk_dst_dev_rcu().

Fixes: 4a6ce2b6f2ec ("net: introduce a new function dst_dev_put()")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_fastopen.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index f1884f0c9e52..de849b8c87ef 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -560,7 +560,6 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct net_device *dev;
-	struct dst_entry *dst;
 	struct sk_buff *skb;
 
 	if (!tp->syn_fastopen)
@@ -576,11 +575,11 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
 		}
 	} else if (tp->syn_fastopen_ch &&
 		   atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
-		dst = sk_dst_get(sk);
-		dev = dst ? dst_dev(dst) : NULL;
+		rcu_read_lock();
+		dev = sk_dst_dev_rcu(sk);
 		if (!(dev && (dev->flags & IFF_LOOPBACK)))
 			atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
-		dst_release(dst);
+		rcu_read_unlock();
 	}
 }
 
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 7/8] tls: Use sk_dst_dev_rcu() in get_netdev_for_sock().

[Kuniyuki Iwashima]

get_netdev_for_sock() is called during setsockopt(),
so not under RCU.

Using sk_dst_get(sk)->dev could trigger UAF.

Let's use sk_dst_dev_rcu().

Note that the only ->ndo_sk_get_lower_dev() user is
bond_sk_get_lower_dev(), which uses RCU.

Fixes: e8f69799810c ("net/tls: Add generic NIC offload infrastructure")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Cc: Ilya Lesokhin <ilyal@mellanox.com>
---
 net/tls/tls_device.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index f672a62a9a52..cacc1709e945 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -123,17 +123,17 @@ static void tls_device_queue_ctx_destruction(struct tls_context *ctx)
 /* We assume that the socket is already connected */
 static struct net_device *get_netdev_for_sock(struct sock *sk)
 {
-	struct dst_entry *dst = sk_dst_get(sk);
-	struct net_device *netdev = NULL;
+	struct net_device *dev, *lowest_dev = NULL;
 
-	if (likely(dst)) {
-		netdev = netdev_sk_get_lowest_dev(dst->dev, sk);
-		dev_hold(netdev);
+	rcu_read_lock();
+	dev = sk_dst_dev_rcu(sk);
+	if (likely(dev)) {
+		lowest_dev = netdev_sk_get_lowest_dev(dev, sk);
+		dev_hold(lowest_dev);
 	}
+	rcu_read_unlock();
 
-	dst_release(dst);
-
-	return netdev;
+	return lowest_dev;
 }
 
 static void destroy_record(struct tls_record_info *record)
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v1 net 8/8] mptcp: Use sk_dst_dev_rcu() in mptcp_active_enable().

[Kuniyuki Iwashima]

mptcp_active_enable() is called from subflow_finish_connect(),
which is icsk->icsk_af_ops->sk_rx_dst_set() and it's not always
under RCU.

Using sk_dst_get(sk)->dev could trigger UAF.

Also, mptcp_active_enable() forgot dst_release().

Let's use sk_dst_dev_rcu().

Fixes: 27069e7cb3d1 ("mptcp: disable active MPTCP in case of blackhole")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: Matthieu Baerts <matttbe@kernel.org>
Cc: Mat Martineau <martineau@kernel.org>
Cc: Geliang Tang <geliang@kernel.org>
---
 net/mptcp/ctrl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index fed40dae5583..f227415e8568 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -501,10 +501,13 @@ void mptcp_active_enable(struct sock *sk)
 	struct mptcp_pernet *pernet = mptcp_get_pernet(sock_net(sk));
 
 	if (atomic_read(&pernet->active_disable_times)) {
-		struct dst_entry *dst = sk_dst_get(sk);
+		struct net_device *dev;
 
-		if (dst && dst->dev && (dst->dev->flags & IFF_LOOPBACK))
+		rcu_read_lock();
+		dev = sk_dst_dev_rcu(sk);
+		if (dev && (dev->flags & IFF_LOOPBACK))
 			atomic_set(&pernet->active_disable_times, 0);
+		rcu_read_unlock();
 	}
 }
 
-- 
2.51.0.384.g4c02a37b29-goog

Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

[Eric Dumazet]

On Wed, Sep 10, 2025 at 8:06 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> tcp_fastopen_active_disable_ofo_check() is called from tcp_disconnect()
> or tcp_v4_destroy_sock(), so not under RCU nor RTNL.
>
> Using sk_dst_get(sk)->dev could trigger UAF.
>
> Let's use sk_dst_dev_rcu().
>
> Fixes: 4a6ce2b6f2ec ("net: introduce a new function dst_dev_put()")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
> Cc: Neal Cardwell <ncardwell@google.com>
> ---
>  net/ipv4/tcp_fastopen.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index f1884f0c9e52..de849b8c87ef 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -560,7 +560,6 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>         struct net_device *dev;
> -       struct dst_entry *dst;
>         struct sk_buff *skb;
>
>         if (!tp->syn_fastopen)
> @@ -576,11 +575,11 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
>                 }
>         } else if (tp->syn_fastopen_ch &&
>                    atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
> -               dst = sk_dst_get(sk);
> -               dev = dst ? dst_dev(dst) : NULL;
> +               rcu_read_lock();
> +               dev = sk_dst_dev_rcu(sk);
>                 if (!(dev && (dev->flags & IFF_LOOPBACK)))
>                         atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
> -               dst_release(dst);
> +               rcu_read_unlock();
>         }
>  }
>
> --
> 2.51.0.384.g4c02a37b29-goog
>

NACK. Please send a series against net-next as I did recently.

You will then discover :

commit b62a59c18b692f892dcb8109c1c2e653b2abc95c
Author: Eric Dumazet <edumazet@google.com>
Date:   Thu Aug 28 19:58:22 2025 +0000

    tcp: use dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check()

    Use RCU to avoid a pair of atomic operations and a potential
    UAF on dst_dev()->flags.

    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Reviewed-by: David Ahern <dsahern@kernel.org>
    Link: https://patch.msgid.link/20250828195823.3958522-8-edumazet@google.com
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index f1884f0c9e523d50b2d120175cc94bc40b489dfb..7d945a527daf093f87882c7949e21058ed6df1cc
100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -576,11 +576,12 @@ void
tcp_fastopen_active_disable_ofo_check(struct sock *sk)
                }
        } else if (tp->syn_fastopen_ch &&
                   atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
-               dst = sk_dst_get(sk);
-               dev = dst ? dst_dev(dst) : NULL;
+               rcu_read_lock();
+               dst = __sk_dst_get(sk);
+               dev = dst ? dst_dev_rcu(dst) : NULL;
                if (!(dev && (dev->flags & IFF_LOOPBACK)))

atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
-               dst_release(dst);
+               rcu_read_unlock();
        }
 }

Re: [PATCH v1 net 4/8] smc: Use sk_dst_dev_rcu() in smc_clc_prfx_match().

[Eric Dumazet]

On Wed, Sep 10, 2025 at 8:06 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> smc_clc_prfx_match() is called from smc_listen_work() and
> not under RCU nor RTNL.
>
> Using sk_dst_get(sk)->dev could trigger UAF.
>
> Let's use sk_dst_dev_rcu().
>
> Fixes: a046d57da19f ("smc: CLC handshake (incl. preparation steps)")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
> Cc: "D. Wythe" <alibuda@linux.alibaba.com>
> Cc: Dust Li <dust.li@linux.alibaba.com>
> Cc: Sidraya Jayagond <sidraya@linux.ibm.com>
> Cc: Wenjia Zhang <wenjia@linux.ibm.com>
> Cc: Mahanta Jambigi <mjambigi@linux.ibm.com>
> Cc: Tony Lu <tonylu@linux.alibaba.com>
> Cc: Wen Gu <guwen@linux.alibaba.com>
> Cc: Ursula Braun <ubraun@linux.vnet.ibm.com>
> ---
>  net/smc/smc_clc.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 9aa1d75d3079..5b4fe2c22879 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -655,26 +655,24 @@ static int smc_clc_prfx_match6_rcu(struct net_device *dev,
>  int smc_clc_prfx_match(struct socket *clcsock,
>                        struct smc_clc_msg_proposal_prefix *prop)
>  {
> -       struct dst_entry *dst = sk_dst_get(clcsock->sk);
> +       struct net_device *dev;
>         int rc;
>
> -       if (!dst) {
> -               rc = -ENOTCONN;
> -               goto out;
> -       }
> -       if (!dst->dev) {
> +       rcu_read_lock();
> +
> +       dev = sk_dst_dev_rcu(clcsock->sk);
> +       if (!dev) {
>                 rc = -ENODEV;
> -               goto out_rel;
> +               goto out;
>         }
> -       rcu_read_lock();
> +
>         if (!prop->ipv6_prefixes_cnt)
> -               rc = smc_clc_prfx_match4_rcu(dst->dev, prop);
> +               rc = smc_clc_prfx_match4_rcu(dev, prop);
>         else
> -               rc = smc_clc_prfx_match6_rcu(dst->dev, prop);
> -       rcu_read_unlock();
> -out_rel:
> -       dst_release(dst);
> +               rc = smc_clc_prfx_match6_rcu(dev, prop);
>  out:
> +       rcu_read_unlock();
> +
>         return rc;
>  }

I had in my tree a smaller patch without a new helper. I am not
convinced we need another one yet.

Re: [PATCH v1 net 3/8] smc: Use sk_dst_dev_rcu() in in smc_clc_prfx_set().

[Eric Dumazet]

On Wed, Sep 10, 2025 at 8:06 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> smc_clc_prfx_set() is called during connect() and not under RCU
> nor RTNL.
>
> Using sk_dst_get(sk)->dev could trigger UAF.
>
> Let's use sk_dst_get_rcu() under rcu_read_lock() after
> kernel_getsockname().
>
> While at it, we change the 1st arg of smc_clc_prfx_set[46]_rcu()
> not to touch dst there.
>
> Fixes: a046d57da19f ("smc: CLC handshake (incl. preparation steps)")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
> Cc: "D. Wythe" <alibuda@linux.alibaba.com>
> Cc: Dust Li <dust.li@linux.alibaba.com>
> Cc: Sidraya Jayagond <sidraya@linux.ibm.com>
> Cc: Wenjia Zhang <wenjia@linux.ibm.com>
> Cc: Mahanta Jambigi <mjambigi@linux.ibm.com>
> Cc: Tony Lu <tonylu@linux.alibaba.com>
> Cc: Wen Gu <guwen@linux.alibaba.com>
> Cc: Ursula Braun <ubraun@linux.vnet.ibm.com>
> ---
>  net/smc/smc_clc.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 08be56dfb3f2..9aa1d75d3079 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -509,10 +509,10 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool check_trl)
>  }
>
>  /* find ipv4 addr on device and get the prefix len, fill CLC proposal msg */
> -static int smc_clc_prfx_set4_rcu(struct dst_entry *dst, __be32 ipv4,
> +static int smc_clc_prfx_set4_rcu(struct net_device *dev, __be32 ipv4,
>                                  struct smc_clc_msg_proposal_prefix *prop)
>  {
> -       struct in_device *in_dev = __in_dev_get_rcu(dst->dev);
> +       struct in_device *in_dev = __in_dev_get_rcu(dev);
>         const struct in_ifaddr *ifa;
>
>         if (!in_dev)
> @@ -530,12 +530,12 @@ static int smc_clc_prfx_set4_rcu(struct dst_entry *dst, __be32 ipv4,
>  }
>
>  /* fill CLC proposal msg with ipv6 prefixes from device */
> -static int smc_clc_prfx_set6_rcu(struct dst_entry *dst,
> +static int smc_clc_prfx_set6_rcu(struct net_device *dev,
>                                  struct smc_clc_msg_proposal_prefix *prop,
>                                  struct smc_clc_ipv6_prefix *ipv6_prfx)
>  {
>  #if IS_ENABLED(CONFIG_IPV6)
> -       struct inet6_dev *in6_dev = __in6_dev_get(dst->dev);
> +       struct inet6_dev *in6_dev = __in6_dev_get(dev);
>         struct inet6_ifaddr *ifa;
>         int cnt = 0;
>
> @@ -564,41 +564,42 @@ static int smc_clc_prfx_set(struct socket *clcsock,
>                             struct smc_clc_msg_proposal_prefix *prop,
>                             struct smc_clc_ipv6_prefix *ipv6_prfx)
>  {
> -       struct dst_entry *dst = sk_dst_get(clcsock->sk);
>         struct sockaddr_storage addrs;
>         struct sockaddr_in6 *addr6;
>         struct sockaddr_in *addr;
> +       struct net_device *dev;
>         int rc = -ENOENT;
>
> -       if (!dst) {
> -               rc = -ENOTCONN;
> -               goto out;
> -       }
> -       if (!dst->dev) {
> -               rc = -ENODEV;
> -               goto out_rel;
> -       }
>         /* get address to which the internal TCP socket is bound */
>         if (kernel_getsockname(clcsock, (struct sockaddr *)&addrs) < 0)
> -               goto out_rel;
> +               goto out;
> +
>         /* analyze IP specific data of net_device belonging to TCP socket */
>         addr6 = (struct sockaddr_in6 *)&addrs;
> +
>         rcu_read_lock();
> +
> +       dev = sk_dst_dev_rcu(clcsock->sk);
> +       if (!dev) {
> +               rc = -ENODEV;
> +               goto out_unlock;
> +       }
> +
>         if (addrs.ss_family == PF_INET) {
>                 /* IPv4 */
>                 addr = (struct sockaddr_in *)&addrs;
> -               rc = smc_clc_prfx_set4_rcu(dst, addr->sin_addr.s_addr, prop);
> +               rc = smc_clc_prfx_set4_rcu(dev, addr->sin_addr.s_addr, prop);
>         } else if (ipv6_addr_v4mapped(&addr6->sin6_addr)) {
>                 /* mapped IPv4 address - peer is IPv4 only */
> -               rc = smc_clc_prfx_set4_rcu(dst, addr6->sin6_addr.s6_addr32[3],
> +               rc = smc_clc_prfx_set4_rcu(dev, addr6->sin6_addr.s6_addr32[3],
>                                            prop);
>         } else {
>                 /* IPv6 */
> -               rc = smc_clc_prfx_set6_rcu(dst, prop, ipv6_prfx);
> +               rc = smc_clc_prfx_set6_rcu(dev, prop, ipv6_prfx);
>         }
> +
> +out_unlock:
>         rcu_read_unlock();
> -out_rel:
> -       dst_release(dst);
>  out:
>         return rc;
>  }
> --
> 2.51.0.384.g4c02a37b29-goog
>

Same comment, I had a patch to fix this without a new helper.

We have hundreds of dst->dev places to fix, very few  sk_dst_get().

This is why I think sk_dst_dev_rcu() is not necessary.

Re: [PATCH v1 net 8/8] mptcp: Use sk_dst_dev_rcu() in mptcp_active_enable().

[Matthieu Baerts]

Hi Kuniyuki,

On 11/09/2025 05:05, Kuniyuki Iwashima wrote:
> mptcp_active_enable() is called from subflow_finish_connect(),
> which is icsk->icsk_af_ops->sk_rx_dst_set() and it's not always
> under RCU.
> 
> Using sk_dst_get(sk)->dev could trigger UAF.
> 
> Also, mptcp_active_enable() forgot dst_release().

Oops! Good catch!

> Let's use sk_dst_dev_rcu().

Thank you! The patch looks good to me, but I also read Eric's replies.

If the patches are sent to net-next, will they still have the 'Fixes'
tag? If not (but I guess they will), could we have at least have a
dedicated patch to add the missing 'dst_release()' please?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

[Kuniyuki Iwashima]

On Wed, Sep 10, 2025 at 10:45 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Sep 10, 2025 at 8:06 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > tcp_fastopen_active_disable_ofo_check() is called from tcp_disconnect()
> > or tcp_v4_destroy_sock(), so not under RCU nor RTNL.
> >
> > Using sk_dst_get(sk)->dev could trigger UAF.
> >
> > Let's use sk_dst_dev_rcu().
> >
> > Fixes: 4a6ce2b6f2ec ("net: introduce a new function dst_dev_put()")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> > Cc: Neal Cardwell <ncardwell@google.com>
> > ---
> >  net/ipv4/tcp_fastopen.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> > index f1884f0c9e52..de849b8c87ef 100644
> > --- a/net/ipv4/tcp_fastopen.c
> > +++ b/net/ipv4/tcp_fastopen.c
> > @@ -560,7 +560,6 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
> >  {
> >         struct tcp_sock *tp = tcp_sk(sk);
> >         struct net_device *dev;
> > -       struct dst_entry *dst;
> >         struct sk_buff *skb;
> >
> >         if (!tp->syn_fastopen)
> > @@ -576,11 +575,11 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
> >                 }
> >         } else if (tp->syn_fastopen_ch &&
> >                    atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
> > -               dst = sk_dst_get(sk);
> > -               dev = dst ? dst_dev(dst) : NULL;
> > +               rcu_read_lock();
> > +               dev = sk_dst_dev_rcu(sk);
> >                 if (!(dev && (dev->flags & IFF_LOOPBACK)))
> >                         atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
> > -               dst_release(dst);
> > +               rcu_read_unlock();
> >         }
> >  }
> >
> > --
> > 2.51.0.384.g4c02a37b29-goog
> >
>
> NACK. Please send a series against net-next as I did recently.
>
> You will then discover :
>

Sorry, I missed this one.  I'll drop this patch and send the
series to net-next.


> commit b62a59c18b692f892dcb8109c1c2e653b2abc95c
> Author: Eric Dumazet <edumazet@google.com>
> Date:   Thu Aug 28 19:58:22 2025 +0000
>
>     tcp: use dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check()
>
>     Use RCU to avoid a pair of atomic operations and a potential
>     UAF on dst_dev()->flags.
>
>     Signed-off-by: Eric Dumazet <edumazet@google.com>
>     Reviewed-by: David Ahern <dsahern@kernel.org>
>     Link: https://patch.msgid.link/20250828195823.3958522-8-edumazet@google.com
>     Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index f1884f0c9e523d50b2d120175cc94bc40b489dfb..7d945a527daf093f87882c7949e21058ed6df1cc
> 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -576,11 +576,12 @@ void
> tcp_fastopen_active_disable_ofo_check(struct sock *sk)
>                 }
>         } else if (tp->syn_fastopen_ch &&
>                    atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
> -               dst = sk_dst_get(sk);
> -               dev = dst ? dst_dev(dst) : NULL;
> +               rcu_read_lock();
> +               dst = __sk_dst_get(sk);
> +               dev = dst ? dst_dev_rcu(dst) : NULL;
>                 if (!(dev && (dev->flags & IFF_LOOPBACK)))
>
> atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
> -               dst_release(dst);
> +               rcu_read_unlock();
>         }
>  }

Re: [PATCH v1 net 3/8] smc: Use sk_dst_dev_rcu() in in smc_clc_prfx_set().

[Kuniyuki Iwashima]

On Wed, Sep 10, 2025 at 11:28 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Sep 10, 2025 at 8:06 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > smc_clc_prfx_set() is called during connect() and not under RCU
> > nor RTNL.
> >
> > Using sk_dst_get(sk)->dev could trigger UAF.
> >
> > Let's use sk_dst_get_rcu() under rcu_read_lock() after
> > kernel_getsockname().
> >
> > While at it, we change the 1st arg of smc_clc_prfx_set[46]_rcu()
> > not to touch dst there.
> >
> > Fixes: a046d57da19f ("smc: CLC handshake (incl. preparation steps)")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> > Cc: "D. Wythe" <alibuda@linux.alibaba.com>
> > Cc: Dust Li <dust.li@linux.alibaba.com>
> > Cc: Sidraya Jayagond <sidraya@linux.ibm.com>
> > Cc: Wenjia Zhang <wenjia@linux.ibm.com>
> > Cc: Mahanta Jambigi <mjambigi@linux.ibm.com>
> > Cc: Tony Lu <tonylu@linux.alibaba.com>
> > Cc: Wen Gu <guwen@linux.alibaba.com>
> > Cc: Ursula Braun <ubraun@linux.vnet.ibm.com>
> > ---
> >  net/smc/smc_clc.c | 39 ++++++++++++++++++++-------------------
> >  1 file changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> > index 08be56dfb3f2..9aa1d75d3079 100644
> > --- a/net/smc/smc_clc.c
> > +++ b/net/smc/smc_clc.c
> > @@ -509,10 +509,10 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool check_trl)
> >  }
> >
> >  /* find ipv4 addr on device and get the prefix len, fill CLC proposal msg */
> > -static int smc_clc_prfx_set4_rcu(struct dst_entry *dst, __be32 ipv4,
> > +static int smc_clc_prfx_set4_rcu(struct net_device *dev, __be32 ipv4,
> >                                  struct smc_clc_msg_proposal_prefix *prop)
> >  {
> > -       struct in_device *in_dev = __in_dev_get_rcu(dst->dev);
> > +       struct in_device *in_dev = __in_dev_get_rcu(dev);
> >         const struct in_ifaddr *ifa;
> >
> >         if (!in_dev)
> > @@ -530,12 +530,12 @@ static int smc_clc_prfx_set4_rcu(struct dst_entry *dst, __be32 ipv4,
> >  }
> >
> >  /* fill CLC proposal msg with ipv6 prefixes from device */
> > -static int smc_clc_prfx_set6_rcu(struct dst_entry *dst,
> > +static int smc_clc_prfx_set6_rcu(struct net_device *dev,
> >                                  struct smc_clc_msg_proposal_prefix *prop,
> >                                  struct smc_clc_ipv6_prefix *ipv6_prfx)
> >  {
> >  #if IS_ENABLED(CONFIG_IPV6)
> > -       struct inet6_dev *in6_dev = __in6_dev_get(dst->dev);
> > +       struct inet6_dev *in6_dev = __in6_dev_get(dev);
> >         struct inet6_ifaddr *ifa;
> >         int cnt = 0;
> >
> > @@ -564,41 +564,42 @@ static int smc_clc_prfx_set(struct socket *clcsock,
> >                             struct smc_clc_msg_proposal_prefix *prop,
> >                             struct smc_clc_ipv6_prefix *ipv6_prfx)
> >  {
> > -       struct dst_entry *dst = sk_dst_get(clcsock->sk);
> >         struct sockaddr_storage addrs;
> >         struct sockaddr_in6 *addr6;
> >         struct sockaddr_in *addr;
> > +       struct net_device *dev;
> >         int rc = -ENOENT;
> >
> > -       if (!dst) {
> > -               rc = -ENOTCONN;
> > -               goto out;
> > -       }
> > -       if (!dst->dev) {
> > -               rc = -ENODEV;
> > -               goto out_rel;
> > -       }
> >         /* get address to which the internal TCP socket is bound */
> >         if (kernel_getsockname(clcsock, (struct sockaddr *)&addrs) < 0)
> > -               goto out_rel;
> > +               goto out;
> > +
> >         /* analyze IP specific data of net_device belonging to TCP socket */
> >         addr6 = (struct sockaddr_in6 *)&addrs;
> > +
> >         rcu_read_lock();
> > +
> > +       dev = sk_dst_dev_rcu(clcsock->sk);
> > +       if (!dev) {
> > +               rc = -ENODEV;
> > +               goto out_unlock;
> > +       }
> > +
> >         if (addrs.ss_family == PF_INET) {
> >                 /* IPv4 */
> >                 addr = (struct sockaddr_in *)&addrs;
> > -               rc = smc_clc_prfx_set4_rcu(dst, addr->sin_addr.s_addr, prop);
> > +               rc = smc_clc_prfx_set4_rcu(dev, addr->sin_addr.s_addr, prop);
> >         } else if (ipv6_addr_v4mapped(&addr6->sin6_addr)) {
> >                 /* mapped IPv4 address - peer is IPv4 only */
> > -               rc = smc_clc_prfx_set4_rcu(dst, addr6->sin6_addr.s6_addr32[3],
> > +               rc = smc_clc_prfx_set4_rcu(dev, addr6->sin6_addr.s6_addr32[3],
> >                                            prop);
> >         } else {
> >                 /* IPv6 */
> > -               rc = smc_clc_prfx_set6_rcu(dst, prop, ipv6_prfx);
> > +               rc = smc_clc_prfx_set6_rcu(dev, prop, ipv6_prfx);
> >         }
> > +
> > +out_unlock:
> >         rcu_read_unlock();
> > -out_rel:
> > -       dst_release(dst);
> >  out:
> >         return rc;
> >  }
> > --
> > 2.51.0.384.g4c02a37b29-goog
> >
>
> Same comment, I had a patch to fix this without a new helper.
>
> We have hundreds of dst->dev places to fix, very few  sk_dst_get().
>
> This is why I think sk_dst_dev_rcu() is not necessary.

I added helpers for new users not to read sk_dst_get()->dst
directly, but the point is fair.  Maybe we can convert all users
to __sk_dst_get() or remove rcu_read_lock() in sk_dst_get().

Re: [PATCH v1 net 8/8] mptcp: Use sk_dst_dev_rcu() in mptcp_active_enable().

[Kuniyuki Iwashima]

On Thu, Sep 11, 2025 at 1:48 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Kuniyuki,
>
> On 11/09/2025 05:05, Kuniyuki Iwashima wrote:
> > mptcp_active_enable() is called from subflow_finish_connect(),
> > which is icsk->icsk_af_ops->sk_rx_dst_set() and it's not always
> > under RCU.
> >
> > Using sk_dst_get(sk)->dev could trigger UAF.
> >
> > Also, mptcp_active_enable() forgot dst_release().
>
> Oops! Good catch!
>
> > Let's use sk_dst_dev_rcu().
>
> Thank you! The patch looks good to me, but I also read Eric's replies.
>
> If the patches are sent to net-next, will they still have the 'Fixes'
> tag? If not (but I guess they will), could we have at least have a
> dedicated patch to add the missing 'dst_release()' please?

Sure, I'll split the patch into two and keep Fixes: for the
dst_release() one.

Re: [PATCH v1 net 8/8] mptcp: Use sk_dst_dev_rcu() in mptcp_active_enable().

[Matthieu Baerts]

On 11/09/2025 18:58, Kuniyuki Iwashima wrote:
> On Thu, Sep 11, 2025 at 1:48 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> Hi Kuniyuki,
>>
>> On 11/09/2025 05:05, Kuniyuki Iwashima wrote:
>>> mptcp_active_enable() is called from subflow_finish_connect(),
>>> which is icsk->icsk_af_ops->sk_rx_dst_set() and it's not always
>>> under RCU.
>>>
>>> Using sk_dst_get(sk)->dev could trigger UAF.
>>>
>>> Also, mptcp_active_enable() forgot dst_release().
>>
>> Oops! Good catch!
>>
>>> Let's use sk_dst_dev_rcu().
>>
>> Thank you! The patch looks good to me, but I also read Eric's replies.
>>
>> If the patches are sent to net-next, will they still have the 'Fixes'
>> tag? If not (but I guess they will), could we have at least have a
>> dedicated patch to add the missing 'dst_release()' please?
> 
> Sure, I'll split the patch into two and keep Fixes: for the
> dst_release() one.

Great, thank you!

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

[Eric Dumazet]

On Thu, Sep 11, 2025 at 9:53 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:

>
> Sorry, I missed this one.  I'll drop this patch and send the
> series to net-next.
>

No problem.

Main reason for us to use net-next is that adding lockdep can bring new issues,
thus we have more time to polish patches before hitting more users.

We also can iterate faster, not having to wait one week for net fan in.

Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

[Kuniyuki Iwashima]

On Thu, Sep 11, 2025 at 10:07 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Sep 11, 2025 at 9:53 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> >
> > Sorry, I missed this one.  I'll drop this patch and send the
> > series to net-next.
> >
>
> No problem.
>
> Main reason for us to use net-next is that adding lockdep can bring new issues,
> thus we have more time to polish patches before hitting more users.
>
> We also can iterate faster, not having to wait one week for net fan in.

Thank you for explaining the reason.  That makes sense.

Then should we keep or drop Fixes: tag ?  At least it will not land
on the mainline and thus stable until the next merge window.

Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

[Eric Dumazet]

On Thu, Sep 11, 2025 at 10:24 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> On Thu, Sep 11, 2025 at 10:07 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Sep 11, 2025 at 9:53 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > >
> > > Sorry, I missed this one.  I'll drop this patch and send the
> > > series to net-next.
> > >
> >
> > No problem.
> >
> > Main reason for us to use net-next is that adding lockdep can bring new issues,
> > thus we have more time to polish patches before hitting more users.
> >
> > We also can iterate faster, not having to wait one week for net fan in.
>
> Thank you for explaining the reason.  That makes sense.
>
> Then should we keep or drop Fixes: tag ?  At least it will not land
> on the mainline and thus stable until the next merge window.

Fixes: tags are fine.

I usually do not add them for all patches, because I know that
everything will be
backported anyway because of dependencies.

Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

[Kuniyuki Iwashima]

On Thu, Sep 11, 2025 at 10:35 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Sep 11, 2025 at 10:24 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > On Thu, Sep 11, 2025 at 10:07 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Thu, Sep 11, 2025 at 9:53 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> > >
> > > >
> > > > Sorry, I missed this one.  I'll drop this patch and send the
> > > > series to net-next.
> > > >
> > >
> > > No problem.
> > >
> > > Main reason for us to use net-next is that adding lockdep can bring new issues,
> > > thus we have more time to polish patches before hitting more users.
> > >
> > > We also can iterate faster, not having to wait one week for net fan in.
> >
> > Thank you for explaining the reason.  That makes sense.
> >
> > Then should we keep or drop Fixes: tag ?  At least it will not land
> > on the mainline and thus stable until the next merge window.
>
> Fixes: tags are fine.
>
> I usually do not add them for all patches, because I know that
> everything will be
> backported anyway because of dependencies.

Thanks for clarification.  I'll keep them this time.

Re: [PATCH v1 net 2/8] smc: Fix use-after-free in __pnet_find_base_ndev().

[Mahanta Jambigi]

On 11/09/25 8:35 am, Kuniyuki Iwashima wrote:
> -	return;
> +	dev = sk_dst_dev_get(sk);
> +	if (dev) {
> +		smc_pnet_find_roce_by_pnetid(dev, ini);
> +		dev_put(dev);

Nit: Should we use netdev_put() along with netdev_hold() in
sk_dst_dev_get()? Same query for other places where we are using dev_put().

Re: [PATCH v1 net 2/8] smc: Fix use-after-free in __pnet_find_base_ndev().

[Kuniyuki Iwashima]

On Fri, Sep 12, 2025 at 12:55 AM Mahanta Jambigi <mjambigi@linux.ibm.com> wrote:
>
> On 11/09/25 8:35 am, Kuniyuki Iwashima wrote:
> > -     return;
> > +     dev = sk_dst_dev_get(sk);
> > +     if (dev) {
> > +             smc_pnet_find_roce_by_pnetid(dev, ini);
> > +             dev_put(dev);
>
> Nit: Should we use netdev_put() along with netdev_hold() in
> sk_dst_dev_get()? Same query for other places where we are using dev_put().

I think dev_hold()/dev_put() is fine for fix and temporary use.