Skip to content

Commit f3b8c28

Browse files
author
Martin KaFai Lau
committed
Merge branch 'bpf-tcp-fix-type-confusion-in-bpf-helper-functions'
Kuniyuki Iwashima says: ==================== bpf: tcp: Fix type confusion in bpf helper functions. 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/ ==================== Link: https://patch.msgid.link/20260504210610.180150-1-kuniyu@google.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
2 parents 0c7ae13 + 1c2958e commit f3b8c28

4 files changed

Lines changed: 37 additions & 6 deletions

File tree

net/core/filter.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5481,7 +5481,7 @@ static int sol_tcp_sockopt(struct sock *sk, int optname,
54815481
char *optval, int *optlen,
54825482
bool getopt)
54835483
{
5484-
if (sk->sk_protocol != IPPROTO_TCP)
5484+
if (!sk_is_tcp(sk))
54855485
return -EINVAL;
54865486

54875487
switch (optname) {
@@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
74757475

74767476
BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
74777477
{
7478-
if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
7478+
if (sk_fullsock(sk) && sk_is_tcp(sk))
74797479
return (unsigned long)sk;
74807480

74817481
return (unsigned long)NULL;
@@ -11947,7 +11947,7 @@ BPF_CALL_1(bpf_skc_to_tcp6_sock, struct sock *, sk)
1194711947
*/
1194811948
BTF_TYPE_EMIT(struct tcp6_sock);
1194911949
if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP &&
11950-
sk->sk_family == AF_INET6)
11950+
sk->sk_type == SOCK_STREAM && sk->sk_family == AF_INET6)
1195111951
return (unsigned long)sk;
1195211952

1195311953
return (unsigned long)NULL;
@@ -11963,7 +11963,7 @@ const struct bpf_func_proto bpf_skc_to_tcp6_sock_proto = {
1196311963

1196411964
BPF_CALL_1(bpf_skc_to_tcp_sock, struct sock *, sk)
1196511965
{
11966-
if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
11966+
if (sk && sk_fullsock(sk) && sk_is_tcp(sk))
1196711967
return (unsigned long)sk;
1196811968

1196911969
return (unsigned long)NULL;

net/mptcp/bpf.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
1616
{
17-
if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
17+
if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
1818
return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
1919

2020
return NULL;

tools/testing/selftests/bpf/prog_tests/sockopt_sk.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ static int getsetsockopt(void)
190190
fd = socket(AF_NETLINK, SOCK_RAW, 0);
191191
if (fd < 0) {
192192
log_err("Failed to create AF_NETLINK socket");
193-
return -1;
193+
goto err;
194194
}
195195

196196
buf.u32 = 1;
@@ -211,6 +211,21 @@ static int getsetsockopt(void)
211211
}
212212
ASSERT_EQ(optlen, 8, "Unexpected NETLINK_LIST_MEMBERSHIPS value");
213213

214+
/* Trick bpf_tcp_sock() with IPPROTO_TCP */
215+
close(fd);
216+
fd = socket(AF_INET, SOCK_RAW, IPPROTO_TCP);
217+
if (!ASSERT_OK_FD(fd, "socket"))
218+
goto err;
219+
220+
/* The BPF prog intercepts this before the kernel sees it, any
221+
* optlen works. Go with 4 bytes for simplicity.
222+
*/
223+
buf.u32 = 1;
224+
optlen = sizeof(buf.u32);
225+
err = setsockopt(fd, SOL_TCP, TCP_SAVED_SYN, &buf, optlen);
226+
if (!ASSERT_ERR(err, "setsockopt(TCP_SAVED_SYN)"))
227+
goto err;
228+
214229
free(big_buf);
215230
close(fd);
216231
return 0;

tools/testing/selftests/bpf/progs/sockopt_sk.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,20 @@ int _setsockopt(struct bpf_sockopt *ctx)
149149
if (sk && sk->family == AF_NETLINK)
150150
goto out;
151151

152+
if (sk && sk->family == AF_INET && sk->type == SOCK_RAW) {
153+
struct bpf_tcp_sock *tp = bpf_tcp_sock(sk);
154+
155+
if (tp) {
156+
char saved_syn[60];
157+
158+
bpf_getsockopt(sk, SOL_TCP, TCP_SAVED_SYN,
159+
&saved_syn, sizeof(saved_syn));
160+
goto consumed;
161+
}
162+
163+
goto out;
164+
}
165+
152166
/* Make sure bpf_get_netns_cookie is callable.
153167
*/
154168
if (bpf_get_netns_cookie(NULL) == 0)
@@ -224,6 +238,8 @@ int _setsockopt(struct bpf_sockopt *ctx)
224238
return 0; /* couldn't get sk storage */
225239

226240
storage->val = optval[0];
241+
242+
consumed:
227243
ctx->optlen = -1; /* BPF has consumed this option, don't call kernel
228244
* setsockopt handler.
229245
*/

0 commit comments

Comments
 (0)