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: 28+ 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
2023-05-02 16:17 ` Jon Hunter
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.