[PATCH net v2 1/2] ipv6: fix cached route reuse for SO_MARK control messages

[PATCH net v2 1/2] ipv6: fix cached route reuse for SO_MARK control messages

[Xia Yan]

A connected IPv6 UDP or ping socket can reuse a route selected for the
socket mark when an SO_MARK control message supplies a different mark. A
packet can then bypass a prohibit rule for its per-message mark instead
of failing with EACCES.

ip6_sk_dst_lookup_flow() consults the socket dst cache even when
connected is false. That argument only controls whether a new route is
cached. The cache check does not compare the mark used for the lookup.

Record whether SO_MARK was supplied by the control messages, including
an explicit zero. For UDP and ping sends with that flag, call
ip6_dst_lookup_flow() directly. Keep the cache handling for sends
without SO_MARK unchanged.

The concurrent SO_MARK case was identified in the review of v1.

The issue was reproduced on UML with connected sockets and mark-based
routing. A separate test kernel paused the sender after it acquired a
cached dst reference; another thread then changed the socket mark to the
per-message mark. The old code still sent the packet. The fix bypasses
that cache path and returns EACCES. The 50 UDP/ping route checks and all
54 cmsg_so_mark.sh cases pass with the fix.

Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
Cc: stable@vger.kernel.org
Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
---
v2:
  - Replace the current sk_mark comparison with a flag recording whether
    SO_MARK was supplied, and bypass the cache for those UDP/ping sends.
  - Add connected-socket regression coverage in patch 2.

v1: https://lore.kernel.org/netdev/20261030141435.779302-1-yanxia@cmss.chinamobile.com/T/
Review: https://lore.kernel.org/netdev/179371875338.488856.4546363285391899557@kernel.org/

Tested on net cb284914e749388eef402938bdb3115fe1f6d337 with GCC 12.3.1:
  - UML normal: baseline 41/50, v1 and v2 50/50; the same extended
    cmsg_so_mark.sh has 2/54 failures on baseline and passes on v1/v2.
  - Test-only pause: baseline/v1 retain the old cached dst and send after
    the socket mark changes; v2 skips the cache helper and gets EACCES.
  - x86_64 W=1: full vmlinux, bzImage and modules builds pass on baseline
    and v2 using configs derived from allmodconfig and allyesconfig.
    WERROR, KASAN and KCSAN are disabled; KUNIT_ALL_TESTS is also disabled
    for allyesconfig. The unmodified configs fail on the same existing
    GCC 12.3.1 Werror diagnostics on both sides. Derived-build diagnostics
    are identical on both sides, including their occurrence counts.


 include/net/ipv6.h  | 1 +
 net/ipv6/datagram.c | 2 ++
 net/ipv6/ping.c     | 5 ++++-
 net/ipv6/udp.c      | 5 ++++-
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -353,6 +353,7 @@
 	__s16 tclass;
 	__u16 gso_size;
 	__s8  dontfrag;
+	bool mark_set;
 	struct ipv6_txoptions *opt;
 };
 
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -819,6 +819,8 @@
 			err = __sock_cmsg_send(sk, cmsg, &ipc6->sockc);
 			if (err)
 				return err;
+			if (cmsg->cmsg_type == SO_MARK)
+				ipc6->mark_set = true;
 			continue;
 		}
 
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1693,7 +1693,10 @@
 
 	fl6->flowlabel = ip6_make_flowinfo(ipc6.tclass, fl6->flowlabel);
 
-	dst = ip6_sk_dst_lookup_flow(sk, fl6, final_p, connected);
+	if (ipc6.mark_set)
+		dst = ip6_dst_lookup_flow(sock_net(sk), sk, fl6, final_p);
+	else
+		dst = ip6_sk_dst_lookup_flow(sk, fl6, final_p, connected);
 	if (IS_ERR(dst)) {
 		err = PTR_ERR(dst);
 		dst = NULL;
diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -148,7 +148,10 @@
 
 	fl6.flowlabel = ip6_make_flowinfo(ipc6.tclass, fl6.flowlabel);
 
-	dst = ip6_sk_dst_lookup_flow(sk, &fl6, daddr, false);
+	if (ipc6.mark_set)
+		dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl6, daddr);
+	else
+		dst = ip6_sk_dst_lookup_flow(sk, &fl6, daddr, false);
 	if (IS_ERR(dst))
 		return PTR_ERR(dst);
 	rt = dst_rt6_info(dst);

