From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
patches@lists.linux.dev, Christoph Paasch <cpaasch@apple.com>,
Paolo Abeni <pabeni@redhat.com>,
Matthieu Baerts <matthieu.baerts@tessares.net>,
"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 6.1 05/16] mptcp: stops worker on unaccepted sockets at listener close
Date: Fri, 28 Apr 2023 13:27:57 +0200 [thread overview]
Message-ID: <20230428112040.251384476@linuxfoundation.org> (raw)
In-Reply-To: <20230428112040.063291126@linuxfoundation.org>
From: Paolo Abeni <pabeni@redhat.com>
commit 2a6a870e44dd88f1a6a2893c65ef756a9edfb4c7 upstream.
This is a partial revert of the blamed commit, with a relevant
change: mptcp_subflow_queue_clean() now just change the msk
socket status and stop the worker, so that the UaF issue addressed
by the blamed commit is not re-introduced.
The above prevents the mptcp worker from running concurrently with
inet_csk_listen_stop(), as such race would trigger a warning, as
reported by Christoph:
RSP: 002b:00007f784fe09cd8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
WARNING: CPU: 0 PID: 25807 at net/ipv4/inet_connection_sock.c:1387 inet_csk_listen_stop+0x664/0x870 net/ipv4/inet_connection_sock.c:1387
RAX: ffffffffffffffda RBX: 00000000006bc050 RCX: 00007f7850afd6a9
RDX: 0000000000000000 RSI: 0000000020000340 RDI: 0000000000000004
Modules linked in:
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006bc05c
R13: fffffffffffffea8 R14: 00000000006bc050 R15: 000000000001fe40
</TASK>
CPU: 0 PID: 25807 Comm: syz-executor.7 Not tainted 6.2.0-g778e54711659 #7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014
RIP: 0010:inet_csk_listen_stop+0x664/0x870 net/ipv4/inet_connection_sock.c:1387
RAX: 0000000000000000 RBX: ffff888100dfbd40 RCX: 0000000000000000
RDX: ffff8881363aab80 RSI: ffffffff81c494f4 RDI: 0000000000000005
RBP: ffff888126dad080 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffff888100dfe040
R13: 0000000000000001 R14: 0000000000000000 R15: ffff888100dfbdd8
FS: 00007f7850a2c800(0000) GS:ffff88813bc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b32d26000 CR3: 000000012fdd8006 CR4: 0000000000770ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
__tcp_close+0x5b2/0x620 net/ipv4/tcp.c:2875
__mptcp_close_ssk+0x145/0x3d0 net/mptcp/protocol.c:2427
mptcp_destroy_common+0x8a/0x1c0 net/mptcp/protocol.c:3277
mptcp_destroy+0x41/0x60 net/mptcp/protocol.c:3304
__mptcp_destroy_sock+0x56/0x140 net/mptcp/protocol.c:2965
__mptcp_close+0x38f/0x4a0 net/mptcp/protocol.c:3057
mptcp_close+0x24/0xe0 net/mptcp/protocol.c:3072
inet_release+0x53/0xa0 net/ipv4/af_inet.c:429
__sock_release+0x4e/0xf0 net/socket.c:651
sock_close+0x15/0x20 net/socket.c:1393
__fput+0xff/0x420 fs/file_table.c:321
task_work_run+0x8b/0xe0 kernel/task_work.c:179
resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
exit_to_user_mode_loop kernel/entry/common.c:171 [inline]
exit_to_user_mode_prepare+0x113/0x120 kernel/entry/common.c:203
__syscall_exit_to_user_mode_work kernel/entry/common.c:285 [inline]
syscall_exit_to_user_mode+0x1d/0x40 kernel/entry/common.c:296
do_syscall_64+0x46/0x90 arch/x86/entry/common.c:86
entry_SYSCALL_64_after_hwframe+0x72/0xdc
RIP: 0033:0x7f7850af70dc
RAX: 0000000000000000 RBX: 0000000000000004 RCX: 00007f7850af70dc
RDX: 00007f7850a2c800 RSI: 0000000000000002 RDI: 0000000000000003
RBP: 00000000006bd980 R08: 0000000000000000 R09: 00000000000018a0
R10: 00000000316338a4 R11: 0000000000000293 R12: 0000000000211e31
R13: 00000000006bc05c R14: 00007f785062c000 R15: 0000000000211af0
Fixes: 0a3f4f1f9c27 ("mptcp: fix UaF in listener shutdown")
Cc: stable@vger.kernel.org
Reported-by: Christoph Paasch <cpaasch@apple.com>
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/371
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/mptcp/protocol.c | 6 ++++
net/mptcp/protocol.h | 1
net/mptcp/subflow.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 79 insertions(+)
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2380,6 +2380,12 @@ static void __mptcp_close_ssk(struct soc
mptcp_subflow_drop_ctx(ssk);
} else {
/* otherwise tcp will dispose of the ssk and subflow ctx */
+ if (ssk->sk_state == TCP_LISTEN) {
+ tcp_set_state(ssk, TCP_CLOSE);
+ mptcp_subflow_queue_clean(sk, ssk);
+ inet_csk_listen_stop(ssk);
+ }
+
__tcp_close(ssk, 0);
/* close acquired an extra ref */
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -615,6 +615,7 @@ void mptcp_close_ssk(struct sock *sk, st
struct mptcp_subflow_context *subflow);
void __mptcp_subflow_send_ack(struct sock *ssk);
void mptcp_subflow_reset(struct sock *ssk);
+void mptcp_subflow_queue_clean(struct sock *sk, struct sock *ssk);
void mptcp_sock_graft(struct sock *sk, struct socket *parent);
struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk);
bool __mptcp_close(struct sock *sk, long timeout);
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1758,6 +1758,78 @@ static void subflow_state_change(struct
}
}
+void mptcp_subflow_queue_clean(struct sock *listener_sk, struct sock *listener_ssk)
+{
+ struct request_sock_queue *queue = &inet_csk(listener_ssk)->icsk_accept_queue;
+ struct mptcp_sock *msk, *next, *head = NULL;
+ struct request_sock *req;
+
+ /* build a list of all unaccepted mptcp sockets */
+ spin_lock_bh(&queue->rskq_lock);
+ for (req = queue->rskq_accept_head; req; req = req->dl_next) {
+ struct mptcp_subflow_context *subflow;
+ struct sock *ssk = req->sk;
+
+ if (!sk_is_mptcp(ssk))
+ continue;
+
+ subflow = mptcp_subflow_ctx(ssk);
+ if (!subflow || !subflow->conn)
+ continue;
+
+ /* skip if already in list */
+ msk = mptcp_sk(subflow->conn);
+ if (msk->dl_next || msk == head)
+ continue;
+
+ sock_hold(subflow->conn);
+ msk->dl_next = head;
+ head = msk;
+ }
+ spin_unlock_bh(&queue->rskq_lock);
+ if (!head)
+ return;
+
+ /* can't acquire the msk socket lock under the subflow one,
+ * or will cause ABBA deadlock
+ */
+ release_sock(listener_ssk);
+
+ for (msk = head; msk; msk = next) {
+ struct sock *sk = (struct sock *)msk;
+
+ lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
+ next = msk->dl_next;
+ msk->dl_next = NULL;
+
+ /* prevent the stack from later re-schedule the worker for
+ * this socket
+ */
+ inet_sk_state_store(sk, TCP_CLOSE);
+ release_sock(sk);
+
+ /* lockdep will report a false positive ABBA deadlock
+ * between cancel_work_sync and the listener socket.
+ * The involved locks belong to different sockets WRT
+ * the existing AB chain.
+ * Using a per socket key is problematic as key
+ * deregistration requires process context and must be
+ * performed at socket disposal time, in atomic
+ * context.
+ * Just tell lockdep to consider the listener socket
+ * released here.
+ */
+ mutex_release(&listener_sk->sk_lock.dep_map, _RET_IP_);
+ mptcp_cancel_work(sk);
+ mutex_acquire(&listener_sk->sk_lock.dep_map, 0, 0, _RET_IP_);
+
+ sock_put(sk);
+ }
+
+ /* we are still under the listener msk socket lock */
+ lock_sock_nested(listener_ssk, SINGLE_DEPTH_NESTING);
+}
+
static int subflow_ulp_init(struct sock *sk)
{
struct inet_connection_sock *icsk = inet_csk(sk);
next prev parent reply other threads:[~2023-04-28 11:29 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-28 11:27 [PATCH 6.1 00/16] 6.1.27-rc1 review Greg Kroah-Hartman
2023-04-28 11:27 ` [PATCH 6.1 01/16] um: Only disable SSE on clang to work around old GCC bugs Greg Kroah-Hartman
2023-04-28 11:27 ` [PATCH 6.1 02/16] phy: phy-brcm-usb: Utilize platform_get_irq_byname_optional() Greg Kroah-Hartman
2023-04-28 11:27 ` [PATCH 6.1 03/16] KVM: arm64: Retry fault if vma_lookup() results become invalid Greg Kroah-Hartman
2023-04-28 11:27 ` [PATCH 6.1 04/16] mm/mempolicy: fix use-after-free of VMA iterator Greg Kroah-Hartman
2023-04-28 11:27 ` Greg Kroah-Hartman [this message]
2023-04-28 11:27 ` [PATCH 6.1 06/16] mptcp: fix accept vs worker race Greg Kroah-Hartman
2023-04-28 11:27 ` [PATCH 6.1 07/16] wifi: brcmfmac: slab-out-of-bounds read in brcmf_get_assoc_ies() Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 08/16] drm/fb-helper: set x/yres_virtual in drm_fb_helper_check_var Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 09/16] gpiolib: acpi: Add a ignore wakeup quirk for Clevo NL5xNU Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 10/16] bluetooth: Perform careful capability checks in hci_sock_ioctl() Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 11/16] btrfs: fix uninitialized variable warnings Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 12/16] USB: serial: option: add UNISOC vendor and TOZED LT70C product Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 13/16] driver core: Dont require dynamic_debug for initcall_debug probe timing Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 14/16] riscv: Move early dtb mapping into the fixmap region Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 15/16] riscv: Do not set initial_boot_params to the linear address of the dtb Greg Kroah-Hartman
2023-04-28 11:28 ` [PATCH 6.1 16/16] riscv: No need to relocate the dtb as it lies in the fixmap region Greg Kroah-Hartman
2023-04-28 14:35 ` [PATCH 6.1 00/16] 6.1.27-rc1 review Markus Reichelt
2023-04-28 22:06 ` Naresh Kamboju
2023-04-28 22:29 ` Shuah Khan
2023-04-29 4:11 ` Guenter Roeck
2023-04-29 6:08 ` Ron Economos
2023-04-29 7:43 ` Bagas Sanjaya
2023-04-29 9:54 ` Conor Dooley
2023-04-29 9:56 ` ogasawara takeshi
2023-04-29 17:14 ` Florian Fainelli
2023-05-02 5:39 ` Chris Paterson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230428112040.251384476@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=cpaasch@apple.com \
--cc=davem@davemloft.net \
--cc=matthieu.baerts@tessares.net \
--cc=pabeni@redhat.com \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).