Re: [PATCH] tcp: Do not reset the icsk_ca_initialized in tcp_init_transfer.

[Eric Dumazet]

On Mon, Jun 28, 2021 at 4:49 PM Nguyen Dinh Phi <phind.uet@xxxxxxxxx> wrote:
>
> icsk_ca_initialized be always set to zero before we examine it in if
> block, this makes the congestion control module's initialization be
> called even if the CC module was initialized already.
> In case the CC module allocates and setups its dynamically allocated
> private data in its init() function, e.g, CDG, the memory leak may occur.
>
> Reported-by: syzbot+f1e24a0594d4e3a895d3@xxxxxxxxxxxxxxxxxxxxxxxxx
>
> Signed-off-by: Nguyen Dinh Phi <phind.uet@xxxxxxxxx>
> ---
>  net/ipv4/tcp_input.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 7d5e59f688de..855ada2be25e 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5922,7 +5922,6 @@ void tcp_init_transfer(struct sock *sk, int bpf_op, struct sk_buff *skb)
>                 tp->snd_cwnd = tcp_init_cwnd(tp, __sk_dst_get(sk));
>         tp->snd_cwnd_stamp = tcp_jiffies32;
>
> -       icsk->icsk_ca_initialized = 0;

Unfortunately this patch might break things.

We keep changing this CC switching, with eBPF being mixed in the equation.

I would suggest you find a Fixes: tag first, so that we can continue
the discussion.

Thank you.

>         bpf_skops_established(sk, bpf_op, skb);
>         if (!icsk->icsk_ca_initialized)
>                 tcp_init_congestion_control(sk);
> --
> 2.25.1
>