Re: [PATCH net v2 2/5] ipv4: fix shift-out-of-bounds in ip_rt_send_redirect()

[Fernando Fernandez Mancera]

On 10/27/26 11:45 AM, Jiayuan Chen wrote:
> ip_rt_redirect_number is the max number of ICMP Redirects we send to one
> endpoint; once we hit it we stop. Before that, ip_rt_redirect_load gives
> the redirects an exponential backoff:
>
> 	if (peer->n_redirects == 0 ||
> 	    time_after(jiffies,
> 		       (peer->rate_last +
> 			(ip_rt_redirect_load << peer->n_redirects)))) {
>
> n_redirects grows until it reaches ip_rt_redirect_number, and both
> redirect_number and redirect_load are plain proc_dointvec sysctls with
> no upper bound. Set redirect_number past 32 and n_redirects climbs past
> 31, so "ip_rt_redirect_load << peer->n_redirects" shifts a 32-bit int by
> > = 32 bits, which is UB:
>
>    UBSAN: shift-out-of-bounds in net/ipv4/route.c:949:6
>    shift exponent 32 is too large for 32-bit type 'int'
>    CPU: 1 UID: 0 PID: 1845 Comm: ping Not tainted 7.3.0-rc7+ #364 PREEMPT
>    Call Trace:
>     <IRQ>
>     dump_stack_lvl+0xc1/0xf0
>     dump_stack+0x10/0x20
>     __ubsan_handle_shift_out_of_bounds+0x1d1/0x390
>     ip_rt_send_redirect.cold+0x2c/0x146
>     ip_forward+0x1ddc/0x2430
>     ip_rcv+0x2b1/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
>
> Bounding redirect_number instead is not an option: 0 disables redirects
> and -1 removes the cap. Compute the backoff in u64 with the shift count
> capped at 31, which cannot overflow once ip_rt_redirect_load is
> non-negative, and clamp the result to MAX_JIFFY_OFFSET: on 32-bit a
> backoff of 2^31 or more is read by time_after() as already expired. This
> keeps redirect_number as the limit on how many ICMP redirects are sent,
> and the shift can no longer go out of bounds.
>
> redirect_load on the other hand gets a lower bound of 0. A negative
> backoff never meant anything, the old code only happened to treat it as
> no backoff, and once converted to u64 it would turn into the maximum
> backoff instead.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>

Reviewed-by: Fernando Fernandez Mancera <fmancera@xxxxxxx>

Thanks!