[syzkaller] UBSAN: shift-out-of-bounds in net/mptcp/protocol.c

[mjmartineau]

Syzkaller & UBSAN found an issue where a large number of retransmits caused an invalid timer_ival:

================================================================================
UBSAN: shift-out-of-bounds in net/mptcp/protocol.c:470:29
shift exponent 32 is too large for 32-bit type 'unsigned int'
CPU: 1 PID: 13059 Comm: kworker/1:0 Not tainted 5.17.0-rc2-00630-g5fbf21c90c60 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Workqueue: events mptcp_worker
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
 ubsan_epilogue+0xb/0x5a lib/ubsan.c:151
 __ubsan_handle_shift_out_of_bounds.cold+0xb2/0x20e lib/ubsan.c:330
 mptcp_set_datafin_timeout net/mptcp/protocol.c:470 [inline]
 __mptcp_retrans.cold+0x72/0x77 net/mptcp/protocol.c:2445
 mptcp_worker+0x58a/0xa70 net/mptcp/protocol.c:2528
 process_one_work+0x9df/0x16d0 kernel/workqueue.c:2307
 worker_thread+0x95/0xe10 kernel/workqueue.c:2454
 kthread+0x2f4/0x3b0 kernel/kthread.c:377
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
 </TASK>
================================================================================

The code around protocol.c:470:

static void mptcp_set_datafin_timeout(const struct sock *sk)                                                                                                  
{                                                                                                                                                             
        struct inet_connection_sock *icsk = inet_csk(sk);                                                                                                     
                                                                                                                                                              
        mptcp_sk(sk)->timer_ival = min(TCP_RTO_MAX,                                                                                                           
                                       TCP_RTO_MIN << icsk->icsk_retransmits);                                                                                
}                                                                                                                                                             

Export branch, tag export/20220204T054625

Full syzkaller log: log-ubsan.gz
Kernel config: config-ubsan.gz

[mjmartineau]

We did discuss this function before merging the code: https://lore.kernel.org/mptcp/a3685615-f9d7-933-fd5a-6d253acf42@linux.intel.com/

One option is to go back to the original proposal (with a comment to explain the constraints on retransmits):

static void mptcp_set_datafin_timeout(const struct sock *sk)
{
	struct inet_connection_sock *icsk = inet_csk(sk);
	int retransmits = clamp_val(icsk->icsk_retransmits, 1,
				    ilog2(TCP_RTO_MAX / TCP_RTO_MIN));

	mptcp_sk(sk)->timer_ival = TCP_RTO_MIN << retransmits;
}

[pabeni]

static void mptcp_set_datafin_timeout(const struct sock *sk)
{
struct inet_connection_sock *icsk = inet_csk(sk);
int retransmits = clamp_val(icsk->icsk_retransmits, 1,
ilog2(TCP_RTO_MAX / TCP_RTO_MIN));

mptcp_sk(sk)->timer_ival = TCP_RTO_MIN << retransmits;
}

or possibly:

u32 retransmits = min(icsk->icsk_retransmits, ilog2(TCP_RTO_MAX / TCP_RTO_MIN));
    mptcp_sk(sk)->timer_ival = TCP_RTO_MIN << retransmits;