Skip to content

Commit 858d2a4

Browse files
edumazetkuba-moo
authored andcommitted
tcp: fix potential race in tcp_v6_syn_recv_sock()
Code in tcp_v6_syn_recv_sock() after the call to tcp_v4_syn_recv_sock() is done too late. After tcp_v4_syn_recv_sock(), the child socket is already visible from TCP ehash table and other cpus might use it. Since newinet->pinet6 is still pointing to the listener ipv6_pinfo bad things can happen as syzbot found. Move the problematic code in tcp_v6_mapped_child_init() and call this new helper from tcp_v4_syn_recv_sock() before the ehash insertion. This allows the removal of one tcp_sync_mss(), since tcp_v4_syn_recv_sock() will call it with the correct context. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: syzbot+937b5bbb6a815b3e5d0b@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/69949275.050a0220.2eeac1.0145.GAE@google.com/ Signed-off-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com> Link: https://patch.msgid.link/20260217161205.2079883-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 8bf22c3 commit 858d2a4

9 files changed

Lines changed: 66 additions & 66 deletions

File tree

include/net/inet_connection_sock.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ struct inet_connection_sock_af_ops {
4242
struct request_sock *req,
4343
struct dst_entry *dst,
4444
struct request_sock *req_unhash,
45-
bool *own_req);
45+
bool *own_req,
46+
void (*opt_child_init)(struct sock *newsk,
47+
const struct sock *sk));
4648
u16 net_header_len;
4749
int (*setsockopt)(struct sock *sk, int level, int optname,
4850
sockptr_t optval, unsigned int optlen);

include/net/tcp.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,9 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
544544
struct request_sock *req,
545545
struct dst_entry *dst,
546546
struct request_sock *req_unhash,
547-
bool *own_req);
547+
bool *own_req,
548+
void (*opt_child_init)(struct sock *newsk,
549+
const struct sock *sk));
548550
int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb);
549551
int tcp_v4_connect(struct sock *sk, struct sockaddr_unsized *uaddr, int addr_len);
550552
int tcp_connect(struct sock *sk);

net/ipv4/syncookies.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
203203
bool own_req;
204204

205205
child = icsk->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
206-
NULL, &own_req);
206+
NULL, &own_req, NULL);
207207
if (child) {
208208
refcount_set(&req->rsk_refcnt, 1);
209209
sock_rps_save_rxhash(child, skb);

net/ipv4/tcp_fastopen.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
333333
bool own_req;
334334

335335
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
336-
NULL, &own_req);
336+
NULL, &own_req, NULL);
337337
if (!child)
338338
return NULL;
339339

net/ipv4/tcp_ipv4.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1705,7 +1705,9 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
17051705
struct request_sock *req,
17061706
struct dst_entry *dst,
17071707
struct request_sock *req_unhash,
1708-
bool *own_req)
1708+
bool *own_req,
1709+
void (*opt_child_init)(struct sock *newsk,
1710+
const struct sock *sk))
17091711
{
17101712
struct inet_request_sock *ireq;
17111713
bool found_dup_sk = false;
@@ -1757,6 +1759,10 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
17571759
}
17581760
sk_setup_caps(newsk, dst);
17591761

1762+
#if IS_ENABLED(CONFIG_IPV6)
1763+
if (opt_child_init)
1764+
opt_child_init(newsk, sk);
1765+
#endif
17601766
tcp_ca_openreq_child(newsk, dst);
17611767

17621768
tcp_sync_mss(newsk, dst4_mtu(dst));

net/ipv4/tcp_minisocks.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
925925
* socket is created, wait for troubles.
926926
*/
927927
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
928-
req, &own_req);
928+
req, &own_req, NULL);
929929
if (!child)
930930
goto listen_overflow;
931931

net/ipv6/tcp_ipv6.c

Lines changed: 42 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,11 +1312,48 @@ static void tcp_v6_restore_cb(struct sk_buff *skb)
13121312
sizeof(struct inet6_skb_parm));
13131313
}
13141314

