[PATCH net 0/2] udp6: allow SO_MARK ctrl msg to affect routing

[PATCH net 0/2] udp6: allow SO_MARK ctrl msg to affect routing

[Jakub Kicinski]

Hi!

Looks like SO_MARK from cmsg does not affect routing policy.
This seems accidental.

I opted for net because of the discrepancy between IPv4
and IPv6, but it never worked and doesn't cause crashes..

Jakub Kicinski (2):
  udp6: allow SO_MARK ctrl msg to affect routing
  selftests: udp: test for passing SO_MARK as cmsg

 net/ipv6/udp.c                              |  2 +-
 tools/testing/selftests/net/.gitignore      |  1 +
 tools/testing/selftests/net/Makefile        |  2 +
 tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
 tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
 5 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
 create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh


base-commit: 6de6e46d27ef386feecdbea56b3bfd6c3b3bc1f9
-- 
2.31.1

[PATCH net 1/2] udp6: allow SO_MARK ctrl msg to affect routing

[Jakub Kicinski]

Commit c6af0c227a22 ("ip: support SO_MARK cmsg")
added propagation of SO_MARK from cmsg to skb->mark.
For IPv4 and raw sockets the mark also affects route
lookup, but in case of IPv6 the flow info is
initialized before cmsg is parsed.

Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
Reported-and-tested-by: Xintong Hu <huxintong@fb.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ipv6/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 8d785232b479..be6dc64ece29 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1435,7 +1435,6 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (!fl6.flowi6_oif)
 		fl6.flowi6_oif = np->sticky_pktinfo.ipi6_ifindex;
 
-	fl6.flowi6_mark = ipc6.sockc.mark;
 	fl6.flowi6_uid = sk->sk_uid;
 
 	if (msg->msg_controllen) {
@@ -1471,6 +1470,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	ipc6.opt = opt;
 
 	fl6.flowi6_proto = sk->sk_protocol;
+	fl6.flowi6_mark = ipc6.sockc.mark;
 	fl6.daddr = *daddr;
 	if (ipv6_addr_any(&fl6.saddr) && !ipv6_addr_any(&np->saddr))
 		fl6.saddr = np->saddr;
-- 
2.31.1

Re: [PATCH net 1/2] udp6: allow SO_MARK ctrl msg to affect routing

[David Ahern]

On 10/29/21 9:51 AM, Jakub Kicinski wrote:
> Commit c6af0c227a22 ("ip: support SO_MARK cmsg")
> added propagation of SO_MARK from cmsg to skb->mark.
> For IPv4 and raw sockets the mark also affects route
> lookup, but in case of IPv6 the flow info is
> initialized before cmsg is parsed.
> 
> Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
> Reported-and-tested-by: Xintong Hu <huxintong@fb.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/ipv6/udp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net 1/2] udp6: allow SO_MARK ctrl msg to affect routing

[Willem de Bruijn]

On Fri, Oct 29, 2021 at 2:22 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 10/29/21 9:51 AM, Jakub Kicinski wrote:
> > Commit c6af0c227a22 ("ip: support SO_MARK cmsg")
> > added propagation of SO_MARK from cmsg to skb->mark.
> > For IPv4 and raw sockets the mark also affects route
> > lookup, but in case of IPv6 the flow info is
> > initialized before cmsg is parsed.
> >
> > Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
> > Reported-and-tested-by: Xintong Hu <huxintong@fb.com>
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  net/ipv6/udp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Thanks!

[PATCH net 2/2] selftests: udp: test for passing SO_MARK as cmsg

[Jakub Kicinski]

Before fix:
|  Case IPv6 rejection returned 0, expected 1
|FAIL - 1/4 cases failed

With the fix:
| OK

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
--
CC: shuah@kernel.org
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/net/.gitignore      |  1 +
 tools/testing/selftests/net/Makefile        |  2 +
 tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
 tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
 4 files changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
 create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 501550501216..7581a7348e1b 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -35,3 +35,4 @@ test_unix_oob
 gro
 ioam6_parser
 toeplitz
