|
Takashi Iwai |
b1f25e |
From 49054556289e8787501630b7c7a9d407da02e296 Mon Sep 17 00:00:00 2001
|
|
Takashi Iwai |
b1f25e |
From: Paolo Abeni <pabeni@redhat.com>
|
|
Takashi Iwai |
b1f25e |
Date: Wed, 29 Sep 2021 11:59:17 +0200
|
|
Takashi Iwai |
b1f25e |
Subject: [PATCH] net: introduce and use lock_sock_fast_nested()
|
|
Takashi Iwai |
b1f25e |
Git-commit: 49054556289e8787501630b7c7a9d407da02e296
|
|
Takashi Iwai |
b1f25e |
Patch-mainline: v5.15-rc4
|
|
Takashi Iwai |
b1f25e |
References: git-fixes
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
Syzkaller reported a false positive deadlock involving
|
|
Takashi Iwai |
b1f25e |
the nl socket lock and the subflow socket lock:
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
Mptcp: kernel_bind error, err=-98
|
|
Takashi Iwai |
b1f25e |
============================================
|
|
Takashi Iwai |
b1f25e |
Warning: possible recursive locking detected
|
|
Takashi Iwai |
b1f25e |
5.15.0-rc1-syzkaller #0 Not tainted
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
Acked-by: Takashi Iwai <tiwai@suse.de>
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
--------------------------------------------
|
|
Takashi Iwai |
b1f25e |
syz-executor998/6520 is trying to acquire lock:
|
|
Takashi Iwai |
b1f25e |
ffff8880795718a0 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x267/0x7b0 net/mptcp/protocol.c:2738
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
but task is already holding lock:
|
|
Takashi Iwai |
b1f25e |
ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1612 [inline]
|
|
Takashi Iwai |
b1f25e |
ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x23/0x7b0 net/mptcp/protocol.c:2720
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
other info that might help us debug this:
|
|
Takashi Iwai |
b1f25e |
Possible unsafe locking scenario:
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
CPU0
|
|
Takashi Iwai |
b1f25e |
----
|
|
Takashi Iwai |
b1f25e |
lock(k-sk_lock-AF_INET);
|
|
Takashi Iwai |
b1f25e |
lock(k-sk_lock-AF_INET);
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
*** DEADLOCK ***
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
May be due to missing lock nesting notation
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
3 locks held by syz-executor998/6520:
|
|
Takashi Iwai |
b1f25e |
#0: ffffffff8d176c50 (cb_lock){++++}-{3:3}, at: genl_rcv+0x15/0x40 net/netlink/genetlink.c:802
|
|
Takashi Iwai |
b1f25e |
#1: ffffffff8d176d08 (genl_mutex){+.+.}-{3:3}, at: genl_lock net/netlink/genetlink.c:33 [inline]
|
|
Takashi Iwai |
b1f25e |
#1: ffffffff8d176d08 (genl_mutex){+.+.}-{3:3}, at: genl_rcv_msg+0x3e0/0x580 net/netlink/genetlink.c:790
|
|
Takashi Iwai |
b1f25e |
#2: ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1612 [inline]
|
|
Takashi Iwai |
b1f25e |
#2: ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x23/0x7b0 net/mptcp/protocol.c:2720
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
stack backtrace:
|
|
Takashi Iwai |
b1f25e |
CPU: 1 PID: 6520 Comm: syz-executor998 Not tainted 5.15.0-rc1-syzkaller #0
|
|
Takashi Iwai |
b1f25e |
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
|
|
Takashi Iwai |
b1f25e |
Call Trace:
|
|
Takashi Iwai |
b1f25e |
__dump_stack lib/dump_stack.c:88 [inline]
|
|
Takashi Iwai |
b1f25e |
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
|
|
Takashi Iwai |
b1f25e |
print_deadlock_bug kernel/locking/lockdep.c:2944 [inline]
|
|
Takashi Iwai |
b1f25e |
check_deadlock kernel/locking/lockdep.c:2987 [inline]
|
|
Takashi Iwai |
b1f25e |
validate_chain kernel/locking/lockdep.c:3776 [inline]
|
|
Takashi Iwai |
b1f25e |
__lock_acquire.cold+0x149/0x3ab kernel/locking/lockdep.c:5015
|
|
Takashi Iwai |
b1f25e |
lock_acquire kernel/locking/lockdep.c:5625 [inline]
|
|
Takashi Iwai |
b1f25e |
lock_acquire+0x1ab/0x510 kernel/locking/lockdep.c:5590
|
|
Takashi Iwai |
b1f25e |
lock_sock_fast+0x36/0x100 net/core/sock.c:3229
|
|
Takashi Iwai |
b1f25e |
mptcp_close+0x267/0x7b0 net/mptcp/protocol.c:2738
|
|
Takashi Iwai |
b1f25e |
inet_release+0x12e/0x280 net/ipv4/af_inet.c:431
|
|
Takashi Iwai |
b1f25e |
__sock_release net/socket.c:649 [inline]
|
|
Takashi Iwai |
b1f25e |
sock_release+0x87/0x1b0 net/socket.c:677
|
|
Takashi Iwai |
b1f25e |
mptcp_pm_nl_create_listen_socket+0x238/0x2c0 net/mptcp/pm_netlink.c:900
|
|
Takashi Iwai |
b1f25e |
mptcp_nl_cmd_add_addr+0x359/0x930 net/mptcp/pm_netlink.c:1170
|
|
Takashi Iwai |
b1f25e |
genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:731
|
|
Takashi Iwai |
b1f25e |
genl_family_rcv_msg net/netlink/genetlink.c:775 [inline]
|
|
Takashi Iwai |
b1f25e |
genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:792
|
|
Takashi Iwai |
b1f25e |
netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2504
|
|
Takashi Iwai |
b1f25e |
genl_rcv+0x24/0x40 net/netlink/genetlink.c:803
|
|
Takashi Iwai |
b1f25e |
netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
|
|
Takashi Iwai |
b1f25e |
netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1340
|
|
Takashi Iwai |
b1f25e |
netlink_sendmsg+0x86d/0xdb0 net/netlink/af_netlink.c:1929
|
|
Takashi Iwai |
b1f25e |
sock_sendmsg_nosec net/socket.c:704 [inline]
|
|
Takashi Iwai |
b1f25e |
sock_sendmsg+0xcf/0x120 net/socket.c:724
|
|
Takashi Iwai |
b1f25e |
sock_no_sendpage+0x101/0x150 net/core/sock.c:2980
|
|
Takashi Iwai |
b1f25e |
kernel_sendpage.part.0+0x1a0/0x340 net/socket.c:3504
|
|
Takashi Iwai |
b1f25e |
kernel_sendpage net/socket.c:3501 [inline]
|
|
Takashi Iwai |
b1f25e |
sock_sendpage+0xe5/0x140 net/socket.c:1003
|
|
Takashi Iwai |
b1f25e |
pipe_to_sendpage+0x2ad/0x380 fs/splice.c:364
|
|
Takashi Iwai |
b1f25e |
splice_from_pipe_feed fs/splice.c:418 [inline]
|
|
Takashi Iwai |
b1f25e |
__splice_from_pipe+0x43e/0x8a0 fs/splice.c:562
|
|
Takashi Iwai |
b1f25e |
splice_from_pipe fs/splice.c:597 [inline]
|
|
Takashi Iwai |
b1f25e |
generic_splice_sendpage+0xd4/0x140 fs/splice.c:746
|
|
Takashi Iwai |
b1f25e |
do_splice_from fs/splice.c:767 [inline]
|
|
Takashi Iwai |
b1f25e |
direct_splice_actor+0x110/0x180 fs/splice.c:936
|
|
Takashi Iwai |
b1f25e |
splice_direct_to_actor+0x34b/0x8c0 fs/splice.c:891
|
|
Takashi Iwai |
b1f25e |
do_splice_direct+0x1b3/0x280 fs/splice.c:979
|
|
Takashi Iwai |
b1f25e |
do_sendfile+0xae9/0x1240 fs/read_write.c:1249
|
|
Takashi Iwai |
b1f25e |
__do_sys_sendfile64 fs/read_write.c:1314 [inline]
|
|
Takashi Iwai |
b1f25e |
__se_sys_sendfile64 fs/read_write.c:1300 [inline]
|
|
Takashi Iwai |
b1f25e |
__x64_sys_sendfile64+0x1cc/0x210 fs/read_write.c:1300
|
|
Takashi Iwai |
b1f25e |
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
|
|
Takashi Iwai |
b1f25e |
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
|
|
Takashi Iwai |
b1f25e |
entry_SYSCALL_64_after_hwframe+0x44/0xae
|
|
Takashi Iwai |
b1f25e |
RIP: 0033:0x7f215cb69969
|
|
Takashi Iwai |
b1f25e |
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 14 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
|
|
Takashi Iwai |
b1f25e |
RSP: 002b:00007ffc96bb3868 EFLAGS: 00000246 ORIG_RAX: 0000000000000028
|
|
Takashi Iwai |
b1f25e |
RAX: ffffffffffffffda RBX: 00007f215cbad072 RCX: 00007f215cb69969
|
|
Takashi Iwai |
b1f25e |
RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000005
|
|
Takashi Iwai |
b1f25e |
RBP: 0000000000000000 R08: 00007ffc96bb3a08 R09: 00007ffc96bb3a08
|
|
Takashi Iwai |
b1f25e |
R10: 0000000100000002 R11: 0000000000000246 R12: 00007ffc96bb387c
|
|
Takashi Iwai |
b1f25e |
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
the problem originates from uncorrect lock annotation in the mptcp
|
|
Takashi Iwai |
b1f25e |
code and is only visible since commit 2dcb96bacce3 ("net: core: Correct
|
|
Takashi Iwai |
b1f25e |
the sock::sk_lock.owned lockdep annotations"), but is present since
|
|
Takashi Iwai |
b1f25e |
the port-based endpoint support initial implementation.
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
This patch addresses the issue introducing a nested variant of
|
|
Takashi Iwai |
b1f25e |
lock_sock_fast() and using it in the relevant code path.
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
|
|
Takashi Iwai |
b1f25e |
Fixes: 2dcb96bacce3 ("net: core: Correct the sock::sk_lock.owned lockdep annotations")
|
|
Takashi Iwai |
b1f25e |
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
|
|
Takashi Iwai |
b1f25e |
Reported-and-tested-by: syzbot+1dd53f7a89b299d59eaf@syzkaller.appspotmail.com
|
|
Takashi Iwai |
b1f25e |
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Takashi Iwai |
b1f25e |
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
|
|
Takashi Iwai |
b1f25e |
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Takashi Iwai |
b1f25e |
---
|
|
Takashi Iwai |
b1f25e |
include/net/sock.h | 31 ++++++++++++++++++++++++++++++-
|
|
Takashi Iwai |
b1f25e |
net/core/sock.c | 20 ++------------------
|
|
Takashi Iwai |
b1f25e |
net/mptcp/protocol.c | 2 +-
|
|
Takashi Iwai |
b1f25e |
3 files changed, 33 insertions(+), 20 deletions(-)
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
diff --git a/include/net/sock.h b/include/net/sock.h
|
|
Takashi Iwai |
b1f25e |
index c005c3c750e8..dc3f8169312e 100644
|
|
Takashi Iwai |
b1f25e |
--- a/include/net/sock.h
|
|
Takashi Iwai |
b1f25e |
+++ b/include/net/sock.h
|
|
Takashi Iwai |
b1f25e |
@@ -1623,7 +1623,36 @@ void release_sock(struct sock *sk);
|
|
Takashi Iwai |
b1f25e |
SINGLE_DEPTH_NESTING)
|
|
Takashi Iwai |
b1f25e |
#define bh_unlock_sock(__sk) spin_unlock(&((__sk)->sk_lock.slock))
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
-bool lock_sock_fast(struct sock *sk) __acquires(&sk->sk_lock.slock);
|
|
Takashi Iwai |
b1f25e |
+bool __lock_sock_fast(struct sock *sk) __acquires(&sk->sk_lock.slock);
|
|
Takashi Iwai |
b1f25e |
+
|
|
Takashi Iwai |
b1f25e |
+/**
|
|
Takashi Iwai |
b1f25e |
+ * lock_sock_fast - fast version of lock_sock
|
|
Takashi Iwai |
b1f25e |
+ * @sk: socket
|
|
Takashi Iwai |
b1f25e |
+ *
|
|
Takashi Iwai |
b1f25e |
+ * This version should be used for very small section, where process wont block
|
|
Takashi Iwai |
b1f25e |
+ * return false if fast path is taken:
|
|
Takashi Iwai |
b1f25e |
+ *
|
|
Takashi Iwai |
b1f25e |
+ * sk_lock.slock locked, owned = 0, BH disabled
|
|
Takashi Iwai |
b1f25e |
+ *
|
|
Takashi Iwai |
b1f25e |
+ * return true if slow path is taken:
|
|
Takashi Iwai |
b1f25e |
+ *
|
|
Takashi Iwai |
b1f25e |
+ * sk_lock.slock unlocked, owned = 1, BH enabled
|
|
Takashi Iwai |
b1f25e |
+ */
|
|
Takashi Iwai |
b1f25e |
+static inline bool lock_sock_fast(struct sock *sk)
|
|
Takashi Iwai |
b1f25e |
+{
|
|
Takashi Iwai |
b1f25e |
+ /* The sk_lock has mutex_lock() semantics here. */
|
|
Takashi Iwai |
b1f25e |
+ mutex_acquire(&sk->sk_lock.dep_map, 0, 0, _RET_IP_);
|
|
Takashi Iwai |
b1f25e |
+
|
|
Takashi Iwai |
b1f25e |
+ return __lock_sock_fast(sk);
|
|
Takashi Iwai |
b1f25e |
+}
|
|
Takashi Iwai |
b1f25e |
+
|
|
Takashi Iwai |
b1f25e |
+/* fast socket lock variant for caller already holding a [different] socket lock */
|
|
Takashi Iwai |
b1f25e |
+static inline bool lock_sock_fast_nested(struct sock *sk)
|
|
Takashi Iwai |
b1f25e |
+{
|
|
Takashi Iwai |
b1f25e |
+ mutex_acquire(&sk->sk_lock.dep_map, SINGLE_DEPTH_NESTING, 0, _RET_IP_);
|
|
Takashi Iwai |
b1f25e |
+
|
|
Takashi Iwai |
b1f25e |
+ return __lock_sock_fast(sk);
|
|
Takashi Iwai |
b1f25e |
+}
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
/**
|
|
Takashi Iwai |
b1f25e |
* unlock_sock_fast - complement of lock_sock_fast
|
|
Takashi Iwai |
b1f25e |
diff --git a/net/core/sock.c b/net/core/sock.c
|
|
Takashi Iwai |
b1f25e |
index 512e629f9780..7060d183216e 100644
|
|
Takashi Iwai |
b1f25e |
--- a/net/core/sock.c
|
|
Takashi Iwai |
b1f25e |
+++ b/net/core/sock.c
|
|
Takashi Iwai |
b1f25e |
@@ -3210,24 +3210,8 @@ void release_sock(struct sock *sk)
|
|
Takashi Iwai |
b1f25e |
}
|
|
Takashi Iwai |
b1f25e |
EXPORT_SYMBOL(release_sock);
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
-/**
|
|
Takashi Iwai |
b1f25e |
- * lock_sock_fast - fast version of lock_sock
|
|
Takashi Iwai |
b1f25e |
- * @sk: socket
|
|
Takashi Iwai |
b1f25e |
- *
|
|
Takashi Iwai |
b1f25e |
- * This version should be used for very small section, where process wont block
|
|
Takashi Iwai |
b1f25e |
- * return false if fast path is taken:
|
|
Takashi Iwai |
b1f25e |
- *
|
|
Takashi Iwai |
b1f25e |
- * sk_lock.slock locked, owned = 0, BH disabled
|
|
Takashi Iwai |
b1f25e |
- *
|
|
Takashi Iwai |
b1f25e |
- * return true if slow path is taken:
|
|
Takashi Iwai |
b1f25e |
- *
|
|
Takashi Iwai |
b1f25e |
- * sk_lock.slock unlocked, owned = 1, BH enabled
|
|
Takashi Iwai |
b1f25e |
- */
|
|
Takashi Iwai |
b1f25e |
-bool lock_sock_fast(struct sock *sk) __acquires(&sk->sk_lock.slock)
|
|
Takashi Iwai |
b1f25e |
+bool __lock_sock_fast(struct sock *sk) __acquires(&sk->sk_lock.slock)
|
|
Takashi Iwai |
b1f25e |
{
|
|
Takashi Iwai |
b1f25e |
- /* The sk_lock has mutex_lock() semantics here. */
|
|
Takashi Iwai |
b1f25e |
- mutex_acquire(&sk->sk_lock.dep_map, 0, 0, _RET_IP_);
|
|
Takashi Iwai |
b1f25e |
-
|
|
Takashi Iwai |
b1f25e |
might_sleep();
|
|
Takashi Iwai |
b1f25e |
spin_lock_bh(&sk->sk_lock.slock);
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
@@ -3256,7 +3240,7 @@ bool lock_sock_fast(struct sock *sk) __acquires(&sk->sk_lock.slock)
|
|
Takashi Iwai |
b1f25e |
spin_unlock_bh(&sk->sk_lock.slock);
|
|
Takashi Iwai |
b1f25e |
return true;
|
|
Takashi Iwai |
b1f25e |
}
|
|
Takashi Iwai |
b1f25e |
-EXPORT_SYMBOL(lock_sock_fast);
|
|
Takashi Iwai |
b1f25e |
+EXPORT_SYMBOL(__lock_sock_fast);
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
int sock_gettstamp(struct socket *sock, void __user *userstamp,
|
|
Takashi Iwai |
b1f25e |
bool timeval, bool time32)
|
|
Takashi Iwai |
b1f25e |
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
|
|
Takashi Iwai |
b1f25e |
index dbcebf56798f..e5df0b5971c8 100644
|
|
Takashi Iwai |
b1f25e |
--- a/net/mptcp/protocol.c
|
|
Takashi Iwai |
b1f25e |
+++ b/net/mptcp/protocol.c
|
|
Takashi Iwai |
b1f25e |
@@ -2735,7 +2735,7 @@ static void mptcp_close(struct sock *sk, long timeout)
|
|
Takashi Iwai |
b1f25e |
inet_csk(sk)->icsk_mtup.probe_timestamp = tcp_jiffies32;
|
|
Takashi Iwai |
b1f25e |
mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
|
|
Takashi Iwai |
b1f25e |
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
|
|
Takashi Iwai |
b1f25e |
- bool slow = lock_sock_fast(ssk);
|
|
Takashi Iwai |
b1f25e |
+ bool slow = lock_sock_fast_nested(ssk);
|
|
Takashi Iwai |
b1f25e |
|
|
Takashi Iwai |
b1f25e |
sock_orphan(ssk);
|
|
Takashi Iwai |
b1f25e |
unlock_sock_fast(ssk, slow);
|
|
Takashi Iwai |
b1f25e |
--
|
|
Takashi Iwai |
b1f25e |
2.26.2
|
|
Takashi Iwai |
b1f25e |
|