Re: [PATCH net v2 2/2] tcp: fix shift-out-of-bounds in inet_csk_rto_backoff()

[Neal Cardwell]

On Mon, Oct 26, 2026 at 8:33 AM Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
>
> icsk_backoff is incremented by tcp_retransmit_timer() on every backed-off
> RTO with no cap of its own, and by tcp_send_probe0() up to tcp_retries2.
> It is used as a shift amount in inet_csk_rto_backoff():
>
>         u64 when = (u64)icsk->icsk_rto << icsk->icsk_backoff;
>
> Once it climbs past 63 the shift is UB. Seen with tcp_retries2=255 and a
> silent peer, when an ICMP host unreachable goes through
> tcp_ld_RTO_revert():
>
>   UBSAN: shift-out-of-bounds in ./include/net/inet_connection_sock.h:258:40
>   shift exponent 73 is too large for 64-bit type 'long long unsigned int'
>   Call Trace:
>    <IRQ>
>    dump_stack_lvl+0xc1/0xf0
>    dump_stack+0x10/0x20
>    __ubsan_handle_shift_out_of_bounds+0x1d1/0x390
>    tcp_ld_RTO_revert.cold+0x16/0x92
>    tcp_v4_err+0xdb3/0x13e0
>    icmp_socket_deliver+0x1a7/0x390
>    icmp_unreach+0x2dd/0xe50
>    icmp_rcv+0x93e/0xe30
>    ip_protocol_deliver_rcu+0x36b/0x3e0
>    ip_local_deliver_finish+0x37d/0x620
>    ip_local_deliver+0x165/0x1d0
>    ip_rcv+0x284/0x320
>    __netif_receive_skb_one_core+0x16b/0x1c0
>    __netif_receive_skb+0x21/0x150
>    process_backlog+0x37e/0x1200
>    __napi_poll.constprop.0+0xa5/0x5e0
>    net_rx_action+0x4c0/0xf70
>    handle_softirqs+0x204/0x9b0
>    __irq_exit_rcu+0x13c/0x220
>    irq_exit_rcu+0xe/0x20
>    sysvec_apic_timer_interrupt+0x92/0xd0
>    </IRQ>
>
> The orphan check in tcp_probe_timer() calls the same helper. On x86-64
> the oversized shift count is taken modulo 64, so an orphan that should
> be dead can be seen as alive, and tcp_ld_RTO_revert() can end up with a
> smaller RTO than intended.
>
> Cap the shift at 32: any non-zero icsk_rto << 32 exceeds TCP_RTO_MAX, so
> the min_t() result is what the unbounded math would give. The
> ilog2(TCP_RTO_MAX / TCP_RTO_MIN) + 1 bound of tcp_probe0_when() does not
> fit here, as icsk_rto has no TCP_RTO_MIN floor the way tcp_probe0_base()
> has.
>
> Fixes: fcdd1cf4dd63 ("tcp: avoid possible arithmetic overflows")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
> ---
>  include/net/inet_connection_sock.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> index 903499581db7..562e12033a5d 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -255,9 +255,10 @@ static inline unsigned long
>  inet_csk_rto_backoff(const struct inet_connection_sock *icsk,
>                      unsigned long max_when)
>  {
> -        u64 when = (u64)icsk->icsk_rto << icsk->icsk_backoff;
> +       u8 backoff = min_t(u8, 32U, icsk->icsk_backoff);
> +       u64 when = (u64)icsk->icsk_rto << backoff;
>
> -        return (unsigned long)min_t(u64, when, max_when);
> +       return (unsigned long)min_t(u64, when, max_when);
>  }
>
>  struct sock *inet_csk_accept(struct sock *sk, struct proto_accept_arg *arg);
> --
> 2.43.0

The following review comments are from an LLM, look legitimate, and
IMHO are worth addressing.

Please either incorporate each piece of feedback or respond to
indicate why you think it's not appropriate.

---

This is exactly the diff Eric suggested in 2020:
https://lore.kernel.org/netdev/CANn89i+=QRibXsm6tw-eVwhB3wjOtgF-MtVu0G--K5=oWP97+A@xxxxxxxxxxxxxx/
Please add "Suggested-by: Eric Dumazet <edumazet@xxxxxxxxxx>" and a Link:
to that message.

As for 1/2, please add the discovery/validation/testing statement
required by maintainer-netdev.rst.

---

Thanks,
neal