Re: [PATCH net v2] mptcp: fix soft lockup in mptcp_recvmsg()

[Li Xiasong <lixiasong1@huawei.com>]

Hi Matt,

On 3/25/2026 3:23 AM, Matthieu Baerts wrote:
> Hi Li,
> 
> On 24/03/2026 09:51, Li Xiasong wrote:
>> syzbot reported a soft lockup in mptcp_recvmsg() [0].
>>
>> When receiving data with MSG_PEEK | MSG_WAITALL flags, the skb is not
>> removed from the sk_receive_queue. This causes sk_wait_data() to always
>> find available data and never perform actual waiting, leading to a soft
>> lockup.
>>
>> Fix this by adding a 'last' parameter to track the last peeked skb.
>> This allows sk_wait_data() to make informed waiting decisions and prevent
>> infinite loops when MSG_PEEK is used.
> 
> (...)
> 
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index cf1852b99963..401fb2b17685 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -2006,7 +2006,7 @@ static void mptcp_eat_recv_skb(struct sock *sk, struct sk_buff *skb)
>>  static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
>>  				size_t len, int flags, int copied_total,
>>  				struct scm_timestamping_internal *tss,
>> -				int *cmsg_flags)
>> +				int *cmsg_flags, struct sk_buff **last)
>>  {
>>  	struct mptcp_sock *msk = mptcp_sk(sk);
>>  	struct sk_buff *skb, *tmp;
>> @@ -2048,6 +2048,7 @@ static int __mptcp_recvmsg_mskq(struct sock *sk, struct msghdr *msg,
>>  		}
>>  
>>  		copied += count;
>> +		*last = skb;
> 
> My bad, my recommendation to move this assignment here was not a good
> idea, because 'skb' can be freed at some points after mptcp_eat_recv_skb
> that can be called here below.
> 
> If I'm not mistaken, when MSG_PEEK is not used, sk_wait_data() can
> always be called with NULL for the last skb, because the queue is
> supposed to be empty. If not, no need to wait for new packets, so NULL
> can be used. Is that correct?
> 
> If yes, then I guess 'last' can be initialised to NULL before calling
> __mptcp_recvmsg_mskq (limit scope), and only set to 'skb' here below,
> when MSG_PEEK is not used (what you had in v1).
> 
> Would that work for you?
> 
> Cheers,
> Matt

Thanks for the review. I analyzed the concern about the 'last' pointer.

When writing the v2 patch, I did consider this case - in non-MSG_PEEK
scenario, the skb is freed by mptcp_eat_recv_skb() after setting *last =
skb, making 'last' point to freed memory. However, in sk_wait_data(), the
'last' parameter is only used for pointer comparison:

    skb_peek_tail(&sk->sk_receive_queue) != skb

It only compares the pointer value without dereferencing, so there's no
actual UAF issue.

However, I agree it's still risky to keep a dangling pointer around. I
appreciate your suggestion and will adopt it in v3.

Best regards,
Li Xiasong