netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg
@ 2019-01-04  9:07 Andrey Ignatov
  2019-01-04  9:07 ` [PATCH bpf v2 1/2] " Andrey Ignatov
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Andrey Ignatov @ 2019-01-04  9:07 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, nitin.rawat, kernel-team

The patch set fixes BSD'ism in sys_sendmsg to rewrite unspecified
destination IPv6 for unconnected UDP sockets in sys_sendmsg with [::1] in
case when either CONFIG_CGROUP_BPF is enabled or when sys_sendmsg BPF hook
sets destination IPv6 to [::].

Patch 1 is the fix and provides more details.
Patch 2 adds two test cases to verify the fix.

v1->v2:
* Fix compile error in patch 1.


Andrey Ignatov (2):
  bpf: Fix [::] -> [::1] rewrite in sys_sendmsg
  selftests/bpf: Test [::] -> [::1] rewrite in sys_sendmsg in
    test_sock_addr

 net/ipv6/udp.c                               |  8 +--
 tools/testing/selftests/bpf/test_sock_addr.c | 53 ++++++++++++++++++--
 2 files changed, 54 insertions(+), 7 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH bpf v2 1/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg
  2019-01-04  9:07 [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg Andrey Ignatov
@ 2019-01-04  9:07 ` Andrey Ignatov
  2019-01-04  9:07 ` [PATCH bpf v2 2/2] selftests/bpf: Test [::] -> [::1] rewrite in sys_sendmsg in test_sock_addr Andrey Ignatov
  2019-01-05  4:26 ` [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg Alexei Starovoitov
  2 siblings, 0 replies; 4+ messages in thread
From: Andrey Ignatov @ 2019-01-04  9:07 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, nitin.rawat, kernel-team

sys_sendmsg has supported unspecified destination IPv6 (wildcard) for
unconnected UDP sockets since 876c7f41. When [::] is passed by user as
destination, sys_sendmsg rewrites it with [::1] to be consistent with
BSD (see "BSD'ism" comment in the code).

This didn't work when cgroup-bpf was enabled though since the rewrite
[::] -> [::1] happened before passing control to cgroup-bpf block where
fl6.daddr was updated with passed by user sockaddr_in6.sin6_addr (that
might or might not be changed by BPF program). That way if user passed
[::] as dst IPv6 it was first rewritten with [::1] by original code from
876c7f41, but then rewritten back with [::] by cgroup-bpf block.

It happened even when BPF_CGROUP_UDP6_SENDMSG program was not present
(CONFIG_CGROUP_BPF=y was enough).

The fix is to apply BSD'ism after cgroup-bpf block so that [::] is
replaced with [::1] no matter where it came from: passed by user to
sys_sendmsg or set by BPF_CGROUP_UDP6_SENDMSG program.

Fixes: 1cedee13d25a ("bpf: Hooks for sys_sendmsg")
Reported-by: Nitin Rawat <nitin.rawat@intel.com>
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 net/ipv6/udp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 9cbf363172bd..7c3505006f8e 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1390,10 +1390,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	ipc6.opt = opt;
 
 	fl6.flowi6_proto = sk->sk_protocol;
-	if (!ipv6_addr_any(daddr))
-		fl6.daddr = *daddr;
-	else
-		fl6.daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
+	fl6.daddr = *daddr;
 	if (ipv6_addr_any(&fl6.saddr) && !ipv6_addr_any(&np->saddr))
 		fl6.saddr = np->saddr;
 	fl6.fl6_sport = inet->inet_sport;
@@ -1421,6 +1418,9 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		}
 	}
 
