From nobody Sun May 5 18:16:24 2024 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.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 6DAD2C945 for ; Thu, 23 Mar 2023 17:49:12 +0000 (UTC) Received: by mail-wr1-f51.google.com with SMTP id l27so13177233wrb.2 for ; Thu, 23 Mar 2023 10:49:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tessares.net; s=google; t=1679593750; 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=1BfWN3A1Bobar20T0OZnQswEN0+I8HQeY/5/OI19JVI=; b=Hg+LytVXsDCO6+hqd/bMD5vjQyRfbCg1veZnII0Aabyo0aelfba1CiNvMYYWN/baRk HrWdyheAYRsH7r6ED3sp5MQfP83+lHzXtr+tBg21lRudQZERamG+46SGrxPL7DIquyw0 edX1NtlKHgSMTh+sQK1+arrjJQrOtOR9LJ3WwbAoxizCjh6i3tT7/264KH8bT85NUm67 qOuBXKGS1MpYLF1kmYAtj+RkYWqON5vi+tvbitlD61GcVGmQ4TpuPYVBNelV+OYImURC ZnKdsLcgOPTRhQ/lBAcRt5VKfPMS3Y1EqALfSv9NQj44pbvqAuh4fFK5CR5UCUNz9P1q 5xEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679593750; 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=1BfWN3A1Bobar20T0OZnQswEN0+I8HQeY/5/OI19JVI=; b=ozCbsLsWl9x7V93Jl/W+E3JM71aRCNMWztKV2UnfSHjBgRYqpeNMunMBenrtBS4O36 iZ1L7TeF0XzOcV1r3OPGjI9alBfz7EYZHEA6XmnEV9dnNqcQCHjv1moOfA8KHIazDOQR cNnWCif0O8sS6s6lBEOIsWLV5Iy996jlmXnLOszktnT/xdzByRs2LUAerSn21HOZ8ilY E1zzJ0cvMakNOpZXW2WVnPQJPc7mwp6ALJ2rvPGZnXedvDHHZjL+zNSOBLp496lfdfw7 UP3h8HI/lEQqe9N4NC97Ed3ertWwCsNS5zhNbLOWuiAAQPZFNW7R8IaiangLQQXWHVw8 256Q== X-Gm-Message-State: AAQBX9fDNjcOWilGMWff+YfbmERrvGfvwMEmS5OPlVwOxS0Xb1WSLQTf tDf68GXQWii3Mt8QHINR3Fu4PQ== X-Google-Smtp-Source: AKy350a5bp740Xdu6DKGbuipFyN+zhVN40OVBY80otZQCprQrgh1+4ZLOJ4AKM4wKZX0ecyvPtbhQg== X-Received: by 2002:adf:e44f:0:b0:2cf:eb86:bd90 with SMTP id t15-20020adfe44f000000b002cfeb86bd90mr4137wrm.63.1679593750437; Thu, 23 Mar 2023 10:49:10 -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 a10-20020a056000050a00b002d78a96cf5fsm9534483wrf.70.2023.03.23.10.49.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Mar 2023 10:49:10 -0700 (PDT) From: Matthieu Baerts Date: Thu, 23 Mar 2023 18:49:01 +0100 Subject: [PATCH 6.1 1/2] mptcp: use the workqueue to destroy unaccepted sockets 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: <20230323-upstream-stable-conflicts-6-1-v1-1-ef2a6180eaf3@tessares.net> References: <20230323-upstream-stable-conflicts-6-1-v1-0-ef2a6180eaf3@tessares.net> In-Reply-To: <20230323-upstream-stable-conflicts-6-1-v1-0-ef2a6180eaf3@tessares.net> To: Greg Kroah-Hartman , Sasha Levin Cc: Paolo Abeni , stable@vger.kernel.org, mptcp@lists.linux.dev, Matthieu Baerts , Christoph Paasch , Jakub Kicinski X-Mailer: b4 0.12.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=9711; i=matthieu.baerts@tessares.net; h=from:subject:message-id; bh=zQHyjtXMq8YhMlfgF3RNcLViKWuLafHBG0HJH+4fWWo=; b=owEBbQKS/ZANAwAIAfa3gk9CaaBzAcsmYgBkHJEUKZYLu+OeK6GbaFSE55UmRp8NHskJzISt9 Yqevs5Q+eOJAjMEAAEIAB0WIQToy4X3aHcFem4n93r2t4JPQmmgcwUCZByRFAAKCRD2t4JPQmmg c68wEACToolarCWWGqcBZEsVHd8o/L6M+8HvXv3Bgo6c6K2jqhsU1IkJhF0pvlpdK4RAcp4pOc7 kglI+IYd2lG2yntxQl6ELCu1IjhoETUblPjk2LAELeYUBGow/rJ9vDyigeM0nERlOJ7UI4OuI88 nifW3lT1GTMLhyfrmqjg2ubRueo+5QKsnahOOkDapfq7uwnU0Pguu1SDQ3ATEqYEX3bSFcuglBS 4yRZINOvIIwZD6QBJMqKha6udJN6WfB2Y/Akbimbp9PpXTtDSWrJRypARJqsxLU8TDh0lC5SeQP FEbBUX0rdneJQkVsyQrVaZAsdM/ZRWzqsosA+CHfYC8qdJolEVIsLm2lbok+eJh7Cg1Wyd2np+i nFPOCSdNx78/1KP3Bk0q138r7pByHab+zZIi79Mn0SF7WF4fJvUSX1nM57XRusZMiZdMHjMoV+4 WPJ1o5oAuctWHk5TvJo4aRVYEdBAwFoa4SM6ZHrMMBIWHwqPaWjiVXOlYycXm6UMSyal1AloaB2 gca9uApyNoOXmtD0vgRlhDo0dq0Z1fWaMPk1gey2t1dHwB+Sbqq7W3gpQndVo0zCY+/p2Sskc9y ZGLSvhMLopYo0gU0Yp2gOfib+0MXHG8jKR/BWwudvNEIigObMecTB1C2uL2xmZOHZyy3UCxOPQn nfaTZf3RggN3s8A== X-Developer-Key: i=matthieu.baerts@tessares.net; a=openpgp; fpr=E8CB85F76877057A6E27F77AF6B7824F4269A073 From: Paolo Abeni [ Upstream commit b6985b9b82954caa53f862d6059d06c0526254f0 ] Backports notes: one simple conflict in net/mptcp/protocol.c with: commit a5ef058dc4d9 ("net: introduce and use custom sockopt socket flag= ") Where the two commits add a new line for different actions in the same context in mptcp_stream_accept(). Christoph reported a UaF at token lookup time after having refactored the passive socket initialization part: BUG: KASAN: use-after-free in __token_bucket_busy+0x253/0x260 Read of size 4 at addr ffff88810698d5b0 by task syz-executor653/3198 CPU: 1 PID: 3198 Comm: syz-executor653 Not tainted 6.2.0-rc59af4eaa31c1f6= c00c8f1e448ed99a45c66340dd5 #6 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-= gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 Call Trace: dump_stack_lvl+0x6e/0x91 print_report+0x16a/0x46f kasan_report+0xad/0x130 __token_bucket_busy+0x253/0x260 mptcp_token_new_connect+0x13d/0x490 mptcp_connect+0x4ed/0x860 __inet_stream_connect+0x80e/0xd90 tcp_sendmsg_fastopen+0x3ce/0x710 mptcp_sendmsg+0xff1/0x1a20 inet_sendmsg+0x11d/0x140 __sys_sendto+0x405/0x490 __x64_sys_sendto+0xdc/0x1b0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x72/0xdc We need to properly clean-up all the paired MPTCP-level resources and be sure to release the msk last, even when the unaccepted subflow is destroyed by the TCP internals via inet_child_forget(). We can re-use the existing MPTCP_WORK_CLOSE_SUBFLOW infra, explicitly checking that for the critical scenario: the closed subflow is the MPC one, the msk is not accepted and eventually going through full cleanup. With such change, __mptcp_destroy_sock() is always called on msk sockets, even on accepted ones. We don't need anymore to transiently drop one sk reference at msk clone time. Please note this commit depends on the parent one: mptcp: refactor passive socket initialization Fixes: 58b09919626b ("mptcp: create msk early") Cc: stable@vger.kernel.org Reported-and-tested-by: Christoph Paasch Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/347 Signed-off-by: Paolo Abeni Reviewed-by: Matthieu Baerts Signed-off-by: Matthieu Baerts Signed-off-by: Jakub Kicinski Signed-off-by: Matthieu Baerts --- net/mptcp/protocol.c | 41 +++++++++++++++++++++++++++++++---------- net/mptcp/protocol.h | 5 ++++- net/mptcp/subflow.c | 17 ++++++++++++----- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 777f795246ed..b679e8a430a8 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2357,7 +2357,6 @@ static void __mptcp_close_ssk(struct sock *sk, struct= sock *ssk, goto out; } =20 - sock_orphan(ssk); subflow->disposable =3D 1; =20 /* if ssk hit tcp_done(), tcp_cleanup_ulp() cleared the related ops @@ -2365,7 +2364,20 @@ static void __mptcp_close_ssk(struct sock *sk, struc= t sock *ssk, * reference owned by msk; */ 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) { @@ -2412,9 +2424,10 @@ static unsigned int mptcp_sync_mss(struct sock *sk, = u32 pmtu) return 0; } =20 -static void __mptcp_close_subflow(struct mptcp_sock *msk) +static void __mptcp_close_subflow(struct sock *sk) { struct mptcp_subflow_context *subflow, *tmp; + struct mptcp_sock *msk =3D mptcp_sk(sk); =20 might_sleep(); =20 @@ -2428,7 +2441,15 @@ static void __mptcp_close_subflow(struct mptcp_sock = *msk) if (!skb_queue_empty_lockless(&ssk->sk_receive_queue)) continue; =20 - mptcp_close_ssk((struct sock *)msk, ssk, subflow); + mptcp_close_ssk(sk, ssk, subflow); + } + + /* 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 @@ -2637,6 +2658,9 @@ static void mptcp_worker(struct work_struct *work) __mptcp_check_send_data_fin(sk); mptcp_check_data_fin(sk); =20 + if (test_and_clear_bit(MPTCP_WORK_CLOSE_SUBFLOW, &msk->flags)) + __mptcp_close_subflow(sk); + /* There is no point in keeping around an orphaned sk timedout or * closed, but we need the msk around to reply to incoming DATA_FIN, * even if it is orphaned and in FIN_WAIT2 state @@ -2652,9 +2676,6 @@ static void mptcp_worker(struct work_struct *work) } } =20 - if (test_and_clear_bit(MPTCP_WORK_CLOSE_SUBFLOW, &msk->flags)) - __mptcp_close_subflow(msk); - if (test_and_clear_bit(MPTCP_WORK_RTX, &msk->flags)) __mptcp_retrans(sk); =20 @@ -3084,6 +3105,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk, msk->local_key =3D subflow_req->local_key; msk->token =3D subflow_req->token; msk->subflow =3D NULL; + msk->in_accept_queue =3D 1; WRITE_ONCE(msk->fully_established, false); if (mp_opt->suboptions & OPTION_MPTCP_CSUMREQD) WRITE_ONCE(msk->csum_enabled, true); @@ -3110,8 +3132,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk, security_inet_csk_clone(nsk, req); bh_unlock_sock(nsk); =20 - /* keep a single reference */ - __sock_put(nsk); + /* note: the newly allocated socket refcount is 2 now */ return nsk; } =20 @@ -3167,8 +3188,6 @@ static struct sock *mptcp_accept(struct sock *sk, int= flags, int *err, goto out; } =20 - /* acquire the 2nd reference for the owning socket */ - sock_hold(new_mptcp_sock); newsk =3D new_mptcp_sock; MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPCAPABLEPASSIVEACK); } else { @@ -3726,6 +3745,8 @@ static int mptcp_stream_accept(struct socket *sock, s= truct socket *newsock, struct mptcp_subflow_context *subflow; struct sock *newsk =3D newsock->sk; =20 + msk->in_accept_queue =3D 0; + lock_sock(newsk); =20 /* set ssk->sk_socket of accept()ed flows to mptcp socket. diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 6f22ae13c984..2cddd5b52e8f 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -286,7 +286,8 @@ struct mptcp_sock { u8 recvmsg_inq:1, cork:1, nodelay:1, - fastopening:1; + fastopening:1, + in_accept_queue:1; int connect_flags; struct work_struct work; struct sk_buff *ooo_last_skb; @@ -651,6 +652,8 @@ void mptcp_subflow_set_active(struct mptcp_subflow_cont= ext *subflow); =20 bool mptcp_subflow_active(struct mptcp_subflow_context *subflow); =20 +void mptcp_subflow_drop_ctx(struct sock *ssk); + static inline void mptcp_subflow_tcp_fallback(struct sock *sk, struct mptcp_subflow_context *ctx) { diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index fe815103060c..459621a0410c 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -636,9 +636,10 @@ static bool subflow_hmac_valid(const struct request_so= ck *req, =20 static void mptcp_force_close(struct sock *sk) { - /* the msk is not yet exposed to user-space */ + /* the msk is not yet exposed to user-space, and refcount is 2 */ inet_sk_state_store(sk, TCP_CLOSE); sk_common_release(sk); + sock_put(sk); } =20 static void subflow_ulp_fallback(struct sock *sk, @@ -654,7 +655,7 @@ static void subflow_ulp_fallback(struct sock *sk, mptcp_subflow_ops_undo_override(sk); } =20 -static void subflow_drop_ctx(struct sock *ssk) +void mptcp_subflow_drop_ctx(struct sock *ssk) { struct mptcp_subflow_context *ctx =3D mptcp_subflow_ctx(ssk); =20 @@ -758,7 +759,7 @@ static struct sock *subflow_syn_recv_sock(const struct = sock *sk, =20 if (new_msk) mptcp_copy_inaddrs(new_msk, child); - subflow_drop_ctx(child); + mptcp_subflow_drop_ctx(child); goto out; } =20 @@ -849,7 +850,7 @@ static struct sock *subflow_syn_recv_sock(const struct = sock *sk, return child; =20 dispose_child: - subflow_drop_ctx(child); + mptcp_subflow_drop_ctx(child); tcp_rsk(req)->drop_req =3D true; inet_csk_prepare_for_destroy_sock(child); tcp_done(child); @@ -1804,7 +1805,6 @@ void mptcp_subflow_queue_clean(struct sock *listener_= sk, struct sock *listener_s struct sock *sk =3D (struct sock *)msk; bool do_cancel_work; =20 - sock_hold(sk); lock_sock_nested(sk, SINGLE_DEPTH_NESTING); next =3D msk->dl_next; msk->first =3D NULL; @@ -1892,6 +1892,13 @@ static void subflow_ulp_release(struct sock *ssk) * when the subflow is still unaccepted */ release =3D ctx->disposable || list_empty(&ctx->node); + + /* inet_child_forget() does not call sk_state_change(), + * explicitly trigger the socket close machinery + */ + if (!release && !test_and_set_bit(MPTCP_WORK_CLOSE_SUBFLOW, + &mptcp_sk(sk)->flags)) + mptcp_schedule_work(sk); sock_put(sk); } =20 --=20 2.39.2 From nobody Sun May 5 18:16:24 2024 Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) (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 1F1ABC946 for ; Thu, 23 Mar 2023 17:49:13 +0000 (UTC) Received: by mail-wr1-f53.google.com with SMTP id v1so15397400wrv.1 for ; Thu, 23 Mar 2023 10:49:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tessares.net; s=google; t=1679593751; 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=fiuLdZpjuG0HQD+1vefGYXnEvP4jmAqSFBj5lWODCIg=; b=H0ZUqj6NVHgEnT162OT1jRFD+CCI/VkTbHJ+wrUpK4b/xRQ+jG9S8iUB+eEnxWeLna MfN7Y2f7If7QbEvT0K6b0ZWcTPUADU08/rSlvaTY0WdvrT+OyZxj8q2cx4H19dT1fzLV +DV/G0xb1BimjRqzE/9QqPnMZW4RrwtMX6Du3bfJVJDvm52YCIRe6DgmhuP0easud82s siSnddw15mbGypoZcTksSRGZjdz3Ml1YvJFJ2GmVMIqN/U4ma+mMGREmiajg/VPABTFd +Fk/fpMkTPV1ZZP9o7J3UrKcSKeu0S1pQAIKEM/5m1jdPPWbIXoT6oQevXe5GNcTjQ9V foog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679593751; 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=fiuLdZpjuG0HQD+1vefGYXnEvP4jmAqSFBj5lWODCIg=; b=0Jy7wZLcGTty7zAVeaKKvHhhyN+j8DIblUvVdeGCvjzFXgYuDnvQ8bOliPyaUjLu5v eFF0M94SAet9xtjgc5VeyZdRdcXMkRP1UeX6Ol+WNlTRSiEERgNqmxBKw9TjVXL4/ekC lV/UjhvTZhyR37SKyAbeTyS3DBzGnJxOra0mbkkg+XZitwT/X7WTdWiXT7EbEyl3AB88 etrzHPmqxlMgBQa0PH9Wg6lwrVfAdyUUxS4AVq29wqHViaxvPq1kMPXMAi8D2gj2D2YA Bv91seeu67Ut9LOY8iSFXztrT6pYn5H6EF1KWoQxUM+8i+1e17ty9nLD8BCK7TUJxnpW jC8w== X-Gm-Message-State: AAQBX9cyEpEd3SH9hLLHrlr8h7QJpzYwQkcg9EnjXlchqupnTHgbepki 9Gn1oa/DPsoj03Cwf6ppuN1UfA== X-Google-Smtp-Source: AKy350bpPRyM5VRTz5Po1/Xsv2QtKdZIROaL1tIJw9PiRzW/nNodA22AbTWNNviT5eyo7ovBwGIuPg== X-Received: by 2002:adf:e2c6:0:b0:2ce:a1a6:d721 with SMTP id d6-20020adfe2c6000000b002cea1a6d721mr570wrj.67.1679593751346; Thu, 23 Mar 2023 10:49:11 -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 a10-20020a056000050a00b002d78a96cf5fsm9534483wrf.70.2023.03.23.10.49.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Mar 2023 10:49:11 -0700 (PDT) From: Matthieu Baerts Date: Thu, 23 Mar 2023 18:49:02 +0100 Subject: [PATCH 6.1 2/2] mptcp: fix UaF in listener shutdown 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: <20230323-upstream-stable-conflicts-6-1-v1-2-ef2a6180eaf3@tessares.net> References: <20230323-upstream-stable-conflicts-6-1-v1-0-ef2a6180eaf3@tessares.net> In-Reply-To: <20230323-upstream-stable-conflicts-6-1-v1-0-ef2a6180eaf3@tessares.net> To: Greg Kroah-Hartman , Sasha Levin Cc: Paolo Abeni , stable@vger.kernel.org, mptcp@lists.linux.dev, Matthieu Baerts , Christoph Paasch , Jakub Kicinski X-Mailer: b4 0.12.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=6728; i=matthieu.baerts@tessares.net; h=from:subject:message-id; bh=sJn0BTts+nPdaj6HqOpzebHLPJkm+ftIKBAPaxdik7o=; b=owEBbQKS/ZANAwAIAfa3gk9CaaBzAcsmYgBkHJEUU21YODHKRh6jb82xS4noV1+aJOB+rF+lb lD6+ZRaRxqJAjMEAAEIAB0WIQToy4X3aHcFem4n93r2t4JPQmmgcwUCZByRFAAKCRD2t4JPQmmg c4rWD/0Wz0el7MnGkz69XralvYkDEyAgnGsOaNhUhaveL19IGONwenJnq01Z62cUtRjNVPWT4gG wGlYWhI9JIyTPeqRBak5fSNkRk4NMaXDQyC5pkX6FG/cVdL0xSmmm8+uN6UNdhsjFUQbGc5Hfe+ iHDG3CzwaDpXh/5VaZeh1US0EY0jJvEpKZBbI+HC6DDfShvPYz286KoQmpZARIKZy3NJpB5es+f XdegFbxxPh/L99KxvB/nQy0n6UzKZ0ugtHLziudjfCP5wJgDpetbqHBXo1diYoFxnTNHSBPlWnc yWdh9pLb0eODM/rT4OunQ6kQqyZsyrreZWlsNWFfHiISthX0xfB/IyKCaLgJj3gkttr9m7+B0T8 jHroU0qWh5Z/RGIpdXmTgnGdnbHOzeOcuurHPHrhsY0CqX5wLdPmE1VZqKP+7v/fRb/ijVgPRqr nk1EQZZhh/fUOX0zlus36LqVXL+DjM4eHylskXd3OpoCAwIeW5HkVsPDg00CFgm4ACoXK9ZSlOT WooK5N57wXI7PFPDrO/d3YcgV7i9xSCt9FINh7iZ2YBXbU2IDYVdSJKtTqy2vQmTYr/nidfPJ3H 0drv8dPtaLW/8tAJj26OcjNxSEChkBnbmHuQvR20J2jK5KQx4p3jXO3ghx+EYxBX6YMVGxPbJGT pQB2VhkQo33KJ+Q== X-Developer-Key: i=matthieu.baerts@tessares.net; a=openpgp; fpr=E8CB85F76877057A6E27F77AF6B7824F4269A073 From: Paolo Abeni [ Upstream commit 0a3f4f1f9c27215e4ddcd312558342e57b93e518 ] Backports notes: one simple conflict in net/mptcp/protocol.c with: commit f8c9dfbd875b ("mptcp: add pm listener events") Where one commit removes code in __mptcp_close_ssk() while the other one adds one line at the same place. We can simply remove the whole condition because this extra instruction is not present in v6.1. As reported by Christoph after having refactored the passive socket initialization, the mptcp listener shutdown path is prone to an UaF issue. BUG: KASAN: use-after-free in _raw_spin_lock_bh+0x73/0xe0 Write of size 4 at addr ffff88810cb23098 by task syz-executor731/1266 CPU: 1 PID: 1266 Comm: syz-executor731 Not tainted 6.2.0-rc59af4eaa31c1f6= c00c8f1e448ed99a45c66340dd5 #6 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-= gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 Call Trace: dump_stack_lvl+0x6e/0x91 print_report+0x16a/0x46f kasan_report+0xad/0x130 kasan_check_range+0x14a/0x1a0 _raw_spin_lock_bh+0x73/0xe0 subflow_error_report+0x6d/0x110 sk_error_report+0x3b/0x190 tcp_disconnect+0x138c/0x1aa0 inet_child_forget+0x6f/0x2e0 inet_csk_listen_stop+0x209/0x1060 __mptcp_close_ssk+0x52d/0x610 mptcp_destroy_common+0x165/0x640 mptcp_destroy+0x13/0x80 __mptcp_destroy_sock+0xe7/0x270 __mptcp_close+0x70e/0x9b0 mptcp_close+0x2b/0x150 inet_release+0xe9/0x1f0 __sock_release+0xd2/0x280 sock_close+0x15/0x20 __fput+0x252/0xa20 task_work_run+0x169/0x250 exit_to_user_mode_prepare+0x113/0x120 syscall_exit_to_user_mode+0x1d/0x40 do_syscall_64+0x48/0x90 entry_SYSCALL_64_after_hwframe+0x72/0xdc The msk grace period can legitly expire in between the last reference count dropped in mptcp_subflow_queue_clean() and the later eventual access in inet_csk_listen_stop() After the previous patch we don't need anymore special-casing msk listener socket cleanup: the mptcp worker will process each of the unaccepted msk sockets. Just drop the now unnecessary code. Please note this commit depends on the two parent ones: mptcp: refactor passive socket initialization mptcp: use the workqueue to destroy unaccepted sockets Fixes: 6aeed9045071 ("mptcp: fix race on unaccepted mptcp sockets") Cc: stable@vger.kernel.org Reported-and-tested-by: Christoph Paasch Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/346 Signed-off-by: Paolo Abeni Reviewed-by: Matthieu Baerts Signed-off-by: Matthieu Baerts Signed-off-by: Jakub Kicinski Signed-off-by: Matthieu Baerts --- net/mptcp/protocol.c | 5 ---- net/mptcp/protocol.h | 1 - net/mptcp/subflow.c | 72 ------------------------------------------------= ---- 3 files changed, 78 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index b679e8a430a8..f0cde2d7233d 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2380,11 +2380,6 @@ 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 2cddd5b52e8f..051e8022d661 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -615,7 +615,6 @@ 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 459621a0410c..fc876c248002 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1764,78 +1764,6 @@ 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; - struct mptcp_sock *msk; - - 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; - - 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; - bool do_cancel_work; - - lock_sock_nested(sk, SINGLE_DEPTH_NESTING); - next =3D msk->dl_next; - msk->first =3D NULL; - msk->dl_next =3D NULL; - - do_cancel_work =3D __mptcp_close(sk, 0); - release_sock(sk); - if (do_cancel_work) { - /* 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, - SINGLE_DEPTH_NESTING, 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