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
asn1_encoder.c
// SPDX-License-Identifier: GPL-2.0-only
/*
* Simple encoder primitives for ASN.1 BER/DER/CER
*
* Copyright (C) 2019 James.Bottomley@HansenPartnership.com
*/
#include <linux/asn1_encoder.h>
#include <linux/bug.h>
#include <linux/string.h>
#include <linux/module.h>
/**
* asn1_encode_integer() - encode positive integer to ASN.1
* @data: pointer to the pointer to the data
* @end_data: end of data pointer, points one beyond last usable byte in @data
* @integer: integer to be encoded
*
* This is a simplified encoder: it only currently does
* positive integers, but it should be simple enough to add the
* negative case if a use comes along.
*/
unsigned char *
asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
s64 integer)
{
int data_len = end_data - data;
unsigned char *d = &data[2];
bool found = false;
int i;
if (WARN(integer < 0,
"BUG: integer encode only supports positive integers"))
return ERR_PTR(-EINVAL);
if (IS_ERR(data))
return data;
/* need at least 3 bytes for tag, length and integer encoding */
if (data_len < 3)
return ERR_PTR(-EINVAL);
/* remaining length where at d (the start of the integer encoding) */
data_len -= 2;
data[0] = _tag(UNIV, PRIM, INT);
if (integer == 0) {
*d++ = 0;
goto out;
}
for (i = sizeof(integer); i > 0 ; i--) {
int byte = integer >> (8 * (i - 1));
if (!found && byte == 0)
continue;
/*
* for a positive number the first byte must have bit
* 7 clear in two's complement (otherwise it's a
* negative number) so prepend a leading zero if
* that's not the case
*/
if (!found && (byte & 0x80)) {
/*
* no check needed here, we already know we
* have len >= 1
*/
*d++ = 0;
data_len--;
}
found = true;
if (data_len == 0)
return ERR_PTR(-EINVAL);
*d++ = byte;
data_len--;
}
out:
data[1] = d - data - 2;
return d;
}
EXPORT_SYMBOL_GPL(asn1_encode_integer);
/* calculate the base 128 digit values setting the top bit of the first octet */
static int asn1_encode_oid_digit(unsigned char **_data, int *data_len, u32 oid)
{
unsigned char *data = *_data;
int start = 7 + 7 + 7 + 7;
int ret = 0;
if (*data_len < 1)
return -EINVAL;
/* quick case */
if (oid == 0) {
*data++ = 0x80;
(*data_len)--;
goto out;
}
while (oid >> start == 0)
start -= 7;
while (start > 0 && *data_len > 0) {
u8 byte;
byte = oid >> start;
oid = oid - (byte << start);
start -= 7;
byte |= 0x80;
*data++ = byte;
(*data_len)--;
}
if (*data_len > 0) {
*data++ = oid;
(*data_len)--;
} else {
ret = -EINVAL;
}
out:
*_data = data;
return ret;
}
/**
* asn1_encode_oid() - encode an oid to ASN.1
* @data: position to begin encoding at
* @end_data: end of data pointer, points one beyond last usable byte in @data
* @oid: array of oids
* @oid_len: length of oid array
*
* this encodes an OID up to ASN.1 when presented as an array of OID values
*/
unsigned char *
asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
u32 oid[], int oid_len)
{
int data_len = end_data - data;
unsigned char *d = data + 2;
int i, ret;
if (WARN(oid_len < 2, "OID must have at least two elements"))
return ERR_PTR(-EINVAL);
if (WARN(oid_len > 32, "OID is too large"))
return ERR_PTR(-EINVAL);
if (IS_ERR(data))
return data;
/* need at least 3 bytes for tag, length and OID encoding */
if (data_len < 3)
return ERR_PTR(-EINVAL);
data[0] = _tag(UNIV, PRIM, OID);
*d++ = oid[0] * 40 + oid[1];
data_len -= 3;
ret = 0;
for (i = 2; i < oid_len; i++) {
ret = asn1_encode_oid_digit(&d, &data_len, oid[i]);
if (ret < 0)
return ERR_PTR(ret);
}
data[1] = d - data - 2;
return d;
}
EXPORT_SYMBOL_GPL(asn1_encode_oid);
/**
* asn1_encode_length() - encode a length to follow an ASN.1 tag
* @data: pointer to encode at
* @data_len: pointer to remaining length (adjusted by routine)
* @len: length to encode
*
* This routine can encode lengths up to 65535 using the ASN.1 rules.
* It will accept a negative length and place a zero length tag
* instead (to keep the ASN.1 valid). This convention allows other
* encoder primitives to accept negative lengths as singalling the
* sequence will be re-encoded when the length is known.
*/
static int asn1_encode_length(unsigned char **data, int *data_len, int len)
{
if (*data_len < 1)
return -EINVAL;
if (len < 0) {
*((*data)++) = 0;
(*data_len)--;
return 0;
}
if (len <= 0x7f) {
*((*data)++) = len;
(*data_len)--;
return 0;
}
if (*data_len < 2)
return -EINVAL;
if (len <= 0xff) {
*((*data)++) = 0x81;
*((*data)++) = len & 0xff;
*data_len -= 2;
return 0;
}
if (*data_len < 3)
return -EINVAL;
if (len <= 0xffff) {
*((*data)++) = 0x82;
*((*data)++) = (len >> 8) & 0xff;
*((*data)++) = len & 0xff;
*data_len -= 3;
return 0;
}
if (WARN(len > 0xffffff, "ASN.1 length can't be > 0xffffff"))
return -EINVAL;
if (*data_len < 4)
return -EINVAL;
*((*data)++) = 0x83;
*((*data)++) = (len >> 16) & 0xff;
*((*data)++) = (len >> 8) & 0xff;
*((*data)++) = len & 0xff;
*data_len -= 4;
return 0;
}
/**
* asn1_encode_tag() - add a tag for optional or explicit value
* @data: pointer to place tag at
* @end_data: end of data pointer, points one beyond last usable byte in @data
* @tag: tag to be placed
* @string: the data to be tagged
* @len: the length of the data to be tagged
*
* Note this currently only handles short form tags < 31.
*
* Standard usage is to pass in a @tag, @string and @length and the
* @string will be ASN.1 encoded with @tag and placed into @data. If
* the encoding would put data past @end_data then an error is
* returned, otherwise a pointer to a position one beyond the encoding
* is returned.
*
* To encode in place pass a NULL @string and -1 for @len and the
* maximum allowable beginning and end of the data; all this will do
* is add the current maximum length and update the data pointer to
* the place where the tag contents should be placed is returned. The
* data should be copied in by the calling routine which should then
* repeat the prior statement but now with the known length. In order
* to avoid having to keep both before and after pointers, the repeat
* expects to be called with @data pointing to where the first encode
* returned it and still NULL for @string but the real length in @len.
*/
unsigned char *
asn1_encode_tag(unsigned char *data, const unsigned char *end_data,
u32 tag, const unsigned char *string, int len)
{
int data_len = end_data - data;
int ret;
if (WARN(tag > 30, "ASN.1 tag can't be > 30"))
return ERR_PTR(-EINVAL);
if (!string && WARN(len > 127,
"BUG: recode tag is too big (>127)"))
return ERR_PTR(-EINVAL);
if (IS_ERR(data))
return data;
if (!string && len > 0) {
/*
* we're recoding, so move back to the start of the
* tag and install a dummy length because the real
* data_len should be NULL
*/
data -= 2;
data_len = 2;
}
if (data_len < 2)
return ERR_PTR(-EINVAL);
*(data++) = _tagn(CONT, CONS, tag);
data_len--;
ret = asn1_encode_length(&data, &data_len, len);
if (ret < 0)
return ERR_PTR(ret);
if (!string)
return data;
if (data_len < len)
return ERR_PTR(-EINVAL);
memcpy(data, string, len);
data += len;
return data;
}
EXPORT_SYMBOL_GPL(asn1_encode_tag);
/**
* asn1_encode_octet_string() - encode an ASN.1 OCTET STRING
* @data: pointer to encode at
* @end_data: end of data pointer, points one beyond last usable byte in @data
* @string: string to be encoded
* @len: length of string
*
* Note ASN.1 octet strings may contain zeros, so the length is obligatory.
*/
unsigned char *
asn1_encode_octet_string(unsigned char *data,
const unsigned char *end_data,
const unsigned char *string, u32 len)
{
int data_len = end_data - data;
int ret;
if (IS_ERR(data))
return data;
/* need minimum of 2 bytes for tag and length of zero length string */
if (data_len < 2)
return ERR_PTR(-EINVAL);
*(data++) = _tag(UNIV, PRIM, OTS);
data_len--;
ret = asn1_encode_length(&data, &data_len, len);
if (ret)
return ERR_PTR(ret);
if (data_len < len)
return ERR_PTR(-EINVAL);
memcpy(data, string, len);
data += len;
return data;
}
EXPORT_SYMBOL_GPL(asn1_encode_octet_string);
/**
* asn1_encode_sequence() - wrap a byte stream in an ASN.1 SEQUENCE
* @data: pointer to encode at
* @end_data: end of data pointer, points one beyond last usable byte in @data
* @seq: data to be encoded as a sequence
* @len: length of the data to be encoded as a sequence
*
* Fill in a sequence. To encode in place, pass NULL for @seq and -1
* for @len; then call again once the length is known (still with NULL
* for @seq). In order to avoid having to keep both before and after
* pointers, the repeat expects to be called with @data pointing to
* where the first encode placed it.
*/
unsigned char *
asn1_encode_sequence(unsigned char *data, const unsigned char *end_data,
const unsigned char *seq, int len)
{
int data_len = end_data - data;
int ret;
if (!seq && WARN(len > 127,
"BUG: recode sequence is too big (>127)"))
return ERR_PTR(-EINVAL);
if (IS_ERR(data))
return data;
if (!seq && len >= 0) {
/*
* we're recoding, so move back to the start of the
* sequence and install a dummy length because the
* real length should be NULL
*/
data -= 2;
data_len = 2;
}
if (data_len < 2)
return ERR_PTR(-EINVAL);
*(data++) = _tag(UNIV, CONS, SEQ);
data_len--;
ret = asn1_encode_length(&data, &data_len, len);
if (ret)
return ERR_PTR(ret);
if (!seq)
return data;
if (data_len < len)
return ERR_PTR(-EINVAL);
memcpy(data, seq, len);
data += len;
return data;
}
EXPORT_SYMBOL_GPL(asn1_encode_sequence);
/**
* asn1_encode_boolean() - encode a boolean value to ASN.1
* @data: pointer to encode at
* @end_data: end of data pointer, points one beyond last usable byte in @data
* @val: the boolean true/false value
*/
unsigned char *
asn1_encode_boolean(unsigned char *data, const unsigned char *end_data,
bool val)
{
int data_len = end_data - data;
if (IS_ERR(data))
return data;
/* booleans are 3 bytes: tag, length == 1 and value == 0 or 1 */
if (data_len < 3)
return ERR_PTR(-EINVAL);
*(data++) = _tag(UNIV, PRIM, BOOL);
data_len--;
asn1_encode_length(&data, &data_len, 1);
if (val)
*(data++) = 1;
else
*(data++) = 0;
return data;
}
EXPORT_SYMBOL_GPL(asn1_encode_boolean);
MODULE_LICENSE("GPL");
Computing file changes ...