Re: [PATCH mptcp-next v2 1/2] mptcp: give MPTCP sockets a distinct lockdep class

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: lockdep warning fix bypassed for passive sockets
--

commit 554f965b1a52d1d1bbbc1a03f3a216e91642e81f
Author: Geliang Tang <geliang@kernel.org>

mptcp: give MPTCP sockets a distinct lockdep class

This commit extracts lock reclassification logic into mptcp_reclassify_socket()
and calls it from __mptcp_init_sock() to fix a lockdep "possible recursive
locking" warning. It aims to cover both active and passive socket creation
paths.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 60cbd91be6320..68f339311993e 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -3245,6 +3245,9 @@ static void __mptcp_init_sock(struct sock *sk)
>  	/* re-use the csk retrans timer for MPTCP-level retrans */
>  	timer_setup(&sk->mptcp_retransmit_timer, mptcp_retransmit_timer, 0);
>  	timer_setup(&msk->sk.mptcp_tout_timer, mptcp_tout_timer, 0);
> +
> +	if (sk->sk_socket)
> +		mptcp_reclassify_socket(sk);

[Severity: Medium]
Does this check evaluate to false for cloned passive sockets, bypassing the
lock reclassification entirely?

During passive socket creation, mptcp_sk_clone_init() calls sk_clone_lock(),
which clones the socket, acquires its spinlock, and explicitly clears sk_socket:

net/core/sock.c:sk_clone() {
    ...
    sk_set_socket(newsk, NULL);
    ...
}

When mptcp_sk_clone_init() then calls __mptcp_init_sock(), sk->sk_socket
is NULL, causing the intended lockdep fix to be bypassed for passive sockets.

Additionally, if mptcp_reclassify_socket() were called unconditionally here
without the sk_socket check, would it re-initialize the actively held spinlock
from sk_clone_lock() and cause a BUG on unlock?

>  }
>  
>  static void mptcp_ca_reset(struct sock *sk)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792114102.git.tanggeliang@kylinos.cn?part=1