Revision 49054556289e8787501630b7c7a9d407da02e296 authored by Paolo Abeni on 29 September 2021, 09:59:17 UTC, committed by David S. Miller on 30 September 2021, 12:06:47 UTC
Syzkaller reported a false positive deadlock involving
the nl socket lock and the subflow socket lock:
MPTCP: kernel_bind error, err=-98
============================================
WARNING: possible recursive locking detected
5.15.0-rc1-syzkaller #0 Not tainted
--------------------------------------------
syz-executor998/6520 is trying to acquire lock:
ffff8880795718a0 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x267/0x7b0 net/mptcp/protocol.c:2738
but task is already holding lock:
ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1612 [inline]
ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x23/0x7b0 net/mptcp/protocol.c:2720
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(k-sk_lock-AF_INET);
lock(k-sk_lock-AF_INET);
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by syz-executor998/6520:
#0: ffffffff8d176c50 (cb_lock){++++}-{3:3}, at: genl_rcv+0x15/0x40 net/netlink/genetlink.c:802
#1: ffffffff8d176d08 (genl_mutex){+.+.}-{3:3}, at: genl_lock net/netlink/genetlink.c:33 [inline]
#1: ffffffff8d176d08 (genl_mutex){+.+.}-{3:3}, at: genl_rcv_msg+0x3e0/0x580 net/netlink/genetlink.c:790
#2: ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock include/net/sock.h:1612 [inline]
#2: ffff8880787c8c60 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x23/0x7b0 net/mptcp/protocol.c:2720
stack backtrace:
CPU: 1 PID: 6520 Comm: syz-executor998 Not tainted 5.15.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_deadlock_bug kernel/locking/lockdep.c:2944 [inline]
check_deadlock kernel/locking/lockdep.c:2987 [inline]
validate_chain kernel/locking/lockdep.c:3776 [inline]
__lock_acquire.cold+0x149/0x3ab kernel/locking/lockdep.c:5015
lock_acquire kernel/locking/lockdep.c:5625 [inline]
lock_acquire+0x1ab/0x510 kernel/locking/lockdep.c:5590
lock_sock_fast+0x36/0x100 net/core/sock.c:3229
mptcp_close+0x267/0x7b0 net/mptcp/protocol.c:2738
inet_release+0x12e/0x280 net/ipv4/af_inet.c:431
__sock_release net/socket.c:649 [inline]
sock_release+0x87/0x1b0 net/socket.c:677
mptcp_pm_nl_create_listen_socket+0x238/0x2c0 net/mptcp/pm_netlink.c:900
mptcp_nl_cmd_add_addr+0x359/0x930 net/mptcp/pm_netlink.c:1170
genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:731
genl_family_rcv_msg net/netlink/genetlink.c:775 [inline]
genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:792
netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2504
genl_rcv+0x24/0x40 net/netlink/genetlink.c:803
netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1340
netlink_sendmsg+0x86d/0xdb0 net/netlink/af_netlink.c:1929
sock_sendmsg_nosec net/socket.c:704 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:724
sock_no_sendpage+0x101/0x150 net/core/sock.c:2980
kernel_sendpage.part.0+0x1a0/0x340 net/socket.c:3504
kernel_sendpage net/socket.c:3501 [inline]
sock_sendpage+0xe5/0x140 net/socket.c:1003
pipe_to_sendpage+0x2ad/0x380 fs/splice.c:364
splice_from_pipe_feed fs/splice.c:418 [inline]
__splice_from_pipe+0x43e/0x8a0 fs/splice.c:562
splice_from_pipe fs/splice.c:597 [inline]
generic_splice_sendpage+0xd4/0x140 fs/splice.c:746
do_splice_from fs/splice.c:767 [inline]
direct_splice_actor+0x110/0x180 fs/splice.c:936
splice_direct_to_actor+0x34b/0x8c0 fs/splice.c:891
do_splice_direct+0x1b3/0x280 fs/splice.c:979
do_sendfile+0xae9/0x1240 fs/read_write.c:1249
__do_sys_sendfile64 fs/read_write.c:1314 [inline]
__se_sys_sendfile64 fs/read_write.c:1300 [inline]
__x64_sys_sendfile64+0x1cc/0x210 fs/read_write.c:1300
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f215cb69969
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
RSP: 002b:00007ffc96bb3868 EFLAGS: 00000246 ORIG_RAX: 0000000000000028
RAX: ffffffffffffffda RBX: 00007f215cbad072 RCX: 00007f215cb69969
RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000005
RBP: 0000000000000000 R08: 00007ffc96bb3a08 R09: 00007ffc96bb3a08
R10: 0000000100000002 R11: 0000000000000246 R12: 00007ffc96bb387c
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000
the problem originates from uncorrect lock annotation in the mptcp
code and is only visible since commit 2dcb96bacce3 ("net: core: Correct
the sock::sk_lock.owned lockdep annotations"), but is present since
the port-based endpoint support initial implementation.
This patch addresses the issue introducing a nested variant of
lock_sock_fast() and using it in the relevant code path.
Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
Fixes: 2dcb96bacce3 ("net: core: Correct the sock::sk_lock.owned lockdep annotations")
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Reported-and-tested-by: syzbot+1dd53f7a89b299d59eaf@syzkaller.appspotmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent d88fd1b
globtest.c
/*
* Extracted fronm glob.c
*/
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/glob.h>
#include <linux/printk.h>
/* Boot with "glob.verbose=1" to show successful tests, too */
static bool verbose = false;
module_param(verbose, bool, 0);
struct glob_test {
char const *pat, *str;
bool expected;
};
static bool __pure __init test(char const *pat, char const *str, bool expected)
{
bool match = glob_match(pat, str);
bool success = match == expected;
/* Can't get string literals into a particular section, so... */
static char const msg_error[] __initconst =
KERN_ERR "glob: \"%s\" vs. \"%s\": %s *** ERROR ***\n";
static char const msg_ok[] __initconst =
KERN_DEBUG "glob: \"%s\" vs. \"%s\": %s OK\n";
static char const mismatch[] __initconst = "mismatch";
char const *message;
if (!success)
message = msg_error;
else if (verbose)
message = msg_ok;
else
return success;
printk(message, pat, str, mismatch + 3*match);
return success;
}
/*
* The tests are all jammed together in one array to make it simpler
* to place that array in the .init.rodata section. The obvious
* "array of structures containing char *" has no way to force the
* pointed-to strings to be in a particular section.
*
* Anyway, a test consists of:
* 1. Expected glob_match result: '1' or '0'.
* 2. Pattern to match: null-terminated string
* 3. String to match against: null-terminated string
*
* The list of tests is terminated with a final '\0' instead of
* a glob_match result character.
*/
static char const glob_tests[] __initconst =
/* Some basic tests */
"1" "a\0" "a\0"
"0" "a\0" "b\0"
"0" "a\0" "aa\0"
"0" "a\0" "\0"
"1" "\0" "\0"
"0" "\0" "a\0"
/* Simple character class tests */
"1" "[a]\0" "a\0"
"0" "[a]\0" "b\0"
"0" "[!a]\0" "a\0"
"1" "[!a]\0" "b\0"
"1" "[ab]\0" "a\0"
"1" "[ab]\0" "b\0"
"0" "[ab]\0" "c\0"
"1" "[!ab]\0" "c\0"
"1" "[a-c]\0" "b\0"
"0" "[a-c]\0" "d\0"
/* Corner cases in character class parsing */
"1" "[a-c-e-g]\0" "-\0"
"0" "[a-c-e-g]\0" "d\0"
"1" "[a-c-e-g]\0" "f\0"
"1" "[]a-ceg-ik[]\0" "a\0"
"1" "[]a-ceg-ik[]\0" "]\0"
"1" "[]a-ceg-ik[]\0" "[\0"
"1" "[]a-ceg-ik[]\0" "h\0"
"0" "[]a-ceg-ik[]\0" "f\0"
"0" "[!]a-ceg-ik[]\0" "h\0"
"0" "[!]a-ceg-ik[]\0" "]\0"
"1" "[!]a-ceg-ik[]\0" "f\0"
/* Simple wild cards */
"1" "?\0" "a\0"
"0" "?\0" "aa\0"
"0" "??\0" "a\0"
"1" "?x?\0" "axb\0"
"0" "?x?\0" "abx\0"
"0" "?x?\0" "xab\0"
/* Asterisk wild cards (backtracking) */
"0" "*??\0" "a\0"
"1" "*??\0" "ab\0"
"1" "*??\0" "abc\0"
"1" "*??\0" "abcd\0"
"0" "??*\0" "a\0"
"1" "??*\0" "ab\0"
"1" "??*\0" "abc\0"
"1" "??*\0" "abcd\0"
"0" "?*?\0" "a\0"
"1" "?*?\0" "ab\0"
"1" "?*?\0" "abc\0"
"1" "?*?\0" "abcd\0"
"1" "*b\0" "b\0"
"1" "*b\0" "ab\0"
"0" "*b\0" "ba\0"
"1" "*b\0" "bb\0"
"1" "*b\0" "abb\0"
"1" "*b\0" "bab\0"
"1" "*bc\0" "abbc\0"
"1" "*bc\0" "bc\0"
"1" "*bc\0" "bbc\0"
"1" "*bc\0" "bcbc\0"
/* Multiple asterisks (complex backtracking) */
"1" "*ac*\0" "abacadaeafag\0"
"1" "*ac*ae*ag*\0" "abacadaeafag\0"
"1" "*a*b*[bc]*[ef]*g*\0" "abacadaeafag\0"
"0" "*a*b*[ef]*[cd]*g*\0" "abacadaeafag\0"
"1" "*abcd*\0" "abcabcabcabcdefg\0"
"1" "*ab*cd*\0" "abcabcabcabcdefg\0"
"1" "*abcd*abcdef*\0" "abcabcdabcdeabcdefg\0"
"0" "*abcd*\0" "abcabcabcabcefg\0"
"0" "*ab*cd*\0" "abcabcabcabcefg\0";
static int __init glob_init(void)
{
unsigned successes = 0;
unsigned n = 0;
char const *p = glob_tests;
static char const message[] __initconst =
KERN_INFO "glob: %u self-tests passed, %u failed\n";
/*
* Tests are jammed together in a string. The first byte is '1'
* or '0' to indicate the expected outcome, or '\0' to indicate the
* end of the tests. Then come two null-terminated strings: the
* pattern and the string to match it against.
*/
while (*p) {
bool expected = *p++ & 1;
char const *pat = p;
p += strlen(p) + 1;
successes += test(pat, p, expected);
p += strlen(p) + 1;
n++;
}
n -= successes;
printk(message, successes, n);
/* What's the errno for "kernel bug detected"? Guess... */
return n ? -ECANCELED : 0;
}
/* We need a dummy exit function to allow unload */
static void __exit glob_fini(void) { }
module_init(glob_init);
module_exit(glob_fini);
MODULE_DESCRIPTION("glob(7) matching tests");
MODULE_LICENSE("Dual MIT/GPL");
Computing file changes ...