+cmsg_so_mark
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 492b273743b4..f56b652d5cc6 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -28,6 +28,7 @@ TEST_PROGS += veth.sh
 TEST_PROGS += ioam6.sh
 TEST_PROGS += gro.sh
 TEST_PROGS += gre_gso.sh
+TEST_PROGS += cmsg_so_mark.sh
 TEST_PROGS_EXTENDED := in_netns.sh
 TEST_GEN_FILES =  socket nettest
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any
@@ -44,6 +45,7 @@ TEST_GEN_FILES += gro
 TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
 TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls
 TEST_GEN_FILES += toeplitz
+TEST_GEN_FILES += cmsg_so_mark
 
 TEST_FILES := settings
 
diff --git a/tools/testing/selftests/net/cmsg_so_mark.c b/tools/testing/selftests/net/cmsg_so_mark.c
new file mode 100644
index 000000000000..27f2804892a7
--- /dev/null
+++ b/tools/testing/selftests/net/cmsg_so_mark.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <errno.h>
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <linux/types.h>
+#include <sys/socket.h>
+
+int main(int argc, const char **argv)
+{
+	char cbuf[CMSG_SPACE(sizeof(__u32))];
+	struct addrinfo hints, *ai;
+	struct cmsghdr *cmsg;
+	struct iovec iov[1];
+	struct msghdr msg;
+	int mark;
+	int err;
+	int fd;
+
+	if (argc != 4) {
+		fprintf(stderr, "Usage: %s <dst_ip> <port> <mark>\n", argv[0]);
+		return 1;
+	}
+	mark = atoi(argv[3]);
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_family = AF_UNSPEC;
+	hints.ai_socktype = SOCK_DGRAM;
+
+	ai = NULL;
+	err = getaddrinfo(argv[1], argv[2], &hints, &ai);
+	if (err) {
+		fprintf(stderr, "Can't resolve address: %s\n", strerror(errno));
+		return 1;
+	}
+
+	fd = socket(ai->ai_family, SOCK_DGRAM, IPPROTO_UDP);
+	if (fd < 0) {
+		fprintf(stderr, "Can't open socket: %s\n", strerror(errno));
+		freeaddrinfo(ai);
+		return 1;
+	}
+
+	iov[0].iov_base = "bla";
+	iov[0].iov_len = 4;
+
+	msg.msg_name = ai->ai_addr;
+	msg.msg_namelen = ai->ai_addrlen;
+	msg.msg_iov = iov;
+	msg.msg_iovlen = 1;
+	msg.msg_control = cbuf;
+	msg.msg_controllen = sizeof(cbuf);
+
+	cmsg = CMSG_FIRSTHDR(&msg);
+	cmsg->cmsg_level = SOL_SOCKET;
+	cmsg->cmsg_type = SO_MARK;
+	cmsg->cmsg_len = CMSG_LEN(sizeof(__u32));
+	*(__u32 *)CMSG_DATA(cmsg) = mark;
+
+	err = sendmsg(fd, &msg, 0);
+
+	close(fd);
+	freeaddrinfo(ai);
+	return err != 4;
+}
diff --git a/tools/testing/selftests/net/cmsg_so_mark.sh b/tools/testing/selftests/net/cmsg_so_mark.sh
new file mode 100755
index 000000000000..19c6aab8d0e9
--- /dev/null
+++ b/tools/testing/selftests/net/cmsg_so_mark.sh
@@ -0,0 +1,61 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+NS=ns
+IP4=172.16.0.1/24
+TGT4=172.16.0.2
+IP6=2001:db8:1::1/64
+TGT6=2001:db8:1::2
+MARK=1000
+
+cleanup()
+{
+    ip netns del $NS
+}
+
+trap cleanup EXIT
+
+# Namespaces
+ip netns add $NS
+
+# Connectivity
+ip -netns $NS link add type dummy
+ip -netns $NS link set dev dummy0 up
+ip -netns $NS addr add $IP4 dev dummy0
+ip -netns $NS addr add $IP6 dev dummy0
+
+ip -netns $NS rule add fwmark $MARK lookup 300
+ip -6 -netns $NS rule add fwmark $MARK lookup 300
+ip -netns $NS route add prohibit any table 300
+ip -6 -netns $NS route add prohibit any table 300
+
+# Test
+BAD=0
+TOTAL=0
+
+check_result() {
+    ((TOTAL++))
+    if [ $1 -ne $2 ]; then
+	echo "  Case $3 returned $1, expected $2"
+	((BAD++))
+    fi
+}
+
+ip netns exec $NS ./cmsg_so_mark $TGT4 1234 $((MARK + 1))
+check_result $? 0 "IPv4 pass"
+ip netns exec $NS ./cmsg_so_mark $TGT6 1234 $((MARK + 1))
+check_result $? 0 "IPv6 pass"
+
+ip netns exec $NS ./cmsg_so_mark $TGT4 1234 $MARK
+check_result $? 1 "IPv4 rejection"
+ip netns exec $NS ./cmsg_so_mark $TGT6 1234 $MARK
+check_result $? 1 "IPv6 rejection"
+
+# Summary
+if [ $BAD -ne 0 ]; then
+    echo "FAIL - $BAD/$TOTAL cases failed"
+    exit 1
+else
+    echo "OK"
+    exit 0
+fi
-- 
2.31.1