+	if (ipv6_addr_any(&fl6.daddr))
+		fl6.daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
+
 	final_p = fl6_update_dst(&fl6, opt, &final);
 	if (final_p)
 		connected = false;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH bpf v2 2/2] selftests/bpf: Test [::] -> [::1] rewrite in sys_sendmsg in test_sock_addr
  2019-01-04  9:07 [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg Andrey Ignatov
  2019-01-04  9:07 ` [PATCH bpf v2 1/2] " Andrey Ignatov
@ 2019-01-04  9:07 ` Andrey Ignatov
  2019-01-05  4:26 ` [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg Alexei Starovoitov
  2 siblings, 0 replies; 4+ messages in thread
From: Andrey Ignatov @ 2019-01-04  9:07 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, nitin.rawat, kernel-team

Test that sys_sendmsg BPF hook doesn't break sys_sendmsg behaviour to
rewrite destination IPv6 = [::] with [::1] (BSD'ism).

Two test cases are added:

1) User passes dst IPv6 = [::] and BPF_CGROUP_UDP6_SENDMSG program
   doesn't touch it.

2) User passes dst IPv6 != [::], but BPF_CGROUP_UDP6_SENDMSG program
   rewrites it with [::].

In both cases [::1] is used by sys_sendmsg code eventually and datagram
is sent successfully for unconnected UDP socket.

Example of relevant output:
  Test case: sendmsg6: set dst IP = [::] (BSD'ism) .. [PASS]
  Test case: sendmsg6: preserve dst IP = [::] (BSD'ism) .. [PASS]

Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 tools/testing/selftests/bpf/test_sock_addr.c | 53 ++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_sock_addr.c b/tools/testing/selftests/bpf/test_sock_addr.c
index 73b7493d4120..d94336cbd8bd 100644
--- a/tools/testing/selftests/bpf/test_sock_addr.c
+++ b/tools/testing/selftests/bpf/test_sock_addr.c
@@ -44,6 +44,7 @@
 #define SERV6_V4MAPPED_IP	"::ffff:192.168.0.4"
 #define SRC6_IP			"::1"
 #define SRC6_REWRITE_IP		"::6"
+#define WILDCARD6_IP		"::"
 #define SERV6_PORT		6060
 #define SERV6_REWRITE_PORT	6666
 
@@ -85,12 +86,14 @@ static int bind4_prog_load(const struct sock_addr_test *test);
 static int bind6_prog_load(const struct sock_addr_test *test);
 static int connect4_prog_load(const struct sock_addr_test *test);
 static int connect6_prog_load(const struct sock_addr_test *test);
+static int sendmsg_allow_prog_load(const struct sock_addr_test *test);
 static int sendmsg_deny_prog_load(const struct sock_addr_test *test);
 static int sendmsg4_rw_asm_prog_load(const struct sock_addr_test *test);
 static int sendmsg4_rw_c_prog_load(const struct sock_addr_test *test);
 static int sendmsg6_rw_asm_prog_load(const struct sock_addr_test *test);
 static int sendmsg6_rw_c_prog_load(const struct sock_addr_test *test);
 static int sendmsg6_rw_v4mapped_prog_load(const struct sock_addr_test *test);
+static int sendmsg6_rw_wildcard_prog_load(const struct sock_addr_test *test);
 
 static struct sock_addr_test tests[] = {
 	/* bind */
@@ -462,6 +465,34 @@ static struct sock_addr_test tests[] = {
 		SRC6_REWRITE_IP,
 		SYSCALL_ENOTSUPP,
 	},
+	{
+		"sendmsg6: set dst IP = [::] (BSD'ism)",
+		sendmsg6_rw_wildcard_prog_load,
+		BPF_CGROUP_UDP6_SENDMSG,
+		BPF_CGROUP_UDP6_SENDMSG,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+		SUCCESS,
+	},
+	{
+		"sendmsg6: preserve dst IP = [::] (BSD'ism)",
+		sendmsg_allow_prog_load,
+		BPF_CGROUP_UDP6_SENDMSG,
+		BPF_CGROUP_UDP6_SENDMSG,
+		AF_INET6,
+		SOCK_DGRAM,
+		WILDCARD6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_PORT,
+		SRC6_IP,
+		SUCCESS,
+	},
 	{
 		"sendmsg6: deny call",
 		sendmsg_deny_prog_load,
@@ -734,16 +765,27 @@ static int connect6_prog_load(const struct sock_addr_test *test)
 	return load_path(test, CONNECT6_PROG_PATH);
 }
 
-static int sendmsg_deny_prog_load(const struct sock_addr_test *test)
+static int sendmsg_ret_only_prog_load(const struct sock_addr_test *test,
+				      int32_t rc)
 {
 	struct bpf_insn insns[] = {
-		/* return 0 */
-		BPF_MOV64_IMM(BPF_REG_0, 0),
+		/* return rc */
+		BPF_MOV64_IMM(BPF_REG_0, rc),
 		BPF_EXIT_INSN(),
 	};
 	return load_insns(test, insns, sizeof(insns) / sizeof(struct bpf_insn));
 }
 
+static int sendmsg_allow_prog_load(const struct sock_addr_test *test)
+{
+	return sendmsg_ret_only_prog_load(test, /*rc*/ 1);
+}
+
+static int sendmsg_deny_prog_load(const struct sock_addr_test *test)
+{
+	return sendmsg_ret_only_prog_load(test, /*rc*/ 0);
+}
+
 static int sendmsg4_rw_asm_prog_load(const struct sock_addr_test *test)
 {
 	struct sockaddr_in dst4_rw_addr;
@@ -864,6 +906,11 @@ static int sendmsg6_rw_v4mapped_prog_load(const struct sock_addr_test *test)
 	return sendmsg6_rw_dst_asm_prog_load(test, SERV6_V4MAPPED_IP);
 }
 
+static int sendmsg6_rw_wildcard_prog_load(const struct sock_addr_test *test)
+{
+	return sendmsg6_rw_dst_asm_prog_load(test, WILDCARD6_IP);
+}
+
 static int sendmsg6_rw_c_prog_load(const struct sock_addr_test *test)
 {
 	return load_path(test, SENDMSG6_PROG_PATH);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg
  2019-01-04  9:07 [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg Andrey Ignatov
  2019-01-04  9:07 ` [PATCH bpf v2 1/2] " Andrey Ignatov
  2019-01-04  9:07 ` [PATCH bpf v2 2/2] selftests/bpf: Test [::] -> [::1] rewrite in sys_sendmsg in test_sock_addr Andrey Ignatov
@ 2019-01-05  4:26 ` Alexei Starovoitov
  2 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2019-01-05  4:26 UTC (permalink / raw)
  To: Andrey Ignatov; +Cc: netdev, ast, daniel, nitin.rawat, kernel-team

On Fri, Jan 04, 2019 at 01:07:06AM -0800, Andrey Ignatov wrote:
> The patch set fixes BSD'ism in sys_sendmsg to rewrite unspecified
> destination IPv6 for unconnected UDP sockets in sys_sendmsg with [::1] in
> case when either CONFIG_CGROUP_BPF is enabled or when sys_sendmsg BPF hook
> sets destination IPv6 to [::].
> 
> Patch 1 is the fix and provides more details.
> Patch 2 adds two test cases to verify the fix.
> 
> v1->v2:
> * Fix compile error in patch 1.

Applied, Thanks

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-01-05  4:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-04  9:07 [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg Andrey Ignatov
2019-01-04  9:07 ` [PATCH bpf v2 1/2] " Andrey Ignatov
2019-01-04  9:07 ` [PATCH bpf v2 2/2] selftests/bpf: Test [::] -> [::1] rewrite in sys_sendmsg in test_sock_addr Andrey Ignatov
2019-01-05  4:26 ` [PATCH bpf v2 0/2] bpf: Fix [::] -> [::1] rewrite in sys_sendmsg Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).