net/mptcp/pm_netlink.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
Christoph reports a lockdep splat in the mptcp_subflow_create_socket()
error path, when such function is invoked by
mptcp_pm_nl_create_listen_socket().
Such code path acquires two separates, nested socket lock, with the
internal lock operation lacking the "nested" annotation. Adding that
in sock_release() for mptcp's sake only could be confusing.
Instead just add a new lockclass to the in-kernel msk socket,
re-initializing the lockdep infra after the socket creation.
Reported-by: Christoph Paasch <cpaasch@apple.com>
Fixes: ad2171009d96 ("mptcp: fix locking for in-kernel listener creation")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/pm_netlink.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index d1d859517d91..6048fdee404b 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -994,9 +994,13 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
return ret;
}
+static struct lock_class_key mptcp_slock_keys[2];
+static struct lock_class_key mptcp_keys[2];
+
static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
struct mptcp_pm_addr_entry *entry)
{
+ bool is_ipv6 = sk->sk_family == AF_INET6;
int addrlen = sizeof(struct sockaddr_in);
struct sockaddr_storage addr;
struct socket *ssock;
@@ -1013,6 +1017,19 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
if (!newsk)
return -EINVAL;
+ /* The subflow socket lock is acquired in a nested to the msk one
+ * in several places, even by the TCP stack, and this msk is a kernel
+ * socket: lockdep complains. Instead of propagating the _nested
+ * modifiers in several places, re-init the lock class for the msk
+ * socket to an mptcp specific one.
+ */
+ sock_lock_init_class_and_name(
+ newsk,
+ is_ipv6 ? "mlock-AF_INET6" : "mlock-AF_INET",
+ &mptcp_slock_keys[is_ipv6],
+ is_ipv6 ? "msk_lock-AF_INET6": "msk_lock-AF_INET",
+ &mptcp_keys[is_ipv6]);
+
lock_sock(newsk);
ssock = __mptcp_nmpc_socket(mptcp_sk(newsk));
release_sock(newsk);
--
2.39.2On Tue, 2023-02-28 at 17:19 +0100, Paolo Abeni wrote:
> Christoph reports a lockdep splat in the mptcp_subflow_create_socket()
> error path, when such function is invoked by
> mptcp_pm_nl_create_listen_socket().
>
> Such code path acquires two separates, nested socket lock, with the
> internal lock operation lacking the "nested" annotation. Adding that
> in sock_release() for mptcp's sake only could be confusing.
>
> Instead just add a new lockclass to the in-kernel msk socket,
> re-initializing the lockdep infra after the socket creation.
>
> Reported-by: Christoph Paasch <cpaasch@apple.com>
> Fixes: ad2171009d96 ("mptcp: fix locking for in-kernel listener creation")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Sorry for the unintentional dup. PEBKAC hit again here :(
I'll drop this one from PW.
/P
Hi Paolo, Christoph,
On 24/02/2023 13:01, Paolo Abeni wrote:
> Christoph reports a lockdep splat in the mptcp_subflow_create_socket()
> error path, when such function is invoked by
> mptcp_pm_nl_create_listen_socket().
>
> Such code path acquires two separates, nested socket lock, with the
> internal lock operation lacking the "nested" annotation. Adding that
> in sock_release() for mptcp's sake only could be confusing.
>
> Instead just add a new lockclass to the in-kernel msk socket,
> re-initializing the lockdep infra after the socket creation.
>
> Reported-by: Christoph Paasch <cpaasch@apple.com>
> Fixes: ad2171009d96 ("mptcp: fix locking for in-kernel listener creation")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Thank you for the patch and for having tested it!
I just applied this patch in our tree (fixes for -net) with Christoph's
Tested-by tag and without the issues reported by checkpatch.pl (I hope
that's OK).
New patches for t/upstream-net and t/upstream:
- 6f002c31c364: mptcp: fix lockdep false positive in
mptcp_pm_nl_create_listen_socket()
- Results: 48ec0932754d..8d03cb1e10a4 (export-net)
- Results: 3bf7fef56098..f84e6823c4bb (export)
Tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230227T211242
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230227T211242
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Hi Paolo,
On 24/02/2023 13:01, Paolo Abeni wrote:
> Christoph reports a lockdep splat in the mptcp_subflow_create_socket()
> error path, when such function is invoked by
> mptcp_pm_nl_create_listen_socket().
>
> Such code path acquires two separates, nested socket lock, with the
> internal lock operation lacking the "nested" annotation. Adding that
> in sock_release() for mptcp's sake only could be confusing.
>
> Instead just add a new lockclass to the in-kernel msk socket,
> re-initializing the lockdep infra after the socket creation.
>
> Reported-by: Christoph Paasch <cpaasch@apple.com>
> Fixes: ad2171009d96 ("mptcp: fix locking for in-kernel listener creation")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Thank you for the fix!
It looks "simpler" than expected, no? :)
The modification seems to make sense to me:
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
I guess we can also add:
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/354
It would be good if Christoph could validate this modification :)
I can wait a bit before including this patch.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
On Fri, 2023-02-24 at 15:02 +0100, Matthieu Baerts wrote:
> Hi Paolo,
>
> On 24/02/2023 13:01, Paolo Abeni wrote:
> > Christoph reports a lockdep splat in the mptcp_subflow_create_socket()
> > error path, when such function is invoked by
> > mptcp_pm_nl_create_listen_socket().
> >
> > Such code path acquires two separates, nested socket lock, with the
> > internal lock operation lacking the "nested" annotation. Adding that
> > in sock_release() for mptcp's sake only could be confusing.
> >
> > Instead just add a new lockclass to the in-kernel msk socket,
> > re-initializing the lockdep infra after the socket creation.
> >
> > Reported-by: Christoph Paasch <cpaasch@apple.com>
> > Fixes: ad2171009d96 ("mptcp: fix locking for in-kernel listener creation")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
> Thank you for the fix!
>
> It looks "simpler" than expected, no? :)
Yep, but I did a few other more complex attempts first.
I think this one should stage a bit in the export branch, to ensure
there are no side effects.
Thanks!
Paolo
© 2016 - 2026 Red Hat, Inc.