[PATCH net v2 2/2] selftests: net: exercise SO_MARK on connected sockets

[Xia Yan]

cmsg_so_mark.sh sends to an explicit destination without connecting the
socket, so it does not exercise the route cached by connect(). This left
the IPv6 SO_MARK cache bug uncovered.

Add a cmsg_sender option to connect before sending and omit msg_name in
that mode. Keep the existing cases and add connected IPv4 and IPv6 UDP,
ping and raw sends with a per-message mark equal to the socket mark,
prohibited by a policy rule, or explicitly zero.

On UML, the added IPv6 UDP and ping cases with the prohibited mark fail
before the kernel fix. With the fix, all 54 cases pass.

Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
---
v2:
  - New patch; add 18 connected-socket cases alongside the existing 36.

The test gap was noted during automated review of v1:
https://lore.kernel.org/netdev/179371875338.488856.4546363285391899557@kernel.org/

Tested the same selftest on baseline, v1 and v2 UML kernels: baseline
fails 2/54 cases; v1/v2 pass all 54. Both versions of cmsg_sender build
with GCC 12.3.1. ShellCheck 0.11.0 reports the same 13 existing SC2086
diagnostics before and after the change.

 tools/testing/selftests/net/cmsg_sender.c   | 21 ++++++++++++++++++---
 tools/testing/selftests/net/cmsg_so_mark.sh | 16 ++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c
--- a/tools/testing/selftests/net/cmsg_sender.c
+++ b/tools/testing/selftests/net/cmsg_sender.c
@@ -34,6 +34,7 @@
 	ERN_CMSG_RD,
 	ERN_CMSG_RCV,
 	ERN_SEND_MORE,
