Commit dfd25ffffc132c00070eed64200e8950da5d7e9d
Committed by
David S. Miller
David S. Miller
1 parent
4e50391968
Exists in
smarc-l5.0.0_1.0.0-ga
and in
5 other branches
tcp: fix syncookie regression
commit ea4fc0d619 (ipv4: Don't use rt->rt_{src,dst} in ip_queue_xmit()) added a serious regression on synflood handling. Simon Kirby discovered a successful connection was delayed by 20 seconds before being responsive. In my tests, I discovered that xmit frames were lost, and needed ~4 retransmits and a socket dst rebuild before being really sent. In case of syncookie initiated connection, we use a different path to initialize the socket dst, and inet->cork.fl.u.ip4 is left cleared. As ip_queue_xmit() now depends on inet flow being setup, fix this by copying the temp flowi4 we use in cookie_v4_check(). Reported-by: Simon Kirby <sim@netnation.com> Bisected-by: Simon Kirby <sim@netnation.com> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Tested-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 2 changed files with 23 additions and 17 deletions Side-by-side Diff
net/ipv4/syncookies.c
| ... | ... | @@ -278,6 +278,7 @@ |
| 278 | 278 | struct rtable *rt; |
| 279 | 279 | __u8 rcv_wscale; |
| 280 | 280 | bool ecn_ok = false; |
| 281 | + struct flowi4 fl4; | |
| 281 | 282 | |
| 282 | 283 | if (!sysctl_tcp_syncookies || !th->ack || th->rst) |
| 283 | 284 | goto out; |
| ... | ... | @@ -346,20 +347,16 @@ |
| 346 | 347 | * hasn't changed since we received the original syn, but I see |
| 347 | 348 | * no easy way to do this. |
| 348 | 349 | */ |
| 349 | - { | |
| 350 | - struct flowi4 fl4; | |
| 351 | - | |
| 352 | - flowi4_init_output(&fl4, 0, sk->sk_mark, RT_CONN_FLAGS(sk), | |
| 353 | - RT_SCOPE_UNIVERSE, IPPROTO_TCP, | |
| 354 | - inet_sk_flowi_flags(sk), | |
| 355 | - (opt && opt->srr) ? opt->faddr : ireq->rmt_addr, | |
| 356 | - ireq->loc_addr, th->source, th->dest); | |
| 357 | - security_req_classify_flow(req, flowi4_to_flowi(&fl4)); | |
| 358 | - rt = ip_route_output_key(sock_net(sk), &fl4); | |
| 359 | - if (IS_ERR(rt)) { | |
| 360 | - reqsk_free(req); | |
| 361 | - goto out; | |
| 362 | - } | |
| 350 | + flowi4_init_output(&fl4, 0, sk->sk_mark, RT_CONN_FLAGS(sk), | |
| 351 | + RT_SCOPE_UNIVERSE, IPPROTO_TCP, | |
| 352 | + inet_sk_flowi_flags(sk), | |
| 353 | + (opt && opt->srr) ? opt->faddr : ireq->rmt_addr, | |
| 354 | + ireq->loc_addr, th->source, th->dest); | |
| 355 | + security_req_classify_flow(req, flowi4_to_flowi(&fl4)); | |
| 356 | + rt = ip_route_output_key(sock_net(sk), &fl4); | |
| 357 | + if (IS_ERR(rt)) { | |
| 358 | + reqsk_free(req); | |
| 359 | + goto out; | |
| 363 | 360 | } |
| 364 | 361 | |
| 365 | 362 | /* Try to redo what tcp_v4_send_synack did. */ |
| ... | ... | @@ -373,6 +370,11 @@ |
| 373 | 370 | ireq->rcv_wscale = rcv_wscale; |
| 374 | 371 | |
| 375 | 372 | ret = get_cookie_sock(sk, skb, req, &rt->dst); |
| 373 | + /* ip_queue_xmit() depends on our flow being setup | |
| 374 | + * Normal sockets get it right from inet_csk_route_child_sock() | |
| 375 | + */ | |
| 376 | + if (ret) | |
| 377 | + inet_sk(ret)->cork.fl.u.ip4 = fl4; | |
| 376 | 378 | out: return ret; |
| 377 | 379 | } |
net/ipv4/tcp_ipv4.c
| ... | ... | @@ -1466,9 +1466,13 @@ |
| 1466 | 1466 | inet_csk(newsk)->icsk_ext_hdr_len = inet_opt->opt.optlen; |
| 1467 | 1467 | newinet->inet_id = newtp->write_seq ^ jiffies; |
| 1468 | 1468 | |
| 1469 | - if (!dst && (dst = inet_csk_route_child_sock(sk, newsk, req)) == NULL) | |
| 1470 | - goto put_and_exit; | |
| 1471 | - | |
| 1469 | + if (!dst) { | |
| 1470 | + dst = inet_csk_route_child_sock(sk, newsk, req); | |
| 1471 | + if (!dst) | |
| 1472 | + goto put_and_exit; | |
| 1473 | + } else { | |
| 1474 | + /* syncookie case : see end of cookie_v4_check() */ | |
| 1475 | + } | |
| 1472 | 1476 | sk_setup_caps(newsk, dst); |
| 1473 | 1477 | |
| 1474 | 1478 | tcp_mtup_init(newsk); |