BPF List
 help / color / mirror / Atom feed
* [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions.
@ 2026-05-04 21:04 Kuniyuki Iwashima
  2026-05-04 21:04 ` [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock() Kuniyuki Iwashima
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:04 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: John Fastabend, Stanislav Fomichev, Song Liu, Yonghong Song,
	Jiri Olsa, Eric Dumazet, Kuniyuki Iwashima, Kuniyuki Iwashima,
	bpf, netdev

bpf_tcp_sock() only check if sk->sk_protocol is IPPROTO_TCP,
but RAW socket can bypass it:

  socket(AF_INET, SOCK_RAW, IPPROTO_TCP)

The same issues exist in other bpf functions:

  * bpf_mptcp_sock_from_subflow()
  * bpf_skc_to_tcp_sock()
  * bpf_skc_to_tcp6_sock()
  * sol_tcp_sockopt()

Patch 1 fixes bpf_tcp_sock() and Patch 2 adds a test for it.
Patch 3 ~ 6 fix the rest of the functions above.


Changes:
  v2:
    * Inverse if (err) to if (!err) in the selftest
    * Add patch 3 ~ 6

  v1: https://lore.kernel.org/bpf/20260430184405.1227386-1-kuniyu@google.com/
      https://lore.kernel.org/mptcp/20260430-mptcp-bpf-mptcp-sock-type-v1-1-d2ed5cda7da9@kernel.org/


Kuniyuki Iwashima (5):
  bpf: tcp: Fix type confusion in bpf_tcp_sock().
  selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.
  bpf: tcp: Fix type confusion in bpf_skc_to_tcp_sock().
  bpf: tcp: Fix type confusion in bpf_skc_to_tcp6_sock().
  bpf: tcp: Fix type confusion in sol_tcp_sockopt().

Matthieu Baerts (NGI0) (1):
  mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()

 net/core/filter.c                              |  8 ++++----
 net/mptcp/bpf.c                                |  2 +-
 .../selftests/bpf/prog_tests/sockopt_sk.c      | 18 +++++++++++++++++-
 tools/testing/selftests/bpf/progs/sockopt_sk.c | 16 ++++++++++++++++
 4 files changed, 38 insertions(+), 6 deletions(-)

-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock().
  2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
@ 2026-05-04 21:04 ` Kuniyuki Iwashima
  2026-05-04 21:50   ` bot+bpf-ci
  2026-05-05  4:14   ` sashiko-bot
  2026-05-04 21:04 ` [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket Kuniyuki Iwashima
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:04 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: John Fastabend, Stanislav Fomichev, Song Liu, Yonghong Song,
	Jiri Olsa, Eric Dumazet, Kuniyuki Iwashima, Kuniyuki Iwashima,
	bpf, netdev, Damiano Melotti

bpf_tcp_sock() only checks if sk->sk_protocol is IPPROTO_TCP,
but RAW socket can bypass it:

  socket(AF_INET, SOCK_RAW, IPPROTO_TCP)

Calling bpf_setsockopt() in SOCKOPT prog triggers out-of-bounds
access to another slab object. [0]

Let's use sk_is_tcp().

[0]:
BUG: KASAN: slab-out-of-bounds in sol_tcp_sockopt (net/core/filter.c:5519)
Read of size 8 at addr ffff88801083d760 by task test_progs/1259

CPU: 1 UID: 0 PID: 1259 Comm: test_progs Tainted: G           OE       7.0.0-11175-gb5c111f4967b #1 PREEMPT(full)
Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
 print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
 kasan_report (mm/kasan/report.c:595)
 sol_tcp_sockopt (net/core/filter.c:5519)
 __bpf_getsockopt (net/core/filter.c:5633)
 bpf_sk_getsockopt (net/core/filter.c:5654)
 bpf_prog_629ba00a1601e9f2__setsockopt+0x86/0x22c
 __cgroup_bpf_run_filter_setsockopt (./include/linux/bpf.h:1402 ./include/linux/filter.h:722 ./include/linux/filter.h:729 kernel/bpf/cgroup.c:81 kernel/bpf/cgroup.c:2026)
 do_sock_setsockopt (net/socket.c:2363)
 __x64_sys_setsockopt (net/socket.c:2406)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
RIP: 0033:0x7f85f82fe7de
Code: 55 48 63 c9 48 63 ff 45 89 c9 48 89 e5 48 83 ec 08 6a 2c e8 34 69 f7 ff c9 c3 66 90 f3 0f 1e fa 49 89 ca b8 36 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 0a c3 66 0f 1f 84 00 00 00 00 00 48 8b 15 e1
RSP: 002b:00007ffe59dcecd8 EFLAGS: 00000202 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f85f82fe7de
RDX: 000000000000001c RSI: 0000000000000006 RDI: 000000000000000d
RBP: 00007ffe59dcef20 R08: 000000000000003c R09: 0000000000000000
R10: 00007ffe59dcef00 R11: 0000000000000202 R12: 00007ffe59dcf268
R13: 0000000000000003 R14: 00007f85f9da5000 R15: 000055b2f3201400
 </TASK>

The buggy address belongs to the object at ffff88801083d280
 which belongs to the cache RAW of size 1792
The buggy address is located 1248 bytes inside of
 allocated 1792-byte region [ffff88801083d280, ffff88801083d980)

Fixes: 655a51e536c0 ("bpf: Add struct bpf_tcp_sock and BPF_FUNC_tcp_sock")
Reported-by: Damiano Melotti <melotti@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/core/filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index bc96c18df4e0..cd88633f8dc1 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
 
 BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
 {
-	if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
+	if (sk_fullsock(sk) && sk_is_tcp(sk))
 		return (unsigned long)sk;
 
 	return (unsigned long)NULL;
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.
  2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
  2026-05-04 21:04 ` [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock() Kuniyuki Iwashima
@ 2026-05-04 21:04 ` Kuniyuki Iwashima
  2026-05-05  4:14   ` sashiko-bot
  2026-05-04 21:04 ` [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow() Kuniyuki Iwashima
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:04 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: John Fastabend, Stanislav Fomichev, Song Liu, Yonghong Song,
	Jiri Olsa, Eric Dumazet, Kuniyuki Iwashima, Kuniyuki Iwashima,
	bpf, netdev

Let's extend sockopt_sk.c to cover bpf_tcp_sock() for the
wrong socket type.

Before:
  # ./test_progs -t sockopt_sk
  [  151.948613] ==================================================================
  [  151.951376] BUG: KASAN: slab-out-of-bounds in sol_tcp_sockopt+0xc7/0x8e0
  [  151.954159] Read of size 8 at addr ffff88801083d760 by task test_progs/1259
  ...
  run_test:FAIL:getsetsockopt unexpected error: -1 (errno 0)
  #427     sockopt_sk:FAIL

After:
  #427     sockopt_sk:OK

While at it, missing free() is fixed up.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v2: Inverted if (err) to if (!err)
---
 .../selftests/bpf/prog_tests/sockopt_sk.c      | 18 +++++++++++++++++-
 tools/testing/selftests/bpf/progs/sockopt_sk.c | 16 ++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
index 53637431ec5d..5fd33ad2eaaf 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
@@ -190,7 +190,7 @@ static int getsetsockopt(void)
 	fd = socket(AF_NETLINK, SOCK_RAW, 0);
 	if (fd < 0) {
 		log_err("Failed to create AF_NETLINK socket");
-		return -1;
+		goto err;
 	}
 
 	buf.u32 = 1;
@@ -211,6 +211,22 @@ static int getsetsockopt(void)
 	}
 	ASSERT_EQ(optlen, 8, "Unexpected NETLINK_LIST_MEMBERSHIPS value");
 
+	/* Trick bpf_tcp_sock() with IPPROTO_TCP */
+	close(fd);
+	fd = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
+	if (fd < 0) {
+		log_err("Failed to create RAW socket");
+		goto err;
+	}
+
+	optlen = 20;
+	errno = 0;
+	err = setsockopt(fd, SOL_TCP, TCP_SAVED_SYN, &buf, optlen);
+	if (!err) {
+		log_err("Unexpected setsockopt(TCP_SAVED_SYN)");
+		goto err;
+	}
+
 	free(big_buf);
 	close(fd);
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/sockopt_sk.c b/tools/testing/selftests/bpf/progs/sockopt_sk.c
index cb990a7d3d45..5e0b27e7855c 100644
--- a/tools/testing/selftests/bpf/progs/sockopt_sk.c
+++ b/tools/testing/selftests/bpf/progs/sockopt_sk.c
@@ -149,6 +149,20 @@ int _setsockopt(struct bpf_sockopt *ctx)
 	if (sk && sk->family == AF_NETLINK)
 		goto out;
 
+	if (sk && sk->family == AF_INET && sk->type == SOCK_RAW) {
+		struct bpf_tcp_sock *tp = bpf_tcp_sock(sk);
+
+		if (tp) {
+			char saved_syn[60];
+
+			bpf_getsockopt(sk, SOL_TCP, TCP_SAVED_SYN,
+				       &saved_syn, sizeof(saved_syn));
+			goto consumed;
+		}
+
+		goto out;
+	}
+
 	/* Make sure bpf_get_netns_cookie is callable.
 	 */
 	if (bpf_get_netns_cookie(NULL) == 0)
@@ -224,6 +238,8 @@ int _setsockopt(struct bpf_sockopt *ctx)
 		return 0; /* couldn't get sk storage */
 
 	storage->val = optval[0];
+
+consumed:
 	ctx->optlen = -1; /* BPF has consumed this option, don't call kernel
 			   * setsockopt handler.
 			   */
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()
  2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
  2026-05-04 21:04 ` [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock() Kuniyuki Iwashima
  2026-05-04 21:04 ` [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket Kuniyuki Iwashima
@ 2026-05-04 21:04 ` Kuniyuki Iwashima
  2026-05-05  4:14   ` sashiko-bot
  2026-05-04 21:04 ` [PATCH v2 bpf 4/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp_sock() Kuniyuki Iwashima
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:04 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: John Fastabend, Stanislav Fomichev, Song Liu, Yonghong Song,
	Jiri Olsa, Eric Dumazet, Kuniyuki Iwashima, Kuniyuki Iwashima,
	bpf, netdev, Matthieu Baerts (NGI0)

From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>

bpf_mptcp_sock_from_subflow() only checks if sk->sk_protocol is
IPPROTO_TCP, but RAW socket can bypass it:

  socket(AF_INET, SOCK_RAW, IPPROTO_TCP)

In this case, it would NOT be valid to call sk_is_mptcp() which will
assume sk is a pointer to a struct tcp_sock, and wrongly checks for:
tcp_sk(sk)->is_mptcp.

Fixes: 3bc253c2e652 ("bpf: Add bpf_skc_to_mptcp_sock_proto")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v1: https://lore.kernel.org/mptcp/20260430-mptcp-bpf-mptcp-sock-type-v1-1-d2ed5cda7da9@kernel.org/
---
 net/mptcp/bpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 8a16672b94e2..4cc16cbeb328 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -14,7 +14,7 @@
 
 struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
 {
-	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
+	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
 		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
 
 	return NULL;
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v2 bpf 4/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp_sock().
  2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2026-05-04 21:04 ` [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow() Kuniyuki Iwashima
@ 2026-05-04 21:04 ` Kuniyuki Iwashima
  2026-05-04 21:04 ` [PATCH v2 bpf 5/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp6_sock() Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:04 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: John Fastabend, Stanislav Fomichev, Song Liu, Yonghong Song,
	Jiri Olsa, Eric Dumazet, Kuniyuki Iwashima, Kuniyuki Iwashima,
	bpf, netdev

bpf_skc_to_tcp_sock() only checks if sk->sk_protocol is
IPPROTO_TCP, but RAW socket can bypass it:

  socket(AF_INET, SOCK_RAW, IPPROTO_TCP)

Let's use sk_is_tcp().

Fixes: 478cfbdf5f13 ("bpf: Add bpf_skc_to_{tcp, tcp_timewait, tcp_request}_sock() helpers")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index cd88633f8dc1..7d945dc2cb92 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11963,7 +11963,7 @@ const struct bpf_func_proto bpf_skc_to_tcp6_sock_proto = {
 
 BPF_CALL_1(bpf_skc_to_tcp_sock, struct sock *, sk)
 {
-	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
+	if (sk && sk_fullsock(sk) && sk_is_tcp(sk))
 		return (unsigned long)sk;
 
 	return (unsigned long)NULL;
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v2 bpf 5/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp6_sock().
  2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
                   ` (3 preceding siblings ...)
  2026-05-04 21:04 ` [PATCH v2 bpf 4/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp_sock() Kuniyuki Iwashima
@ 2026-05-04 21:04 ` Kuniyuki Iwashima
  2026-05-04 21:04 ` [PATCH v2 bpf 6/6] bpf: tcp: Fix type confusion in sol_tcp_sockopt() Kuniyuki Iwashima
  2026-05-08 19:10 ` [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions patchwork-bot+netdevbpf
  6 siblings, 0 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:04 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: John Fastabend, Stanislav Fomichev, Song Liu, Yonghong Song,
	Jiri Olsa, Eric Dumazet, Kuniyuki Iwashima, Kuniyuki Iwashima,
	bpf, netdev

bpf_skc_to_tcp6_sock() only checks if sk->sk_protocol is IPPROTO_TCP
and sk->sk_family is AF_INET6, but RAW socket can bypass it:

  socket(AF_INET6, SOCK_RAW, IPPROTO_TCP)

Let's check sk->sk_type too.

Fixes: af7ec1383361 ("bpf: Add bpf_skc_to_tcp6_sock() helper")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 7d945dc2cb92..684922efd481 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11947,7 +11947,7 @@ BPF_CALL_1(bpf_skc_to_tcp6_sock, struct sock *, sk)
 	 */
 	BTF_TYPE_EMIT(struct tcp6_sock);
 	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP &&
-	    sk->sk_family == AF_INET6)
+	    sk->sk_type == SOCK_STREAM && sk->sk_family == AF_INET6)
 		return (unsigned long)sk;
 
 	return (unsigned long)NULL;
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH v2 bpf 6/6] bpf: tcp: Fix type confusion in sol_tcp_sockopt().
  2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
                   ` (4 preceding siblings ...)
  2026-05-04 21:04 ` [PATCH v2 bpf 5/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp6_sock() Kuniyuki Iwashima
@ 2026-05-04 21:04 ` Kuniyuki Iwashima
  2026-05-08 19:10 ` [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions patchwork-bot+netdevbpf
  6 siblings, 0 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:04 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: John Fastabend, Stanislav Fomichev, Song Liu, Yonghong Song,
	Jiri Olsa, Eric Dumazet, Kuniyuki Iwashima, Kuniyuki Iwashima,
	bpf, netdev

sol_tcp_sockopt() only checks if sk->sk_protocol is IPPROTO_TCP,
but RAW socket can bypass it:

  socket(AF_INET, SOCK_RAW, IPPROTO_TCP)

Let's use sk_is_tcp().

Note that initially sol_tcp_sockopt() checked sk->sk_prot->setsockopt.

Fixes: 2ab42c7b871f ("bpf: Check the protocol of a sock to agree the calls to bpf_setsockopt().")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 684922efd481..ef0877eefaa7 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5481,7 +5481,7 @@ static int sol_tcp_sockopt(struct sock *sk, int optname,
 			   char *optval, int *optlen,
 			   bool getopt)
 {
-	if (sk->sk_protocol != IPPROTO_TCP)
+	if (!sk_is_tcp(sk))
 		return -EINVAL;
 
 	switch (optname) {
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock().
  2026-05-04 21:04 ` [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock() Kuniyuki Iwashima
@ 2026-05-04 21:50   ` bot+bpf-ci
  2026-05-04 21:53     ` Kuniyuki Iwashima
  2026-05-05  4:14   ` sashiko-bot
  1 sibling, 1 reply; 17+ messages in thread
From: bot+bpf-ci @ 2026-05-04 21:50 UTC (permalink / raw)
  To: kuniyu, martin.lau, daniel, ast, andrii, eddyz87, memxor
  Cc: john.fastabend, sdf, song, yonghong.song, jolsa, edumazet, kuniyu,
	kuni1840, bpf, netdev, melotti, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 3247 bytes --]

> diff --git a/net/core/filter.c b/net/core/filter.c
> index bc96c18df4e0..cd88633f8dc1 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
>
>  BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
>  {
> -	if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
> +	if (sk_fullsock(sk) && sk_is_tcp(sk))
>  		return (unsigned long)sk;
>
>  	return (unsigned long)NULL;

While this patch fixes bpf_tcp_sock(), does sol_tcp_sockopt() remain
vulnerable? Since ctx->sk evaluates to PTR_TO_SOCKET in a CGROUP_SOCKOPT
program and is directly accepted by bpf_setsockopt() and bpf_getsockopt()
via ARG_PTR_TO_BTF_ID_SOCK_COMMON, can a user create a RAW socket with
IPPROTO_TCP and directly pass ctx->sk to bpf_setsockopt(), bypassing
bpf_tcp_sock() entirely?

Looking at sol_tcp_sockopt():

static int sol_tcp_sockopt(struct sock *sk, int optname,
			   char *optval, int *optlen,
			   bool getopt)
{
	if (sk->sk_protocol != IPPROTO_TCP)
		return -EINVAL;

This appears to only check sk->sk_protocol == IPPROTO_TCP without verifying
sk->sk_type == SOCK_STREAM, which would allow the same slab out-of-bounds
access through a different code path.

Additionally, do bpf_skc_to_tcp_sock() and bpf_skc_to_tcp6_sock() suffer
from the same type confusion? They appear to check sk->sk_protocol ==
IPPROTO_TCP without verifying sk->sk_type == SOCK_STREAM.

Could a BPF program pass a RAW socket with IPPROTO_TCP to these helpers
and receive a valid BTF pointer to struct tcp_sock, allowing out-of-bounds
slab reads?

Looking at bpf_skc_to_tcp_sock():

BPF_CALL_1(bpf_skc_to_tcp_sock, struct sock *, sk)
{
	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
		return (unsigned long)sk;

	return (unsigned long)NULL;
}

And bpf_skc_to_tcp6_sock():

BPF_CALL_1(bpf_skc_to_tcp6_sock, struct sock *, sk)
{
	/* tcp6_sock type is not generated in dwarf and hence btf,
	 * trigger an explicit type generation here.
	 */
	BTF_TYPE_EMIT(struct tcp6_sock);
	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP &&
	    sk->sk_family == AF_INET6)
		return (unsigned long)sk;

Finally, does bpf_mptcp_sock_from_subflow() in net/mptcp/bpf.c also need
a similar update? It checks sk->sk_protocol == IPPROTO_TCP and immediately
evaluates sk_is_mptcp(sk).

Since sk_is_mptcp() indiscriminately casts the socket to struct tcp_sock
and accesses tcp_sk(sk)->is_mptcp, passing a RAW socket could trigger an
immediate out-of-bounds slab read. If this memory happens to be non-zero,
it might proceed to call mptcp_subflow_ctx(sk)->conn, which would read a
garbage pointer from out-of-bounds memory:

struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
{
	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);

	return NULL;
}

These issues were raised by sashiko-bot@kernel.org in v1:
https://lore.kernel.org/bpf/20260501184452.7F46FC2BCB4@smtp.kernel.org/


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25344293379

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock().
  2026-05-04 21:50   ` bot+bpf-ci
@ 2026-05-04 21:53     ` Kuniyuki Iwashima
  0 siblings, 0 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-04 21:53 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: martin.lau, daniel, ast, andrii, eddyz87, memxor, john.fastabend,
	sdf, song, yonghong.song, jolsa, edumazet, kuni1840, bpf, netdev,
	melotti, martin.lau, clm, ihor.solodrai

On Mon, May 4, 2026 at 2:50 PM <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index bc96c18df4e0..cd88633f8dc1 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
> >
> >  BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
> >  {
> > -     if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
> > +     if (sk_fullsock(sk) && sk_is_tcp(sk))
> >               return (unsigned long)sk;
> >
> >       return (unsigned long)NULL;
>
> While this patch fixes bpf_tcp_sock(), does sol_tcp_sockopt() remain

It seems Sashiko does not have context on the following patches.

All helper functions mentioned in the review comment are fixed
in the later patches separately.



> vulnerable? Since ctx->sk evaluates to PTR_TO_SOCKET in a CGROUP_SOCKOPT
> program and is directly accepted by bpf_setsockopt() and bpf_getsockopt()
> via ARG_PTR_TO_BTF_ID_SOCK_COMMON, can a user create a RAW socket with
> IPPROTO_TCP and directly pass ctx->sk to bpf_setsockopt(), bypassing
> bpf_tcp_sock() entirely?
>
> Looking at sol_tcp_sockopt():
>
> static int sol_tcp_sockopt(struct sock *sk, int optname,
>                            char *optval, int *optlen,
>                            bool getopt)
> {
>         if (sk->sk_protocol != IPPROTO_TCP)
>                 return -EINVAL;
>
> This appears to only check sk->sk_protocol == IPPROTO_TCP without verifying
> sk->sk_type == SOCK_STREAM, which would allow the same slab out-of-bounds
> access through a different code path.
>
> Additionally, do bpf_skc_to_tcp_sock() and bpf_skc_to_tcp6_sock() suffer
> from the same type confusion? They appear to check sk->sk_protocol ==
> IPPROTO_TCP without verifying sk->sk_type == SOCK_STREAM.
>
> Could a BPF program pass a RAW socket with IPPROTO_TCP to these helpers
> and receive a valid BTF pointer to struct tcp_sock, allowing out-of-bounds
> slab reads?
>
> Looking at bpf_skc_to_tcp_sock():
>
> BPF_CALL_1(bpf_skc_to_tcp_sock, struct sock *, sk)
> {
>         if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
>                 return (unsigned long)sk;
>
>         return (unsigned long)NULL;
> }
>
> And bpf_skc_to_tcp6_sock():
>
> BPF_CALL_1(bpf_skc_to_tcp6_sock, struct sock *, sk)
> {
>         /* tcp6_sock type is not generated in dwarf and hence btf,
>          * trigger an explicit type generation here.
>          */
>         BTF_TYPE_EMIT(struct tcp6_sock);
>         if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP &&
>             sk->sk_family == AF_INET6)
>                 return (unsigned long)sk;
>
> Finally, does bpf_mptcp_sock_from_subflow() in net/mptcp/bpf.c also need
> a similar update? It checks sk->sk_protocol == IPPROTO_TCP and immediately
> evaluates sk_is_mptcp(sk).
>
> Since sk_is_mptcp() indiscriminately casts the socket to struct tcp_sock
> and accesses tcp_sk(sk)->is_mptcp, passing a RAW socket could trigger an
> immediate out-of-bounds slab read. If this memory happens to be non-zero,
> it might proceed to call mptcp_subflow_ctx(sk)->conn, which would read a
> garbage pointer from out-of-bounds memory:
>
> struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
> {
>         if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
>                 return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
>
>         return NULL;
> }
>
> These issues were raised by sashiko-bot@kernel.org in v1:
> https://lore.kernel.org/bpf/20260501184452.7F46FC2BCB4@smtp.kernel.org/
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25344293379

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock().
  2026-05-04 21:04 ` [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock() Kuniyuki Iwashima
  2026-05-04 21:50   ` bot+bpf-ci
@ 2026-05-05  4:14   ` sashiko-bot
  2026-05-05  5:37     ` Kuniyuki Iwashima
  1 sibling, 1 reply; 17+ messages in thread
From: sashiko-bot @ 2026-05-05  4:14 UTC (permalink / raw)
  To: Kuniyuki Iwashima; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The OpenVPN (ovpn) driver contains a critical type confusion vulnerability due to insufficient socket type checking, leading to out-of-bounds slab writes.
--

commit 3e93f247feecc891b15b8d713331586af9371dfd
Author: Kuniyuki Iwashima <kuniyu@google.com>

bpf: tcp: Fix type confusion in bpf_tcp_sock().

This commit fixes an out-of-bounds slab access bug in BPF by using sk_is_tcp()
instead of just checking sk_protocol for IPPROTO_TCP. This prevents raw sockets
created with IPPROTO_TCP from bypassing the type check.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index bc96c18df4e03..cd88633f8dc14 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
>  
>  BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
>  {
> -	if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
> +	if (sk_fullsock(sk) && sk_is_tcp(sk))

This correctly prevents a type confusion where a raw socket can bypass the
protocol check. Does the OpenVPN (ovpn) driver need a similar fix for this
exact same pattern?

In drivers/net/ovpn/netlink.c, the driver accepts an arbitrary socket file
descriptor from userspace and determines the socket type using only sk_protocol
without verifying sk_type.

If a user creates a raw socket via socket(AF_INET, SOCK_RAW, IPPROTO_UDP) and
passes it to the OpenVPN netlink interface, it seems it would pass the protocol
check and call ovpn_udp_socket_attach().

This eventually passes the socket to setup_udp_tunnel_sock(), which casts the
socket to udp_sk(sk) and overwrites multiple fields (such as encap_type,
encap_rcv, and gro_receive).

Since struct udp_sock is larger than struct raw_sock, could this result in an
out-of-bounds slab write?

Similarly, passing a raw TCP socket appears to cause ovpn_tcp_socket_attach()
to replace sk->sk_prot with ovpn_tcp_prot. This might mean subsequent TCP
socket operations will operate out-of-bounds on the raw_sock object.

Should ovpn be updated to validate sk_type == SOCK_DGRAM or SOCK_STREAM in a
similar manner to this fix?

>  		return (unsigned long)sk;
>  
>  	return (unsigned long)NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504210610.180150-1-kuniyu@google.com?part=1

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.
  2026-05-04 21:04 ` [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket Kuniyuki Iwashima
@ 2026-05-05  4:14   ` sashiko-bot
  2026-05-05  5:38     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 17+ messages in thread
From: sashiko-bot @ 2026-05-05  4:14 UTC (permalink / raw)
  To: Kuniyuki Iwashima; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Newly added code uses manual error checks instead of the mandated modern ASSERT_* macros.
--

commit 1a574056fa08ab8c66dcd9b6ee5f0739ff6721b7
Author: Kuniyuki Iwashima <kuniyu@google.com>

selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.

This commit adds a test to cover bpf_tcp_sock() for wrong socket types,
specifically tricking it with IPPROTO_TCP on an AF_INET RAW socket. It
also fixes a missing free() in the test.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> index 53637431ec5d..5fd33ad2eaaf 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c

[ ... ]

> @@ -211,6 +211,22 @@ static int getsetsockopt(void)
>  	}
>  	ASSERT_EQ(optlen, 8, "Unexpected NETLINK_LIST_MEMBERSHIPS value");
>  
> +	/* Trick bpf_tcp_sock() with IPPROTO_TCP */
> +	close(fd);
> +	fd = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
> +	if (fd < 0) {
> +		log_err("Failed to create RAW socket");
> +		goto err;
> +	}

This isn't a bug, but should this new code use the modern ASSERT_OK_FD()
macro instead of manual fd checks and log_err()?

> +
> +	optlen = 20;
> +	errno = 0;
> +	err = setsockopt(fd, SOL_TCP, TCP_SAVED_SYN, &buf, optlen);
> +	if (!err) {
> +		log_err("Unexpected setsockopt(TCP_SAVED_SYN)");
> +		goto err;
> +	}

Could we use ASSERT_ERR() for this error check as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504210610.180150-1-kuniyu@google.com?part=2

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()
  2026-05-04 21:04 ` [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow() Kuniyuki Iwashima
@ 2026-05-05  4:14   ` sashiko-bot
  2026-05-05  5:43     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 17+ messages in thread
From: sashiko-bot @ 2026-05-05  4:14 UTC (permalink / raw)
  To: Kuniyuki Iwashima; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] NULL pointer dereferences in bpf_mptcp_sock_from_subflow due to lockless concurrent accesses
- [High] Use-After-Free in BPF programs returning freed mptcp socket during teardown
--

commit 52dbcdaad7d49ff5db2faacf11c47b1c581024a8
Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>
mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()

This commit fixes a type confusion issue in bpf_mptcp_sock_from_subflow() by
checking if the socket is actually a TCP socket before calling sk_is_mptcp().
Previously, raw sockets could bypass the IPPROTO_TCP check and cause incorrect
assumptions about the socket type.

> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 8a16672b94e2..4cc16cbeb328 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -14,7 +14,7 @@
>  
>  struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
>  {
> -	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> +	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
>  		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);

Could this cause a regression by dereferencing a NULL pointer when accessed
concurrently?

In subflow_ulp_fallback(), icsk->icsk_ulp_data is set to NULL prior to
clearing tcp_sk(sk)->is_mptcp. If a BPF program triggers in this window,
sk_is_mptcp() would return true but mptcp_subflow_ctx(sk) would return NULL,
leading to a NULL pointer dereference.

Additionally, could ctx->conn be NULL when read here?

During subflow_ulp_init(), the context is allocated and is_mptcp is set to 1,
but ctx->conn is initially NULL. Passing NULL to mptcp_sk(ctx->conn) triggers
a kernel panic on CONFIG_DEBUG_NET builds. On non-debug builds, it returns a
non-NULL offset pointer, which bypasses BPF verifier NULL checks and causes
invalid memory accesses.

Could this introduce a regression with a use-after-free of the mptcp_sock
during teardown?

During subflow_ulp_release(), sock_put(ctx->conn) drops the reference to the
parent mptcp_sock and may free it. However, ctx->conn is not cleared, and the
subflow context remains valid until the RCU grace period ends.

If a BPF tracing program triggers after sock_put(), could this retrieve
and return the dangling ctx->conn pointer, enabling the BPF program to read
from freed kernel slab memory?

>  
>  	return NULL;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504210610.180150-1-kuniyu@google.com?part=3

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock().
  2026-05-05  4:14   ` sashiko-bot
@ 2026-05-05  5:37     ` Kuniyuki Iwashima
  0 siblings, 0 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-05  5:37 UTC (permalink / raw)
  To: sashiko; +Cc: bpf

On Mon, May 4, 2026 at 9:14 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] The OpenVPN (ovpn) driver contains a critical type confusion vulnerability due to insufficient socket type checking, leading to out-of-bounds slab writes.
> --
>
> commit 3e93f247feecc891b15b8d713331586af9371dfd
> Author: Kuniyuki Iwashima <kuniyu@google.com>
>
> bpf: tcp: Fix type confusion in bpf_tcp_sock().
>
> This commit fixes an out-of-bounds slab access bug in BPF by using sk_is_tcp()
> instead of just checking sk_protocol for IPPROTO_TCP. This prevents raw sockets
> created with IPPROTO_TCP from bypassing the type check.
>
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index bc96c18df4e03..cd88633f8dc14 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
> >
> >  BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
> >  {
> > -     if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
> > +     if (sk_fullsock(sk) && sk_is_tcp(sk))
>
> This correctly prevents a type confusion where a raw socket can bypass the
> protocol check. Does the OpenVPN (ovpn) driver need a similar fix for this
> exact same pattern?
>
> In drivers/net/ovpn/netlink.c, the driver accepts an arbitrary socket file
> descriptor from userspace and determines the socket type using only sk_protocol
> without verifying sk_type.

Not related to BPF.  The point sounds valid, but I'd let
ovpn folks follow up.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.
  2026-05-05  4:14   ` sashiko-bot
@ 2026-05-05  5:38     ` Kuniyuki Iwashima
  0 siblings, 0 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-05  5:38 UTC (permalink / raw)
  To: sashiko; +Cc: bpf

On Mon, May 4, 2026 at 9:14 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Newly added code uses manual error checks instead of the mandated modern ASSERT_* macros.
> --
>
> commit 1a574056fa08ab8c66dcd9b6ee5f0739ff6721b7
> Author: Kuniyuki Iwashima <kuniyu@google.com>
>
> selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.
>
> This commit adds a test to cover bpf_tcp_sock() for wrong socket types,
> specifically tricking it with IPPROTO_TCP on an AF_INET RAW socket. It
> also fixes a missing free() in the test.
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> > index 53637431ec5d..5fd33ad2eaaf 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_sk.c
>
> [ ... ]
>
> > @@ -211,6 +211,22 @@ static int getsetsockopt(void)
> >       }
> >       ASSERT_EQ(optlen, 8, "Unexpected NETLINK_LIST_MEMBERSHIPS value");
> >
> > +     /* Trick bpf_tcp_sock() with IPPROTO_TCP */
> > +     close(fd);
> > +     fd = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
> > +     if (fd < 0) {
> > +             log_err("Failed to create RAW socket");
> > +             goto err;
> > +     }
>
> This isn't a bug, but should this new code use the modern ASSERT_OK_FD()
> macro instead of manual fd checks and log_err()?

I don't mix different styles in the same file, also such
cleanup should be done in bpf-next.


>
> > +
> > +     optlen = 20;
> > +     errno = 0;
> > +     err = setsockopt(fd, SOL_TCP, TCP_SAVED_SYN, &buf, optlen);
> > +     if (!err) {
> > +             log_err("Unexpected setsockopt(TCP_SAVED_SYN)");
> > +             goto err;
> > +     }
>
> Could we use ASSERT_ERR() for this error check as well?

Same.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()
  2026-05-05  4:14   ` sashiko-bot
@ 2026-05-05  5:43     ` Kuniyuki Iwashima
  2026-05-05 15:14       ` Matthieu Baerts
  0 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-05  5:43 UTC (permalink / raw)
  To: sashiko, Matthieu Baerts; +Cc: bpf

Hi Matt,

Could you follow up on the comments by Sashiko ?

On Mon, May 4, 2026 at 9:14 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] NULL pointer dereferences in bpf_mptcp_sock_from_subflow due to lockless concurrent accesses
> - [High] Use-After-Free in BPF programs returning freed mptcp socket during teardown
> --
>
> commit 52dbcdaad7d49ff5db2faacf11c47b1c581024a8
> Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()
>
> This commit fixes a type confusion issue in bpf_mptcp_sock_from_subflow() by
> checking if the socket is actually a TCP socket before calling sk_is_mptcp().
> Previously, raw sockets could bypass the IPPROTO_TCP check and cause incorrect
> assumptions about the socket type.
>
> > diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> > index 8a16672b94e2..4cc16cbeb328 100644
> > --- a/net/mptcp/bpf.c
> > +++ b/net/mptcp/bpf.c
> > @@ -14,7 +14,7 @@
> >
> >  struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
> >  {
> > -     if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> > +     if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
> >               return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
>
> Could this cause a regression by dereferencing a NULL pointer when accessed
> concurrently?
>
> In subflow_ulp_fallback(), icsk->icsk_ulp_data is set to NULL prior to
> clearing tcp_sk(sk)->is_mptcp. If a BPF program triggers in this window,
> sk_is_mptcp() would return true but mptcp_subflow_ctx(sk) would return NULL,
> leading to a NULL pointer dereference.
>
> Additionally, could ctx->conn be NULL when read here?
>
> During subflow_ulp_init(), the context is allocated and is_mptcp is set to 1,
> but ctx->conn is initially NULL. Passing NULL to mptcp_sk(ctx->conn) triggers
> a kernel panic on CONFIG_DEBUG_NET builds. On non-debug builds, it returns a
> non-NULL offset pointer, which bypasses BPF verifier NULL checks and causes
> invalid memory accesses.
>
> Could this introduce a regression with a use-after-free of the mptcp_sock
> during teardown?
>
> During subflow_ulp_release(), sock_put(ctx->conn) drops the reference to the
> parent mptcp_sock and may free it. However, ctx->conn is not cleared, and the
> subflow context remains valid until the RCU grace period ends.
>
> If a BPF tracing program triggers after sock_put(), could this retrieve
> and return the dangling ctx->conn pointer, enabling the BPF program to read
> from freed kernel slab memory?
>
> >
> >       return NULL;
> >  }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260504210610.180150-1-kuniyu@google.com?part=3

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()
  2026-05-05  5:43     ` Kuniyuki Iwashima
@ 2026-05-05 15:14       ` Matthieu Baerts
  0 siblings, 0 replies; 17+ messages in thread
From: Matthieu Baerts @ 2026-05-05 15:14 UTC (permalink / raw)
  To: Kuniyuki Iwashima, sashiko; +Cc: bpf, MPTCP Linux

Hi Kuniyuki,

(+cc MPTCP ML)

On 05/05/2026 07:43, Kuniyuki Iwashima wrote:
> Could you follow up on the comments by Sashiko ?

Thank you for having forwarded this!

> On Mon, May 4, 2026 at 9:14 PM <sashiko-bot@kernel.org> wrote:

(...)

>>> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
>>> index 8a16672b94e2..4cc16cbeb328 100644
>>> --- a/net/mptcp/bpf.c
>>> +++ b/net/mptcp/bpf.c
>>> @@ -14,7 +14,7 @@
>>>
>>>  struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
>>>  {
>>> -     if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
>>> +     if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
>>>               return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
>>
>> Could this cause a regression by dereferencing a NULL pointer when accessed
>> concurrently?
>>
>> In subflow_ulp_fallback(), icsk->icsk_ulp_data is set to NULL prior to
>> clearing tcp_sk(sk)->is_mptcp. If a BPF program triggers in this window,
>> sk_is_mptcp() would return true but mptcp_subflow_ctx(sk) would return NULL,
>> leading to a NULL pointer dereference.

This would need to be confirmed, but from what I see, that's not a new
bug introduced by the patch here.

>> Additionally, could ctx->conn be NULL when read here?
>>
>> During subflow_ulp_init(), the context is allocated and is_mptcp is set to 1,
>> but ctx->conn is initially NULL. Passing NULL to mptcp_sk(ctx->conn) triggers
>> a kernel panic on CONFIG_DEBUG_NET builds. On non-debug builds, it returns a
>> non-NULL offset pointer, which bypasses BPF verifier NULL checks and causes
>> invalid memory accesses.

Same here: not a new bug.

>> Could this introduce a regression with a use-after-free of the mptcp_sock
>> during teardown?
>>
>> During subflow_ulp_release(), sock_put(ctx->conn) drops the reference to the
>> parent mptcp_sock and may free it. However, ctx->conn is not cleared, and the
>> subflow context remains valid until the RCU grace period ends.
>>
>> If a BPF tracing program triggers after sock_put(), could this retrieve
>> and return the dangling ctx->conn pointer, enabling the BPF program to read
>> from freed kernel slab memory?
Same here: not a new bug.

Sashiko is good at finding potential existing bugs that predate a patch,
which is good, but someone has to fix them :)

Because the issues raised here are not new, I suggest not blocking this
patch. I will check if someone can look at it quickly.

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


^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions.
  2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
                   ` (5 preceding siblings ...)
  2026-05-04 21:04 ` [PATCH v2 bpf 6/6] bpf: tcp: Fix type confusion in sol_tcp_sockopt() Kuniyuki Iwashima
@ 2026-05-08 19:10 ` patchwork-bot+netdevbpf
  6 siblings, 0 replies; 17+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-05-08 19:10 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: martin.lau, daniel, ast, andrii, eddyz87, memxor, john.fastabend,
	sdf, song, yonghong.song, jolsa, edumazet, kuni1840, bpf, netdev

Hello:

This series was applied to bpf/bpf.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:

On Mon,  4 May 2026 21:04:47 +0000 you wrote:
> bpf_tcp_sock() only check if sk->sk_protocol is IPPROTO_TCP,
> but RAW socket can bypass it:
> 
>   socket(AF_INET, SOCK_RAW, IPPROTO_TCP)
> 
> The same issues exist in other bpf functions:
> 
> [...]

Here is the summary with links:
  - [v2,bpf,1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock().
    https://git.kernel.org/bpf/bpf/c/481c2265286e
  - [v2,bpf,2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket.
    https://git.kernel.org/bpf/bpf/c/d73549b8bb7f
  - [v2,bpf,3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow()
    https://git.kernel.org/bpf/bpf/c/7995b216a731
  - [v2,bpf,4/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp_sock().
    https://git.kernel.org/bpf/bpf/c/decb84b8383a
  - [v2,bpf,5/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp6_sock().
    https://git.kernel.org/bpf/bpf/c/843064b0a77e
  - [v2,bpf,6/6] bpf: tcp: Fix type confusion in sol_tcp_sockopt().
    https://git.kernel.org/bpf/bpf/c/1c2958e4ab1e

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



^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-05-08 19:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 21:04 [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions Kuniyuki Iwashima
2026-05-04 21:04 ` [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock() Kuniyuki Iwashima
2026-05-04 21:50   ` bot+bpf-ci
2026-05-04 21:53     ` Kuniyuki Iwashima
2026-05-05  4:14   ` sashiko-bot
2026-05-05  5:37     ` Kuniyuki Iwashima
2026-05-04 21:04 ` [PATCH v2 bpf 2/6] selftest: bpf: Add test for bpf_tcp_sock() and RAW socket Kuniyuki Iwashima
2026-05-05  4:14   ` sashiko-bot
2026-05-05  5:38     ` Kuniyuki Iwashima
2026-05-04 21:04 ` [PATCH v2 bpf 3/6] mptcp: bpf: fix type confusion in bpf_mptcp_sock_from_subflow() Kuniyuki Iwashima
2026-05-05  4:14   ` sashiko-bot
2026-05-05  5:43     ` Kuniyuki Iwashima
2026-05-05 15:14       ` Matthieu Baerts
2026-05-04 21:04 ` [PATCH v2 bpf 4/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp_sock() Kuniyuki Iwashima
2026-05-04 21:04 ` [PATCH v2 bpf 5/6] bpf: tcp: Fix type confusion in bpf_skc_to_tcp6_sock() Kuniyuki Iwashima
2026-05-04 21:04 ` [PATCH v2 bpf 6/6] bpf: tcp: Fix type confusion in sol_tcp_sockopt() Kuniyuki Iwashima
2026-05-08 19:10 ` [PATCH v2 bpf 0/6] bpf: tcp: Fix type confusion in bpf helper functions patchwork-bot+netdevbpf

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