Re: [PATCH net 2/2] selftests: udp: test for passing SO_MARK as cmsg

[David Ahern]

On 10/29/21 9:51 AM, Jakub Kicinski wrote:
> Before fix:
> |  Case IPv6 rejection returned 0, expected 1
> |FAIL - 1/4 cases failed
> 
> With the fix:
> | OK
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> --
> CC: shuah@kernel.org
> CC: linux-kselftest@vger.kernel.org
> ---
>  tools/testing/selftests/net/.gitignore      |  1 +
>  tools/testing/selftests/net/Makefile        |  2 +
>  tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
>  tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
>  4 files changed, 131 insertions(+)
>  create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
>  create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh
>
Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net 2/2] selftests: udp: test for passing SO_MARK as cmsg

[Willem de Bruijn]

On Fri, Oct 29, 2021 at 2:24 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 10/29/21 9:51 AM, Jakub Kicinski wrote:
> > Before fix:
> > |  Case IPv6 rejection returned 0, expected 1
> > |FAIL - 1/4 cases failed
> >
> > With the fix:
> > | OK
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > --
> > CC: shuah@kernel.org
> > CC: linux-kselftest@vger.kernel.org
> > ---
> >  tools/testing/selftests/net/.gitignore      |  1 +
> >  tools/testing/selftests/net/Makefile        |  2 +
> >  tools/testing/selftests/net/cmsg_so_mark.c  | 67 +++++++++++++++++++++
> >  tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++
> >  4 files changed, 131 insertions(+)
> >  create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c
> >  create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh
> >
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net 0/2] udp6: allow SO_MARK ctrl msg to affect routing

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 29 Oct 2021 08:51:33 -0700 you wrote:
> Hi!
> 
> Looks like SO_MARK from cmsg does not affect routing policy.
> This seems accidental.
> 
> I opted for net because of the discrepancy between IPv4
> and IPv6, but it never worked and doesn't cause crashes..
> 
> [...]

Here is the summary with links:
  - [net,1/2] udp6: allow SO_MARK ctrl msg to affect routing
    https://git.kernel.org/netdev/net/c/42dcfd850e51
  - [net,2/2] selftests: udp: test for passing SO_MARK as cmsg
    https://git.kernel.org/netdev/net/c/b0ced8f290fb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html