From nobody Thu Apr 25 22:39:11 2024 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C07C5259A for ; Mon, 24 Apr 2023 14:09:21 +0000 (UTC) Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-3f1957e80a2so79730005e9.1 for ; Mon, 24 Apr 2023 07:09:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tessares.net; s=google; t=1682345360; x=1684937360; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=Cn96K4ewEfXzrj1A/qyaR4zIjE/JZ3DKSqclOjk5uAE=; b=VtQgHiN7zilA5np7cKWfqtMpar5F9pvgfc3JUq8HtYsfGvBkRF7BhMGBovzuFxdGX6 WrsSH8Wsi95jv8Z5/o+CCfwImjQOdfgkMXwyE/S33GFNYdWNJLRT36wXLJ2Lhl5ZVV1S eSya6ZVhPp7MrlePG5L6X9rCJb6TyY6wyVVapCAgBiDKu8B9m2qmmIi9Ey1IWoH/AzfD AXwlFHHLOcG3P/eFx6V+WOOKm0SJn0Qd5fiHmWLgl8eSamjBIOAn/8pVurXnF9hhKwOt Zrt2fs5hjlT9y2dnKPcP3bu0iezf1O1GfRNjLtwa3Y6WdhH0anlcfjxtpynVgMfJalFM VmSw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682345360; x=1684937360; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Cn96K4ewEfXzrj1A/qyaR4zIjE/JZ3DKSqclOjk5uAE=; b=eSayzBsaU5ZwnVTqzIEBlliBWUAaev0whjJvp9GCRBZye4a+/PbQ+Hxni7i1o/bm7w kzY1fCb4ueJSqQJFE9lRLXIAQ2JeWk9mVh7YkTV7cWlRTKeD+3+c5ZIl9oJHfOMyR2sm Al3yOzvrdOybv7BDWwJsdYKkrZlW0j1CPJSJvfWH6dVqxqr222bRjE9PieuGWVtkBdtj 0RL4j+//u5PpOXCz7cyowx+cH0FNIv8qdunRZ4BF22LAdwEM5YGzFn16LRCKRB6tWrOp XOgvshqzpIPm4yV9/pKSZZB5ViUhubz8Rc5KHMYNcXcQPN27HHO4cTpJbKUFKtvERpZR q2Ng== X-Gm-Message-State: AAQBX9f7s8dre7RLROwkdrVT/KF5bCuZRV+/9GcOmJBJ0Fsn7/CWuZp2 6vevEyUZDICQz9L6tyoQxYsmog== X-Google-Smtp-Source: AKy350bo3gCSg2hnIiZ47Dx59Pn8o8QojjeXtWAPWVtcay8Lvny2+jNpNx0ae6Hpy3Es1KqlLCufUA== X-Received: by 2002:a05:600c:d9:b0:3f1:6ead:e396 with SMTP id u25-20020a05600c00d900b003f16eade396mr7476580wmm.17.1682345359750; Mon, 24 Apr 2023 07:09:19 -0700 (PDT) Received: from vdi08.nix.tessares.net (static.219.156.76.144.clients.your-server.de. [144.76.156.219]) by smtp.gmail.com with ESMTPSA id m6-20020a05600c4f4600b003ee5fa61f45sm15729235wmq.3.2023.04.24.07.09.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Apr 2023 07:09:19 -0700 (PDT) From: Matthieu Baerts Date: Mon, 24 Apr 2023 16:09:08 +0200 Subject: [PATCH 6.1 1/2] mptcp: stops worker on unaccepted sockets at listener close Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20230424-upstream-stable-20230424-conflicts-6-1-v1-1-b054457d3646@tessares.net> References: <20230424-upstream-stable-20230424-conflicts-6-1-v1-0-b054457d3646@tessares.net> In-Reply-To: <20230424-upstream-stable-20230424-conflicts-6-1-v1-0-b054457d3646@tessares.net> To: Greg Kroah-Hartman , Sasha Levin Cc: Paolo Abeni , stable@vger.kernel.org, mptcp@lists.linux.dev, Matthieu Baerts , Christoph Paasch , "David S. Miller" X-Mailer: b4 0.12.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=8093; i=matthieu.baerts@tessares.net; h=from:subject:message-id; bh=DtyF8ql1cbSXnRlBL89NKeeIZwkoYE+ZEI6rgiyFfx0=; b=owEBbQKS/ZANAwAIAfa3gk9CaaBzAcsmYgBkRo2NyWY3rpwrSMxd1R8bwW0iYQW433TR53622 GWlVCQMpRuJAjMEAAEIAB0WIQToy4X3aHcFem4n93r2t4JPQmmgcwUCZEaNjQAKCRD2t4JPQmmg c1UeEACpitOMY2lbaf+AyMQtuiNb7bAjI9EawrWhBexluRCQcaMZIyLNSXyNObp0gQcZbv3aPp4 NaaPFwNuyPUgGEqZSG2B1fD/AOT+9kvvNrtKytR4GP2qr9b4tLoJ98Pl7F2HVLKBK1NWSrAK/7k ohtoD6bChTW3eLEAi0kZ1T1TtvDN6bJDyG1v/Bpd/DjO/iv2GJwSV3Z+Q09OswR4wFMJkhmc3lF Ns406OQ+UoDd3lC7Jcx+4ua/83UfvZyhZo49vbLpfr90pJ5vJMIyYgRaucsW4yc/yGSup/ke1/1 4G1pazuOVEiUI+1NKz3hK/X9jywAZEQUQhM8azU883NuHapdt2vVHdrQihjgg6OgcQ7btltYDDM issG1epsKemBHtqJyrxCifaak28meItA0h25O2f3TMn+nQDQxSzTaY0dYrVeuK7+Pye2l3mlCzN DOcdeRSHnHVZEXgqMKG5GJEBoCgrB2yEBE4zZnwG3BXEuc7Kf7oeJTGw1DSQQlc27JPdFoAqDXx SKzZdBofzjsSKaNgRT4qcWyMAyIw4YeXGGZvsRtdjsveJLCyoMnlE/a0h31IQwYLxD1K6SbjueM 7rf5xdYqu3j4QOYKSflfBDLVflI4t/7Auk56Ik+bQGeFHznBtJCcfYKIjG84Kunw9YSudAwotQK b9m1CLh2gIfbW7Q== X-Developer-Key: i=matthieu.baerts@tessares.net; a=openpgp; fpr=E8CB85F76877057A6E27F77AF6B7824F4269A073 From: Paolo Abeni [ Upstream commit 2a6a870e44dd88f1a6a2893c65ef756a9edfb4c7 ] Backports notes: similar to the conflicts with the partially reverted commit 5564be74a22a ("mptcp: fix UaF in listener shutdown"), there was one simple conflict in net/mptcp/protocol.c with: commit f8c9dfbd875b ("mptcp: add pm listener events") We just need to omit one line linked to the PM listener event. 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 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: __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 Link: https://github.com/multipath-tcp/mptcp_net-next/issues/371 Signed-off-by: Paolo Abeni Reviewed-by: Matthieu Baerts Signed-off-by: Matthieu Baerts Signed-off-by: David S. Miller Signed-off-by: Matthieu Baerts --- net/mptcp/protocol.c | 6 +++++ net/mptcp/protocol.h | 1 + net/mptcp/subflow.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++= ++++ 3 files changed, 79 insertions(+) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index b1bbb0b75a13..a4d5072c6e98 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2380,6 +2380,12 @@ static void __mptcp_close_ssk(struct sock *sk, struc= t sock *ssk, mptcp_subflow_drop_ctx(ssk); } else { /* otherwise tcp will dispose of the ssk and subflow ctx */ + if (ssk->sk_state =3D=3D TCP_LISTEN) { + tcp_set_state(ssk, TCP_CLOSE); + mptcp_subflow_queue_clean(sk, ssk); + inet_csk_listen_stop(ssk); + } + __tcp_close(ssk, 0); =20 /* close acquired an extra ref */ diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 051e8022d661..2cddd5b52e8f 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -615,6 +615,7 @@ void mptcp_close_ssk(struct sock *sk, struct sock *ssk, 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); diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 125d1f58d6a4..d9bc4bdb94b7 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1758,6 +1758,78 @@ static void subflow_state_change(struct sock *sk) } } =20 +void mptcp_subflow_queue_clean(struct sock *listener_sk, struct sock *list= ener_ssk) +{ + struct request_sock_queue *queue =3D &inet_csk(listener_ssk)->icsk_accept= _queue; + struct mptcp_sock *msk, *next, *head =3D NULL; + struct request_sock *req; + + /* build a list of all unaccepted mptcp sockets */ + spin_lock_bh(&queue->rskq_lock); + for (req =3D queue->rskq_accept_head; req; req =3D req->dl_next) { + struct mptcp_subflow_context *subflow; + struct sock *ssk =3D req->sk; + + if (!sk_is_mptcp(ssk)) + continue; + + subflow =3D mptcp_subflow_ctx(ssk); + if (!subflow || !subflow->conn) + continue; + + /* skip if already in list */ + msk =3D mptcp_sk(subflow->conn); + if (msk->dl_next || msk =3D=3D head) + continue; + + sock_hold(subflow->conn); + msk->dl_next =3D head; + head =3D 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 =3D head; msk; msk =3D next) { + struct sock *sk =3D (struct sock *)msk; + + lock_sock_nested(sk, SINGLE_DEPTH_NESTING); + next =3D msk->dl_next; + msk->dl_next =3D 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 =3D inet_csk(sk); --=20 2.39.2 From nobody Thu Apr 25 22:39:11 2024 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 971DB28FA for ; Mon, 24 Apr 2023 14:09:22 +0000 (UTC) Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-3f19ab99540so21647145e9.2 for ; Mon, 24 Apr 2023 07:09:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tessares.net; s=google; t=1682345361; x=1684937361; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=Hc99Xfr6pVuENOSF5bpLIfHEqcxQtxjr9hf/j9uZ9wY=; b=wC3fsTnBr2FXauBCpd60riPKBLYha0ovm217SPrieSpLOZN+zfHM70MXosVSTmamC2 e7sakwHxwqyi04qasb6ryKXM9C51vR0VdPnfIEmdj60x6EbWyyDtWm7yzatJ1g+H5qvt Dwp4Ho7WH+TRYXgXkDMfwky7zgHhUPwT9EuZszzhKbx/NEIP0fDBsHPDQWvZfejx6tsb +kDgiBZOl7cLTYXZzpq8mSIRGhuWgvGRwZUHKYIMTdgrfFxtIWyUId5rIFm60nAnuK/Y nPYTpcdFEpcPOEHJxW1NQKMGTp64AuIVs5b9jLclQu34vNUu1ffZrVmjPyB+ABdH9+VT Ze6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682345361; x=1684937361; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Hc99Xfr6pVuENOSF5bpLIfHEqcxQtxjr9hf/j9uZ9wY=; b=IizLPcdJ357cwflXyRhv08TVXPpQfO2RUyGd9S6sNQwq19qExEsiS0xj287VdrjPcu XW92ItN7s2svoXcCraaMn9NBlkYoc+Wl7vyaZIlxDnckgJZjr+XZN4kiVdfT/7EVB7Od St/ECi0+DI0fntIsHgAhyMFL8/6eKp+jwx+bTClDOZIiUif7k8nIRm5t/iAPgaL+P9OW yR23f6Qx9PEkdOuR5d1WlGPa779f4sIdbwWkIc/rbmhtDldmsCNML1XH0VCYI/d3NUsw IU/6ccbniiWBt3oUgKBT1zdNukRO3DS1yLT31l2dJ5YhNyThiaKNf2Rdgn3PNybGvXaQ BfUA== X-Gm-Message-State: AAQBX9et8ZbYUloTAHxfQKql2xHvAg/wi119QAp+SMDZRtvk6Hm6jkZN 54lhe7XlktlUG9BWKxeOzw+1uA== X-Google-Smtp-Source: AKy350Yp6if0s710jV7YAiDxBgBmyafL/VgUAfLu4ekSbfbRHDF7CwwsRXQ/VWhLWQUoJmKF9Lhulw== X-Received: by 2002:a7b:c045:0:b0:3f1:662a:93c4 with SMTP id u5-20020a7bc045000000b003f1662a93c4mr8287706wmc.36.1682345360735; Mon, 24 Apr 2023 07:09:20 -0700 (PDT) Received: from vdi08.nix.tessares.net (static.219.156.76.144.clients.your-server.de. [144.76.156.219]) by smtp.gmail.com with ESMTPSA id m6-20020a05600c4f4600b003ee5fa61f45sm15729235wmq.3.2023.04.24.07.09.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Apr 2023 07:09:20 -0700 (PDT) From: Matthieu Baerts Date: Mon, 24 Apr 2023 16:09:09 +0200 Subject: [PATCH 6.1 2/2] mptcp: fix accept vs worker race Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20230424-upstream-stable-20230424-conflicts-6-1-v1-2-b054457d3646@tessares.net> References: <20230424-upstream-stable-20230424-conflicts-6-1-v1-0-b054457d3646@tessares.net> In-Reply-To: <20230424-upstream-stable-20230424-conflicts-6-1-v1-0-b054457d3646@tessares.net> To: Greg Kroah-Hartman , Sasha Levin Cc: Paolo Abeni , stable@vger.kernel.org, mptcp@lists.linux.dev, Matthieu Baerts , Christoph Paasch , "David S. Miller" X-Mailer: b4 0.12.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=11567; i=matthieu.baerts@tessares.net; h=from:subject:message-id; bh=ik0y/F3/ETptwHcGH47CfgtNVwu/y4XJw8FAP25SDQY=; b=owEBbQKS/ZANAwAIAfa3gk9CaaBzAcsmYgBkRo2N4REjjlJE4Cy9UaAHxqQRxonA6IBed/D4C IhrDIqDf/2JAjMEAAEIAB0WIQToy4X3aHcFem4n93r2t4JPQmmgcwUCZEaNjQAKCRD2t4JPQmmg c8YAD/4qk1xfzvuxTnqiAD+Cy/7eW7AqOjlNixc8Nxr/2pDArXbrb05tirMLJJNWVLy0r1FRZrL k07+YXecKrO/0EqOZt+MSa6STHKjRF3IWvv/NzXEveO040SuVBFj4SzUnwfkod72/UhTIzJ+NW1 lBbKpAbOiAKnusEaJNGyEd4OvpLlaEc4db+x4VUF2V3+83/qb5242CguU3SZ94Wj0ym6zEyiRnx 2hAyKFqQDYRXjpERCA57MLf3oN5hrGKXFKkPS+6rEUEX+RKBhyRi0AnJPvg9tPxuv1gle1D6epP 8NEvPkiSxiiK4AIuwjBiHaM0/Bdxp3hiGmCXLHNHSRQ/Gh/T0N7YasOTZq/Sw/S5C+Ib6FJua3o 6r2yD14lgWTIcKzxAYuOZvseL966HCfxmtoAYk3Q1HAgBdgzzafN3lwtJW3GQ//2Qk+UyjcfrTA ZoM6RDcOUBTOPBWOsj6lNVzMqwQkn4bJT201+199zKt9C2oJoq162rkWjGtT0AsjNKZOLahwETx y5t356S8b5WyLWKgB/QwY8tgNz/0GhfR/n+uGqclXCv3Bm4is9XVhv1x0RJLWH/EcpHBXDw+ib+ 5iN71YS6oSWeiaf4nY+AWfiQMIKxT3sL1097FqV+TNKgWhASSBFZ9I99r3B1yMmAkl3TnuXU91G 8/R5t9KuJxGgbOQ== X-Developer-Key: i=matthieu.baerts@tessares.net; a=openpgp; fpr=E8CB85F76877057A6E27F77AF6B7824F4269A073 From: Paolo Abeni [ Upstream commit 63740448a32eb662e05894425b47bcc5814136f4 ] Backports notes: one simple conflict in net/mptcp/protocol.h with commit 36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()") Where the two commits add the signature of one function in protocol.h at the same place. The mptcp worker and mptcp_accept() can race, as reported by Christoph: refcount_t: addition on 0; use-after-free. WARNING: CPU: 1 PID: 14351 at lib/refcount.c:25 refcount_warn_saturate+0x10= 5/0x1b0 lib/refcount.c:25 Modules linked in: CPU: 1 PID: 14351 Comm: syz-executor.2 Not tainted 6.3.0-rc1-gde5e8fd0123c = #11 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04= /01/2014 RIP: 0010:refcount_warn_saturate+0x105/0x1b0 lib/refcount.c:25 Code: 02 31 ff 89 de e8 1b f0 a7 ff 84 db 0f 85 6e ff ff ff e8 3e f5 a7 ff = 48 c7 c7 d8 c7 34 83 c6 05 6d 2d 0f 02 01 e8 cb 3d 90 ff <0f> 0b e9 4f ff f= f ff e8 1f f5 a7 ff 0f b6 1d 54 2d 0f 02 31 ff 89 RSP: 0018:ffffc90000a47bf8 EFLAGS: 00010282 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: ffff88802eae98c0 RSI: ffffffff81097d4f RDI: 0000000000000001 RBP: ffff88802e712180 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000001 R11: ffff88802eaea148 R12: ffff88802e712100 R13: ffff88802e712a88 R14: ffff888005cb93a8 R15: ffff88802e712a88 FS: 0000000000000000(0000) GS:ffff88803ed00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f277fd89120 CR3: 0000000035486002 CR4: 0000000000370ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: __refcount_add include/linux/refcount.h:199 [inline] __refcount_inc include/linux/refcount.h:250 [inline] refcount_inc include/linux/refcount.h:267 [inline] sock_hold include/net/sock.h:775 [inline] __mptcp_close+0x4c6/0x4d0 net/mptcp/protocol.c:3051 mptcp_close+0x24/0xe0 net/mptcp/protocol.c:3072 inet_release+0x56/0xa0 net/ipv4/af_inet.c:429 __sock_release+0x51/0xf0 net/socket.c:653 sock_close+0x18/0x20 net/socket.c:1395 __fput+0x113/0x430 fs/file_table.c:321 task_work_run+0x96/0x100 kernel/task_work.c:179 exit_task_work include/linux/task_work.h:38 [inline] do_exit+0x4fc/0x10c0 kernel/exit.c:869 do_group_exit+0x51/0xf0 kernel/exit.c:1019 get_signal+0x12b0/0x1390 kernel/signal.c:2859 arch_do_signal_or_restart+0x25/0x260 arch/x86/kernel/signal.c:306 exit_to_user_mode_loop kernel/entry/common.c:168 [inline] exit_to_user_mode_prepare+0x131/0x1a0 kernel/entry/common.c:203 __syscall_exit_to_user_mode_work kernel/entry/common.c:285 [inline] syscall_exit_to_user_mode+0x19/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:0x7fec4b4926a9 Code: Unable to access opcode bytes at 0x7fec4b49267f. RSP: 002b:00007fec49f9dd78 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca RAX: fffffffffffffe00 RBX: 00000000006bc058 RCX: 00007fec4b4926a9 RDX: 0000000000000000 RSI: 0000000000000080 RDI: 00000000006bc058 RBP: 00000000006bc050 R08: 00000000007df998 R09: 00000000007df998 R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006bc05c R13: fffffffffffffea8 R14: 000000000000000b R15: 000000000001fe40 The root cause is that the worker can force fallback to TCP the first mptcp subflow, actually deleting the unaccepted msk socket. We can explicitly prevent the race delaying the unaccepted msk deletion at listener shutdown time. In case the closed subflow is later accepted, just drop the mptcp context and let the user-space deal with the paired mptcp socket. Fixes: b6985b9b8295 ("mptcp: use the workqueue to destroy unaccepted socket= s") Cc: stable@vger.kernel.org Reported-by: Christoph Paasch Link: https://github.com/multipath-tcp/mptcp_net-next/issues/375 Signed-off-by: Paolo Abeni Reviewed-by: Matthieu Baerts Tested-by: Christoph Paasch Signed-off-by: Matthieu Baerts Signed-off-by: David S. Miller Signed-off-by: Matthieu Baerts --- net/mptcp/protocol.c | 68 ++++++++++++++++++++++++++++++++++--------------= ---- net/mptcp/protocol.h | 1 + net/mptcp/subflow.c | 22 +++++++++-------- 3 files changed, 58 insertions(+), 33 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index a4d5072c6e98..ea46a5cb1c30 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2330,7 +2330,26 @@ static void __mptcp_close_ssk(struct sock *sk, struc= t sock *ssk, unsigned int flags) { struct mptcp_sock *msk =3D mptcp_sk(sk); - bool need_push, dispose_it; + bool dispose_it, need_push =3D false; + + /* If the first subflow moved to a close state before accept, e.g. due + * to an incoming reset, mptcp either: + * - if either the subflow or the msk are dead, destroy the context + * (the subflow socket is deleted by inet_child_forget) and the msk + * - otherwise do nothing at the moment and take action at accept and/or + * listener shutdown - user-space must be able to accept() the closed + * socket. + */ + if (msk->in_accept_queue && msk->first =3D=3D ssk) { + if (!sock_flag(sk, SOCK_DEAD) && !sock_flag(ssk, SOCK_DEAD)) + return; + + /* ensure later check in mptcp_worker() will dispose the msk */ + sock_set_flag(sk, SOCK_DEAD); + lock_sock_nested(ssk, SINGLE_DEPTH_NESTING); + mptcp_subflow_drop_ctx(ssk); + goto out_release; + } =20 dispose_it =3D !msk->subflow || ssk !=3D msk->subflow->sk; if (dispose_it) @@ -2366,18 +2385,6 @@ static void __mptcp_close_ssk(struct sock *sk, struc= t sock *ssk, if (!inet_csk(ssk)->icsk_ulp_ops) { WARN_ON_ONCE(!sock_flag(ssk, SOCK_DEAD)); kfree_rcu(subflow, rcu); - } else if (msk->in_accept_queue && msk->first =3D=3D ssk) { - /* if the first subflow moved to a close state, e.g. due to - * incoming reset and we reach here before inet_child_forget() - * the TCP stack could later try to close it via - * inet_csk_listen_stop(), or deliver it to the user space via - * accept(). - * We can't delete the subflow - or risk a double free - nor let - * the msk survive - or will be leaked in the non accept scenario: - * fallback and let TCP cope with the subflow cleanup. - */ - WARN_ON_ONCE(sock_flag(ssk, SOCK_DEAD)); - mptcp_subflow_drop_ctx(ssk); } else { /* otherwise tcp will dispose of the ssk and subflow ctx */ if (ssk->sk_state =3D=3D TCP_LISTEN) { @@ -2391,6 +2398,8 @@ static void __mptcp_close_ssk(struct sock *sk, struct= sock *ssk, /* close acquired an extra ref */ __sock_put(ssk); } + +out_release: release_sock(ssk); =20 sock_put(ssk); @@ -2445,21 +2454,14 @@ static void __mptcp_close_subflow(struct sock *sk) mptcp_close_ssk(sk, ssk, subflow); } =20 - /* if the MPC subflow has been closed before the msk is accepted, - * msk will never be accept-ed, close it now - */ - if (!msk->first && msk->in_accept_queue) { - sock_set_flag(sk, SOCK_DEAD); - inet_sk_state_store(sk, TCP_CLOSE); - } } =20 -static bool mptcp_check_close_timeout(const struct sock *sk) +static bool mptcp_should_close(const struct sock *sk) { s32 delta =3D tcp_jiffies32 - inet_csk(sk)->icsk_mtup.probe_timestamp; struct mptcp_subflow_context *subflow; =20 - if (delta >=3D TCP_TIMEWAIT_LEN) + if (delta >=3D TCP_TIMEWAIT_LEN || mptcp_sk(sk)->in_accept_queue) return true; =20 /* if all subflows are in closed status don't bother with additional @@ -2667,7 +2669,7 @@ static void mptcp_worker(struct work_struct *work) * even if it is orphaned and in FIN_WAIT2 state */ if (sock_flag(sk, SOCK_DEAD)) { - if (mptcp_check_close_timeout(sk)) { + if (mptcp_should_close(sk)) { inet_sk_state_store(sk, TCP_CLOSE); mptcp_do_fastclose(sk); } @@ -2912,6 +2914,14 @@ static void __mptcp_destroy_sock(struct sock *sk) sock_put(sk); } =20 +void __mptcp_unaccepted_force_close(struct sock *sk) +{ + sock_set_flag(sk, SOCK_DEAD); + inet_sk_state_store(sk, TCP_CLOSE); + mptcp_do_fastclose(sk); + __mptcp_destroy_sock(sk); +} + static __poll_t mptcp_check_readable(struct mptcp_sock *msk) { /* Concurrent splices from sk_receive_queue into receive_queue will @@ -3759,6 +3769,18 @@ static int mptcp_stream_accept(struct socket *sock, = struct socket *newsock, if (!ssk->sk_socket) mptcp_sock_graft(ssk, newsock); } + + /* Do late cleanup for the first subflow as necessary. Also + * deal with bad peers not doing a complete shutdown. + */ + if (msk->first && + unlikely(inet_sk_state_load(msk->first) =3D=3D TCP_CLOSE)) { + __mptcp_close_ssk(newsk, msk->first, + mptcp_subflow_ctx(msk->first), 0); + if (unlikely(list_empty(&msk->conn_list))) + inet_sk_state_store(newsk, TCP_CLOSE); + } + release_sock(newsk); } =20 diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 2cddd5b52e8f..441feeaeb242 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -620,6 +620,7 @@ void mptcp_sock_graft(struct sock *sk, struct socket *p= arent); struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk); bool __mptcp_close(struct sock *sk, long timeout); void mptcp_cancel_work(struct sock *sk); +void __mptcp_unaccepted_force_close(struct sock *sk); =20 bool mptcp_addresses_equal(const struct mptcp_addr_info *a, const struct mptcp_addr_info *b, bool use_port); diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index d9bc4bdb94b7..67ddbf6f2e4e 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -661,9 +661,12 @@ void mptcp_subflow_drop_ctx(struct sock *ssk) if (!ctx) return; =20 - subflow_ulp_fallback(ssk, ctx); - if (ctx->conn) - sock_put(ctx->conn); + list_del(&mptcp_subflow_ctx(ssk)->node); + if (inet_csk(ssk)->icsk_ulp_ops) { + subflow_ulp_fallback(ssk, ctx); + if (ctx->conn) + sock_put(ctx->conn); + } =20 kfree_rcu(ctx, rcu); } @@ -1763,6 +1766,7 @@ void mptcp_subflow_queue_clean(struct sock *listener_= sk, struct sock *listener_s struct request_sock_queue *queue =3D &inet_csk(listener_ssk)->icsk_accept= _queue; struct mptcp_sock *msk, *next, *head =3D NULL; struct request_sock *req; + struct sock *sk; =20 /* build a list of all unaccepted mptcp sockets */ spin_lock_bh(&queue->rskq_lock); @@ -1778,11 +1782,12 @@ void mptcp_subflow_queue_clean(struct sock *listene= r_sk, struct sock *listener_s continue; =20 /* skip if already in list */ - msk =3D mptcp_sk(subflow->conn); + sk =3D subflow->conn; + msk =3D mptcp_sk(sk); if (msk->dl_next || msk =3D=3D head) continue; =20 - sock_hold(subflow->conn); + sock_hold(sk); msk->dl_next =3D head; head =3D msk; } @@ -1796,16 +1801,13 @@ void mptcp_subflow_queue_clean(struct sock *listene= r_sk, struct sock *listener_s release_sock(listener_ssk); =20 for (msk =3D head; msk; msk =3D next) { - struct sock *sk =3D (struct sock *)msk; + sk =3D (struct sock *)msk; =20 lock_sock_nested(sk, SINGLE_DEPTH_NESTING); next =3D msk->dl_next; msk->dl_next =3D NULL; =20 - /* prevent the stack from later re-schedule the worker for - * this socket - */ - inet_sk_state_store(sk, TCP_CLOSE); + __mptcp_unaccepted_force_close(sk); release_sock(sk); =20 /* lockdep will report a false positive ABBA deadlock --=20 2.39.2