[PATCH mptcp-net] mptcp: fix NULL pointer dereference on fastopen early fallback

Paolo Abeni posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/07059bdc844f97430ff5c92afc548485b1c20d74.1680253474.git.pabeni@redhat.com
Maintainers: Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Dmytro Shytyi <dmytro@shytyi.net>
net/mptcp/fastopen.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
[PATCH mptcp-net] mptcp: fix NULL pointer dereference on fastopen early fallback
Posted by Paolo Abeni 1 year, 1 month ago
In case of early fallback to TCP, subflow_syn_recv_sock() deletes
the subflow context before returning the newly allocated sock to
the caller.

The fastopen path does not cope with the above unconditionally
dereferencing the subflow context.

Fixes: 36b122baf6a8 ("mptcp: add subflow_v(4,6)_send_synack()")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/fastopen.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index d237d142171c..bceaab8dd8e4 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -9,11 +9,18 @@
 void mptcp_fastopen_subflow_synack_set_params(struct mptcp_subflow_context *subflow,
 					      struct request_sock *req)
 {
-	struct sock *ssk = subflow->tcp_sock;
-	struct sock *sk = subflow->conn;
+	struct sock *sk, *ssk;
 	struct sk_buff *skb;
 	struct tcp_sock *tp;
 
+	/* on early fallback the subflow context is deleted by
+	 * subflow_syn_recv_sock()
+	 */
+	if (!subflow)
+		return;
+
+	ssk = subflow->tcp_sock;
+	sk = subflow->conn;
 	tp = tcp_sk(ssk);
 
 	subflow->is_mptfo = 1;
-- 
2.39.2
Re: [PATCH mptcp-net] mptcp: fix NULL pointer dereference on fastopen early fallback
Posted by Matthieu Baerts 1 year, 1 month ago
Hi Paolo,

On 31/03/2023 11:17, Paolo Abeni wrote:
> In case of early fallback to TCP, subflow_syn_recv_sock() deletes
> the subflow context before returning the newly allocated sock to
> the caller.
> 
> The fastopen path does not cope with the above unconditionally
> dereferencing the subflow context.

Good catch! Thank you for the patch!

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- f3df6e0f6a79: mptcp: fix NULL pointer dereference on fastopen early
fallback
- Results: f01e02dfadff..2ef375cf1603 (export-net)
- Results: 68957ea8fdf9..036dd2fa6b15 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230331T163332
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230331T163332

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net