1315+
/* Called from tcp_v4_syn_recv_sock() for v6_mapped children. */
1316+
static void tcp_v6_mapped_child_init(struct sock *newsk, const struct sock *sk)
1317+
{
1318+
struct inet_sock *newinet = inet_sk(newsk);
1319+
struct ipv6_pinfo *newnp;
1320+
1321+
newinet->pinet6 = newnp = tcp_inet6_sk(newsk);
1322+
newinet->ipv6_fl_list = NULL;
1323+
1324+
memcpy(newnp, tcp_inet6_sk(sk), sizeof(struct ipv6_pinfo));
1325+
1326+
newnp->saddr = newsk->sk_v6_rcv_saddr;
1327+
1328+
inet_csk(newsk)->icsk_af_ops = &ipv6_mapped;
1329+
if (sk_is_mptcp(newsk))
1330+
mptcpv6_handle_mapped(newsk, true);
1331+
newsk->sk_backlog_rcv = tcp_v4_do_rcv;
1332+
#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
1333+
tcp_sk(newsk)->af_specific = &tcp_sock_ipv6_mapped_specific;
1334+
#endif
1335+
1336+
newnp->ipv6_mc_list = NULL;
1337+
newnp->ipv6_ac_list = NULL;
1338+
newnp->pktoptions = NULL;
1339+
newnp->opt = NULL;
1340+
1341+
/* tcp_v4_syn_recv_sock() has initialized newinet->mc_{index,ttl} */
1342+
newnp->mcast_oif = newinet->mc_index;
1343+
newnp->mcast_hops = newinet->mc_ttl;
1344+
1345+
newnp->rcv_flowinfo = 0;
1346+
if (inet6_test_bit(REPFLOW, sk))
1347+
newnp->flow_label = 0;
1348+
}
1349+
13151350
static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
13161351
struct request_sock *req,
13171352
struct dst_entry *dst,
13181353
struct request_sock *req_unhash,
1319-
bool *own_req)
1354+
bool *own_req,
1355+
void (*opt_child_init)(struct sock *newsk,
1356+
const struct sock *sk))
13201357
{
13211358
const struct ipv6_pinfo *np = tcp_inet6_sk(sk);
13221359
struct inet_request_sock *ireq;
@@ -1332,61 +1369,10 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
13321369
#endif
13331370
struct flowi6 fl6;
13341371

1335-
if (skb->protocol == htons(ETH_P_IP)) {
1336-
/*
1337-
* v6 mapped
1338-
*/
1339-
1340-
newsk = tcp_v4_syn_recv_sock(sk, skb, req, dst,
1341-
req_unhash, own_req);
1342-
1343-
if (!newsk)
1344-
return NULL;
1345-
1346-
newinet = inet_sk(newsk);
1347-
newinet->pinet6 = tcp_inet6_sk(newsk);
1348-
newinet->ipv6_fl_list = NULL;
1349-
1350-
newnp = tcp_inet6_sk(newsk);
1351-
newtp = tcp_sk(newsk);
1352-
1353-
memcpy(newnp, np, sizeof(struct ipv6_pinfo));
1354-
1355-
newnp->saddr = newsk->sk_v6_rcv_saddr;
1356-
1357-
inet_csk(newsk)->icsk_af_ops = &ipv6_mapped;
1358-
if (sk_is_mptcp(newsk))
1359-
mptcpv6_handle_mapped(newsk, true);
1360-
newsk->sk_backlog_rcv = tcp_v4_do_rcv;
1361-
#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
1362-
newtp->af_specific = &tcp_sock_ipv6_mapped_specific;
1363-
#endif
1364-
1365-
newnp->ipv6_mc_list = NULL;
1366-
newnp->ipv6_ac_list = NULL;
1367-
newnp->pktoptions = NULL;
1368-
newnp->opt = NULL;
1369-
newnp->mcast_oif = inet_iif(skb);
1370-
newnp->mcast_hops = ip_hdr(skb)->ttl;
1371-
newnp->rcv_flowinfo = 0;
1372-
if (inet6_test_bit(REPFLOW, sk))
1373-
newnp->flow_label = 0;
1374-
1375-
/*
1376-
* No need to charge this sock to the relevant IPv6 refcnt debug socks count
1377-
* here, tcp_create_openreq_child now does this for us, see the comment in
1378-
* that function for the gory details. -acme
1379-
*/
1380-
1381-
/* It is tricky place. Until this moment IPv4 tcp
1382-
worked with IPv6 icsk.icsk_af_ops.
1383-
Sync it now.
1384-
*/
1385-
tcp_sync_mss(newsk, inet_csk(newsk)->icsk_pmtu_cookie);
1386-
1387-
return newsk;
1388-
}
1389-
1372+
if (skb->protocol == htons(ETH_P_IP))
1373+
return tcp_v4_syn_recv_sock(sk, skb, req, dst,
1374+
req_unhash, own_req,
1375+
tcp_v6_mapped_child_init);
13901376
ireq = inet_rsk(req);
13911377

13921378
if (sk_acceptq_is_full(sk))

net/mptcp/subflow.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,9 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
808808
struct request_sock *req,
809809
struct dst_entry *dst,
810810
struct request_sock *req_unhash,
811-
bool *own_req)
811+
bool *own_req,
812+
void (*opt_child_init)(struct sock *newsk,
813+
const struct sock *sk))
812814
{
813815
struct mptcp_subflow_context *listener = mptcp_subflow_ctx(sk);
814816
struct mptcp_subflow_request_sock *subflow_req;
@@ -855,7 +857,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
855857

856858
create_child:
857859
child = listener->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
858-
req_unhash, own_req);
860+
req_unhash, own_req, opt_child_init);
859861

860862
if (child && *own_req) {
861863
struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(child);

net/smc/af_smc.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
124124
struct request_sock *req,
125125
struct dst_entry *dst,
126126
struct request_sock *req_unhash,
127-
bool *own_req)
127+
bool *own_req,
128+
void (*opt_child_init)(struct sock *newsk,
129+
const struct sock *sk))
128130
{
129131
struct smc_sock *smc;
130132
struct sock *child;
@@ -142,7 +144,7 @@ static struct sock *smc_tcp_syn_recv_sock(const struct sock *sk,
142144

143145
/* passthrough to original syn recv sock fct */
144146
child = smc->ori_af_ops->syn_recv_sock(sk, skb, req, dst, req_unhash,
145-
own_req);
147+
own_req, opt_child_init);
146148
/* child must not inherit smc or its ops */
147149
if (child) {
148150
rcu_assign_sk_user_data(child, NULL);

0 commit comments

Comments
 (0)