[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240608132544.GF27689@kernel.org>
Date: Sat, 8 Jun 2024 14:25:44 +0100
From: Simon Horman <horms@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org, Neal Cardwell <ncardwell@...gle.com>,
eric.dumazet@...il.com, David Ahern <dsahern@...nel.org>
Subject: Re: [PATCH net] tcp: fix race in tcp_v6_syn_recv_sock()
+ David Ahern
On Thu, Jun 06, 2024 at 03:46:51PM +0000, Eric Dumazet wrote:
> tcp_v6_syn_recv_sock() calls ip6_dst_store() before
> inet_sk(newsk)->pinet6 has been set up.
>
> This means ip6_dst_store() writes over the parent (listener)
> np->dst_cookie.
>
> This is racy because multiple threads could share the same
> parent and their final np->dst_cookie could be wrong.
>
> Move ip6_dst_store() call after inet_sk(newsk)->pinet6
> has been changed and after the copy of parent ipv6_pinfo.
>
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
Reviewed-by: Simon Horman <horms@...nel.org>
> ---
> net/ipv6/tcp_ipv6.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 8c577b651bfcd2f94b45e339ed4a2b47e93ff17a..729faf8bd366ad25d093a4ae931fb46ebd45b79c 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1439,7 +1439,6 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
> */
>
> newsk->sk_gso_type = SKB_GSO_TCPV6;
> - ip6_dst_store(newsk, dst, NULL, NULL);
> inet6_sk_rx_dst_set(newsk, skb);
>
> inet_sk(newsk)->pinet6 = tcp_inet6_sk(newsk);
> @@ -1450,6 +1449,8 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
>
> memcpy(newnp, np, sizeof(struct ipv6_pinfo));
>
> + ip6_dst_store(newsk, dst, NULL, NULL);
> +
> newsk->sk_v6_daddr = ireq->ir_v6_rmt_addr;
> newnp->saddr = ireq->ir_v6_loc_addr;
> newsk->sk_v6_rcv_saddr = ireq->ir_v6_loc_addr;
> --
> 2.45.1.467.gbab1589fc0-goog
>
>
Powered by blists - more mailing lists