+	ERN_CONNECT,
 };
 
 struct option_cmsg_u32 {
@@ -48,6 +49,7 @@
 	unsigned int size;
 	unsigned int num_pkt;
 	bool msg_more;
+	bool connected;
 	struct {
 		unsigned int mark;
 		unsigned int dontfrag;
@@ -93,6 +95,7 @@
 	printf("Usage: %s [opts] <dst host> <dst port / service>\n", bin);
 	printf("Options:\n"
 	       "\t\t-s      Silent send() failures\n"
+	       "\t\t-k      Connect before sending\n"
 	       "\t\t-S      send() size\n"
 	       "\t\t-4/-6   Force IPv4 / IPv6 only\n"
 	       "\t\t-p prot Socket protocol\n"
@@ -121,8 +124,12 @@
 {
 	int o;
 
-	while ((o = getopt(argc, argv, "46sS:p:P:m:M:n:d:tf:F:c:C:l:L:H:Q:")) != -1) {
+	while ((o = getopt(argc, argv,
+			   "46ksS:p:P:m:M:n:d:tf:F:c:C:l:L:H:Q:")) != -1) {
 		switch (o) {
+		case 'k':
+			opt.connected = true;
+			break;
 		case 's':
 			opt.silent_send = true;
 			break;
@@ -521,6 +528,12 @@
 
 	ca_set_sockopts(fd);
 
+	if (opt.connected && connect(fd, ai->ai_addr, ai->ai_addrlen)) {
+		fprintf(stderr, "Can't connect socket: %s\n", strerror(errno));
+		err = ERN_CONNECT;
+		goto err_out;
+	}
+
 	if (clock_gettime(CLOCK_REALTIME, &time_start_real))
 		error(ERN_GETTIME, errno, "gettime REALTIME");
 	if (clock_gettime(CLOCK_MONOTONIC, &time_start_mono))
@@ -530,8 +543,10 @@
 	iov[0].iov_len = opt.size;
 
 	memset(&msg, 0, sizeof(msg));
-	msg.msg_name = ai->ai_addr;
-	msg.msg_namelen = ai->ai_addrlen;
+	if (!opt.connected) {
+		msg.msg_name = ai->ai_addr;
+		msg.msg_namelen = ai->ai_addrlen;
+	}
 	msg.msg_iov = iov;
 	msg.msg_iovlen = 1;
 
diff --git a/tools/testing/selftests/net/cmsg_so_mark.sh b/tools/testing/selftests/net/cmsg_so_mark.sh
--- a/tools/testing/selftests/net/cmsg_so_mark.sh
+++ b/tools/testing/selftests/net/cmsg_so_mark.sh
@@ -68,6 +68,22 @@
     done
 done
 
+# Check per-message marks after connect() has populated the route cache.
+for i in 4 6; do
+    [ "$i" = 4 ] && TGT=$TGT4 || TGT=$TGT6
+
+    for p in u i r; do
+	for mark in "$((MARK + 1))" "$MARK" 0; do
+	    expected=0
+	    [ "$mark" -eq "$MARK" ] && expected=1
+
+	    ip netns exec "$NS" ./cmsg_sender -"$i" -p "$p" -k \
+		-M "$((MARK + 1))" -m "$mark" -s "$TGT" 1234
+	    check_result "$?" "$expected" "IPv$i $p connected mark $mark"
+	done
+    done
+done
+
 # Summary
 if [ $BAD -ne 0 ]; then
     echo "FAIL - $BAD/$TOTAL cases failed"

[PATCH net v2 1/2] ipv6: fix cached route reuse for SO_MARK control messages

[Xia Yan]

A connected IPv6 UDP or ping socket can reuse a route selected for the
socket mark when an SO_MARK control message supplies a different mark. A
packet can then bypass a prohibit rule for its per-message mark instead
of failing with EACCES.

ip6_sk_dst_lookup_flow() consults the socket dst cache even when
connected is false. That argument only controls whether a new route is
cached. The cache check does not compare the mark used for the lookup.

Record whether SO_MARK was supplied by the control messages, including
an explicit zero. For UDP and ping sends with that flag, call
ip6_dst_lookup_flow() directly. Keep the cache handling for sends
without SO_MARK unchanged.

The concurrent SO_MARK case was identified in the review of v1.

The issue was reproduced on UML with connected sockets and mark-based
routing. A separate test kernel paused the sender after it acquired a
cached dst reference; another thread then changed the socket mark to the
per-message mark. The old code still sent the packet. The fix bypasses
that cache path and returns EACCES. The 50 UDP/ping route checks and all
54 cmsg_so_mark.sh cases pass with the fix.

Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
Cc: stable@vger.kernel.org
Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
---
v2:
  - Replace the current sk_mark comparison with a flag recording whether
    SO_MARK was supplied, and bypass the cache for those UDP/ping sends.
  - Add connected-socket regression coverage in patch 2.

v1: https://lore.kernel.org/netdev/20261030141435.779302-1-yanxia@cmss.chinamobile.com/T/
Review: https://lore.kernel.org/netdev/179371875338.488856.4546363285391899557@kernel.org/

Tested on net cb284914e749388eef402938bdb3115fe1f6d337 with GCC 12.3.1:
  - UML normal: baseline 41/50, v1 and v2 50/50; the same extended
    cmsg_so_mark.sh has 2/54 failures on baseline and passes on v1/v2.
  - Test-only pause: baseline/v1 retain the old cached dst and send after
    the socket mark changes; v2 skips the cache helper and gets EACCES.
  - x86_64 W=1: full vmlinux, bzImage and modules builds pass on baseline
    and v2 using configs derived from allmodconfig and allyesconfig.
    WERROR, KASAN and KCSAN are disabled; KUNIT_ALL_TESTS is also disabled
    for allyesconfig. The unmodified configs fail on the same existing
    GCC 12.3.1 Werror diagnostics on both sides. Derived-build diagnostics
    are identical on both sides, including their occurrence counts.


 include/net/ipv6.h  | 1 +
 net/ipv6/datagram.c | 2 ++
 net/ipv6/ping.c     | 5 ++++-
 net/ipv6/udp.c      | 5 ++++-
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -353,6 +353,7 @@
 	__s16 tclass;
 	__u16 gso_size;
 	__s8  dontfrag;
+	bool mark_set;
 	struct ipv6_txoptions *opt;
 };
 
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -819,6 +819,8 @@
 			err = __sock_cmsg_send(sk, cmsg, &ipc6->sockc);
 			if (err)
 				return err;
+			if (cmsg->cmsg_type == SO_MARK)
+				ipc6->mark_set = true;
 			continue;
 		}
 
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1693,7 +1693,10 @@
 
 	fl6->flowlabel = ip6_make_flowinfo(ipc6.tclass, fl6->flowlabel);
 
-	dst = ip6_sk_dst_lookup_flow(sk, fl6, final_p, connected);
+	if (ipc6.mark_set)
+		dst = ip6_dst_lookup_flow(sock_net(sk), sk, fl6, final_p);
+	else
+		dst = ip6_sk_dst_lookup_flow(sk, fl6, final_p, connected);
 	if (IS_ERR(dst)) {
 		err = PTR_ERR(dst);
 		dst = NULL;
diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -148,7 +148,10 @@
 
 	fl6.flowlabel = ip6_make_flowinfo(ipc6.tclass, fl6.flowlabel);
 
-	dst = ip6_sk_dst_lookup_flow(sk, &fl6, daddr, false);
+	if (ipc6.mark_set)
+		dst = ip6_dst_lookup_flow(sock_net(sk), sk, &fl6, daddr);
+	else
+		dst = ip6_sk_dst_lookup_flow(sk, &fl6, daddr, false);
 	if (IS_ERR(dst))
 		return PTR_ERR(dst);
 	rt = dst_rt6_info(dst);

[PATCH net v2 2/2] selftests: net: exercise SO_MARK on connected sockets

[Xia Yan]

cmsg_so_mark.sh sends to an explicit destination without connecting the
socket, so it does not exercise the route cached by connect(). This left
the IPv6 SO_MARK cache bug uncovered.

Add a cmsg_sender option to connect before sending and omit msg_name in
that mode. Keep the existing cases and add connected IPv4 and IPv6 UDP,
ping and raw sends with a per-message mark equal to the socket mark,
prohibited by a policy rule, or explicitly zero.

On UML, the added IPv6 UDP and ping cases with the prohibited mark fail
before the kernel fix. With the fix, all 54 cases pass.

Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
---
v2:
  - New patch; add 18 connected-socket cases alongside the existing 36.

The test gap was noted during automated review of v1:
https://lore.kernel.org/netdev/179371875338.488856.4546363285391899557@kernel.org/

Tested the same selftest on baseline, v1 and v2 UML kernels: baseline
fails 2/54 cases; v1/v2 pass all 54. Both versions of cmsg_sender build
with GCC 12.3.1. ShellCheck 0.11.0 reports the same 13 existing SC2086
diagnostics before and after the change.

 tools/testing/selftests/net/cmsg_sender.c   | 21 ++++++++++++++++++---
 tools/testing/selftests/net/cmsg_so_mark.sh | 16 ++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c
--- a/tools/testing/selftests/net/cmsg_sender.c
+++ b/tools/testing/selftests/net/cmsg_sender.c
@@ -34,6 +34,7 @@
 	ERN_CMSG_RD,
 	ERN_CMSG_RCV,
 	ERN_SEND_MORE,
+	ERN_CONNECT,
 };
 
 struct option_cmsg_u32 {
@@ -48,6 +49,7 @@
 	unsigned int size;
 	unsigned int num_pkt;
 	bool msg_more;
+	bool connected;
 	struct {
 		unsigned int mark;
 		unsigned int dontfrag;
@@ -93,6 +95,7 @@
 	printf("Usage: %s [opts] <dst host> <dst port / service>\n", bin);
 	printf("Options:\n"
 	       "\t\t-s      Silent send() failures\n"
+	       "\t\t-k      Connect before sending\n"
 	       "\t\t-S      send() size\n"
 	       "\t\t-4/-6   Force IPv4 / IPv6 only\n"
 	       "\t\t-p prot Socket protocol\n"
@@ -121,8 +124,12 @@
 {
 	int o;
 
-	while ((o = getopt(argc, argv, "46sS:p:P:m:M:n:d:tf:F:c:C:l:L:H:Q:")) != -1) {
+	while ((o = getopt(argc, argv,
+			   "46ksS:p:P:m:M:n:d:tf:F:c:C:l:L:H:Q:")) != -1) {
 		switch (o) {
+		case 'k':
+			opt.connected = true;
+			break;
 		case 's':
 			opt.silent_send = true;
 			break;
@@ -521,6 +528,12 @@
 
 	ca_set_sockopts(fd);
 
+	if (opt.connected && connect(fd, ai->ai_addr, ai->ai_addrlen)) {
+		fprintf(stderr, "Can't connect socket: %s\n", strerror(errno));
+		err = ERN_CONNECT;
+		goto err_out;
+	}
+
 	if (clock_gettime(CLOCK_REALTIME, &time_start_real))
 		error(ERN_GETTIME, errno, "gettime REALTIME");
 	if (clock_gettime(CLOCK_MONOTONIC, &time_start_mono))
@@ -530,8 +543,10 @@
 	iov[0].iov_len = opt.size;
 
 	memset(&msg, 0, sizeof(msg));
-	msg.msg_name = ai->ai_addr;
-	msg.msg_namelen = ai->ai_addrlen;
+	if (!opt.connected) {
+		msg.msg_name = ai->ai_addr;
+		msg.msg_namelen = ai->ai_addrlen;
+	}
 	msg.msg_iov = iov;
 	msg.msg_iovlen = 1;
 
diff --git a/tools/testing/selftests/net/cmsg_so_mark.sh b/tools/testing/selftests/net/cmsg_so_mark.sh
--- a/tools/testing/selftests/net/cmsg_so_mark.sh
+++ b/tools/testing/selftests/net/cmsg_so_mark.sh
@@ -68,6 +68,22 @@
     done
 done
 
+# Check per-message marks after connect() has populated the route cache.
+for i in 4 6; do
+    [ "$i" = 4 ] && TGT=$TGT4 || TGT=$TGT6
+
+    for p in u i r; do
+	for mark in "$((MARK + 1))" "$MARK" 0; do
+	    expected=0
+	    [ "$mark" -eq "$MARK" ] && expected=1
+
+	    ip netns exec "$NS" ./cmsg_sender -"$i" -p "$p" -k \
+		-M "$((MARK + 1))" -m "$mark" -s "$TGT" 1234
+	    check_result "$?" "$expected" "IPv$i $p connected mark $mark"
+	done
+    done
+done
+
 # Summary
 if [ $BAD -ne 0 ]; then
     echo "FAIL - $BAD/$TOTAL cases failed"

Re: [PATCH net v2 1/2] ipv6: fix cached route reuse for SO_MARK control messages

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2 1/2] ipv6: fix cached route reuse for SO_MARK control messages

[Xia Yan]

Hi,

On how the issue was discovered: during code review of IPv6 UDP route
caching. v1 carried this in its commit message ("Found during code
review of IPv6 UDP route caching."); the v2 rewrite dropped the
sentence:

https://lore.kernel.org/netdev/20261030141435.779302-1-yanxia@cmss.chinamobile.com/

The concurrent variant came out of the review of v1, which prompted
the v2 rework. The v2 commit message notes this already.

Best regards,
Xia Yan