From: Greg Kroah-Hartman Date: Mon, 26 Feb 2024 13:22:40 +0000 (+0100) Subject: 6.7-stable patches X-Git-Tag: v4.19.308~45 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b2e2e047c2dd55087c2ed3b80c4d7d4b37815ef2;p=thirdparty%2Fkernel%2Fstable-queue.git 6.7-stable patches added patches: mptcp-fix-data-races-on-local_id.patch mptcp-fix-data-races-on-remote_id.patch mptcp-fix-duplicate-subflow-creation.patch mptcp-fix-lockless-access-in-subflow-ulp-diag.patch --- diff --git a/queue-6.7/mptcp-fix-data-races-on-local_id.patch b/queue-6.7/mptcp-fix-data-races-on-local_id.patch new file mode 100644 index 00000000000..6a03d66c806 --- /dev/null +++ b/queue-6.7/mptcp-fix-data-races-on-local_id.patch @@ -0,0 +1,182 @@ +From a7cfe776637004a4c938fde78be4bd608c32c3ef Mon Sep 17 00:00:00 2001 +From: Paolo Abeni +Date: Thu, 15 Feb 2024 19:25:31 +0100 +Subject: mptcp: fix data races on local_id + +From: Paolo Abeni + +commit a7cfe776637004a4c938fde78be4bd608c32c3ef upstream. + +The local address id is accessed lockless by the NL PM, add +all the required ONCE annotation. There is a caveat: the local +id can be initialized late in the subflow life-cycle, and its +validity is controlled by the local_id_valid flag. + +Remove such flag and encode the validity in the local_id field +itself with negative value before initialization. That allows +accessing the field consistently with a single read operation. + +Fixes: 0ee4261a3681 ("mptcp: implement mptcp_pm_remove_subflow") +Cc: stable@vger.kernel.org +Signed-off-by: Paolo Abeni +Reviewed-by: Mat Martineau +Signed-off-by: Matthieu Baerts (NGI0) +Signed-off-by: David S. Miller +Signed-off-by: Greg Kroah-Hartman +--- + net/mptcp/diag.c | 2 +- + net/mptcp/pm_netlink.c | 6 +++--- + net/mptcp/pm_userspace.c | 2 +- + net/mptcp/protocol.c | 2 +- + net/mptcp/protocol.h | 15 ++++++++++++--- + net/mptcp/subflow.c | 9 +++++---- + 6 files changed, 23 insertions(+), 13 deletions(-) + +--- a/net/mptcp/diag.c ++++ b/net/mptcp/diag.c +@@ -65,7 +65,7 @@ static int subflow_get_info(struct sock + sf->map_data_len) || + nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_FLAGS, flags) || + nla_put_u8(skb, MPTCP_SUBFLOW_ATTR_ID_REM, sf->remote_id) || +- nla_put_u8(skb, MPTCP_SUBFLOW_ATTR_ID_LOC, sf->local_id)) { ++ nla_put_u8(skb, MPTCP_SUBFLOW_ATTR_ID_LOC, subflow_get_local_id(sf))) { + err = -EMSGSIZE; + goto nla_failure; + } +--- a/net/mptcp/pm_netlink.c ++++ b/net/mptcp/pm_netlink.c +@@ -800,7 +800,7 @@ static void mptcp_pm_nl_rm_addr_or_subfl + mptcp_for_each_subflow_safe(msk, subflow, tmp) { + struct sock *ssk = mptcp_subflow_tcp_sock(subflow); + int how = RCV_SHUTDOWN | SEND_SHUTDOWN; +- u8 id = subflow->local_id; ++ u8 id = subflow_get_local_id(subflow); + + if (rm_type == MPTCP_MIB_RMADDR && subflow->remote_id != rm_id) + continue; +@@ -809,7 +809,7 @@ static void mptcp_pm_nl_rm_addr_or_subfl + + pr_debug(" -> %s rm_list_ids[%d]=%u local_id=%u remote_id=%u mpc_id=%u", + rm_type == MPTCP_MIB_RMADDR ? "address" : "subflow", +- i, rm_id, subflow->local_id, subflow->remote_id, ++ i, rm_id, id, subflow->remote_id, + msk->mpc_endpoint_id); + spin_unlock_bh(&msk->pm.lock); + mptcp_subflow_shutdown(sk, ssk, how); +@@ -1994,7 +1994,7 @@ static int mptcp_event_add_subflow(struc + if (WARN_ON_ONCE(!sf)) + return -EINVAL; + +- if (nla_put_u8(skb, MPTCP_ATTR_LOC_ID, sf->local_id)) ++ if (nla_put_u8(skb, MPTCP_ATTR_LOC_ID, subflow_get_local_id(sf))) + return -EMSGSIZE; + + if (nla_put_u8(skb, MPTCP_ATTR_REM_ID, sf->remote_id)) +--- a/net/mptcp/pm_userspace.c ++++ b/net/mptcp/pm_userspace.c +@@ -234,7 +234,7 @@ static int mptcp_userspace_pm_remove_id_ + + lock_sock(sk); + mptcp_for_each_subflow(msk, subflow) { +- if (subflow->local_id == 0) { ++ if (READ_ONCE(subflow->local_id) == 0) { + has_id_0 = true; + break; + } +--- a/net/mptcp/protocol.c ++++ b/net/mptcp/protocol.c +@@ -99,7 +99,7 @@ static int __mptcp_socket_create(struct + subflow->subflow_id = msk->subflow_id++; + + /* This is the first subflow, always with id 0 */ +- subflow->local_id_valid = 1; ++ WRITE_ONCE(subflow->local_id, 0); + mptcp_sock_graft(msk->first, sk->sk_socket); + iput(SOCK_INODE(ssock)); + +--- a/net/mptcp/protocol.h ++++ b/net/mptcp/protocol.h +@@ -491,10 +491,9 @@ struct mptcp_subflow_context { + remote_key_valid : 1, /* received the peer key from */ + disposable : 1, /* ctx can be free at ulp release time */ + stale : 1, /* unable to snd/rcv data, do not use for xmit */ +- local_id_valid : 1, /* local_id is correctly initialized */ + valid_csum_seen : 1, /* at least one csum validated */ + is_mptfo : 1, /* subflow is doing TFO */ +- __unused : 9; ++ __unused : 10; + bool data_avail; + bool scheduled; + u32 remote_nonce; +@@ -505,7 +504,7 @@ struct mptcp_subflow_context { + u8 hmac[MPTCPOPT_HMAC_LEN]; /* MPJ subflow only */ + u64 iasn; /* initial ack sequence number, MPC subflows only */ + }; +- u8 local_id; ++ s16 local_id; /* if negative not initialized yet */ + u8 remote_id; + u8 reset_seen:1; + u8 reset_transient:1; +@@ -556,6 +555,7 @@ mptcp_subflow_ctx_reset(struct mptcp_sub + { + memset(&subflow->reset, 0, sizeof(subflow->reset)); + subflow->request_mptcp = 1; ++ WRITE_ONCE(subflow->local_id, -1); + } + + static inline u64 +@@ -1022,6 +1022,15 @@ int mptcp_pm_get_local_id(struct mptcp_s + int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc); + int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc); + ++static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow) ++{ ++ int local_id = READ_ONCE(subflow->local_id); ++ ++ if (local_id < 0) ++ return 0; ++ return local_id; ++} ++ + void __init mptcp_pm_nl_init(void); + void mptcp_pm_nl_work(struct mptcp_sock *msk); + void mptcp_pm_nl_rm_subflow_received(struct mptcp_sock *msk, +--- a/net/mptcp/subflow.c ++++ b/net/mptcp/subflow.c +@@ -577,8 +577,8 @@ do_reset: + + static void subflow_set_local_id(struct mptcp_subflow_context *subflow, int local_id) + { +- subflow->local_id = local_id; +- subflow->local_id_valid = 1; ++ WARN_ON_ONCE(local_id < 0 || local_id > 255); ++ WRITE_ONCE(subflow->local_id, local_id); + } + + static int subflow_chk_local_id(struct sock *sk) +@@ -587,7 +587,7 @@ static int subflow_chk_local_id(struct s + struct mptcp_sock *msk = mptcp_sk(subflow->conn); + int err; + +- if (likely(subflow->local_id_valid)) ++ if (likely(subflow->local_id >= 0)) + return 0; + + err = mptcp_pm_get_local_id(msk, (struct sock_common *)sk); +@@ -1731,6 +1731,7 @@ static struct mptcp_subflow_context *sub + pr_debug("subflow=%p", ctx); + + ctx->tcp_sock = sk; ++ WRITE_ONCE(ctx->local_id, -1); + + return ctx; + } +@@ -1966,7 +1967,7 @@ static void subflow_ulp_clone(const stru + new_ctx->idsn = subflow_req->idsn; + + /* this is the first subflow, id is always 0 */ +- new_ctx->local_id_valid = 1; ++ subflow_set_local_id(new_ctx, 0); + } else if (subflow_req->mp_join) { + new_ctx->ssn_offset = subflow_req->ssn_offset; + new_ctx->mp_join = 1; diff --git a/queue-6.7/mptcp-fix-data-races-on-remote_id.patch b/queue-6.7/mptcp-fix-data-races-on-remote_id.patch new file mode 100644 index 00000000000..bd86171554f --- /dev/null +++ b/queue-6.7/mptcp-fix-data-races-on-remote_id.patch @@ -0,0 +1,86 @@ +From 967d3c27127e71a10ff5c083583a038606431b61 Mon Sep 17 00:00:00 2001 +From: Paolo Abeni +Date: Thu, 15 Feb 2024 19:25:32 +0100 +Subject: mptcp: fix data races on remote_id + +From: Paolo Abeni + +commit 967d3c27127e71a10ff5c083583a038606431b61 upstream. + +Similar to the previous patch, address the data race on +remote_id, adding the suitable ONCE annotations. + +Fixes: bedee0b56113 ("mptcp: address lookup improvements") +Cc: stable@vger.kernel.org +Signed-off-by: Paolo Abeni +Reviewed-by: Mat Martineau +Signed-off-by: Matthieu Baerts (NGI0) +Signed-off-by: David S. Miller +Signed-off-by: Greg Kroah-Hartman +--- + net/mptcp/pm_netlink.c | 8 ++++---- + net/mptcp/subflow.c | 6 +++--- + 2 files changed, 7 insertions(+), 7 deletions(-) + +--- a/net/mptcp/pm_netlink.c ++++ b/net/mptcp/pm_netlink.c +@@ -443,7 +443,7 @@ static unsigned int fill_remote_addresse + mptcp_for_each_subflow(msk, subflow) { + ssk = mptcp_subflow_tcp_sock(subflow); + remote_address((struct sock_common *)ssk, &addrs[i]); +- addrs[i].id = subflow->remote_id; ++ addrs[i].id = READ_ONCE(subflow->remote_id); + if (deny_id0 && !addrs[i].id) + continue; + +@@ -799,18 +799,18 @@ static void mptcp_pm_nl_rm_addr_or_subfl + + mptcp_for_each_subflow_safe(msk, subflow, tmp) { + struct sock *ssk = mptcp_subflow_tcp_sock(subflow); ++ u8 remote_id = READ_ONCE(subflow->remote_id); + int how = RCV_SHUTDOWN | SEND_SHUTDOWN; + u8 id = subflow_get_local_id(subflow); + +- if (rm_type == MPTCP_MIB_RMADDR && subflow->remote_id != rm_id) ++ if (rm_type == MPTCP_MIB_RMADDR && remote_id != rm_id) + continue; + if (rm_type == MPTCP_MIB_RMSUBFLOW && !mptcp_local_id_match(msk, id, rm_id)) + continue; + + pr_debug(" -> %s rm_list_ids[%d]=%u local_id=%u remote_id=%u mpc_id=%u", + rm_type == MPTCP_MIB_RMADDR ? "address" : "subflow", +- i, rm_id, id, subflow->remote_id, +- msk->mpc_endpoint_id); ++ i, rm_id, id, remote_id, msk->mpc_endpoint_id); + spin_unlock_bh(&msk->pm.lock); + mptcp_subflow_shutdown(sk, ssk, how); + +--- a/net/mptcp/subflow.c ++++ b/net/mptcp/subflow.c +@@ -535,7 +535,7 @@ static void subflow_finish_connect(struc + subflow->backup = mp_opt.backup; + subflow->thmac = mp_opt.thmac; + subflow->remote_nonce = mp_opt.nonce; +- subflow->remote_id = mp_opt.join_id; ++ WRITE_ONCE(subflow->remote_id, mp_opt.join_id); + pr_debug("subflow=%p, thmac=%llu, remote_nonce=%u backup=%d", + subflow, subflow->thmac, subflow->remote_nonce, + subflow->backup); +@@ -1567,7 +1567,7 @@ int __mptcp_subflow_connect(struct sock + pr_debug("msk=%p remote_token=%u local_id=%d remote_id=%d", msk, + remote_token, local_id, remote_id); + subflow->remote_token = remote_token; +- subflow->remote_id = remote_id; ++ WRITE_ONCE(subflow->remote_id, remote_id); + subflow->request_join = 1; + subflow->request_bkup = !!(flags & MPTCP_PM_ADDR_FLAG_BACKUP); + subflow->subflow_id = msk->subflow_id++; +@@ -1974,7 +1974,7 @@ static void subflow_ulp_clone(const stru + new_ctx->fully_established = 1; + new_ctx->remote_key_valid = 1; + new_ctx->backup = subflow_req->backup; +- new_ctx->remote_id = subflow_req->remote_id; ++ WRITE_ONCE(new_ctx->remote_id, subflow_req->remote_id); + new_ctx->token = subflow_req->token; + new_ctx->thmac = subflow_req->thmac; + diff --git a/queue-6.7/mptcp-fix-duplicate-subflow-creation.patch b/queue-6.7/mptcp-fix-duplicate-subflow-creation.patch new file mode 100644 index 00000000000..0e56c28c01b --- /dev/null +++ b/queue-6.7/mptcp-fix-duplicate-subflow-creation.patch @@ -0,0 +1,97 @@ +From 045e9d812868a2d80b7a57b224ce8009444b7bbc Mon Sep 17 00:00:00 2001 +From: Paolo Abeni +Date: Thu, 15 Feb 2024 19:25:33 +0100 +Subject: mptcp: fix duplicate subflow creation + +From: Paolo Abeni + +commit 045e9d812868a2d80b7a57b224ce8009444b7bbc upstream. + +Fullmesh endpoints could end-up unexpectedly generating duplicate +subflows - same local and remote addresses - when multiple incoming +ADD_ADDR are processed before the PM creates the subflow for the local +endpoints. + +Address the issue explicitly checking for duplicates at subflow +creation time. + +To avoid a quadratic computational complexity, track the unavailable +remote address ids in a temporary bitmap and initialize such bitmap +with the remote ids of all the existing subflows matching the local +address currently processed. + +The above allows additionally replacing the existing code checking +for duplicate entry in the current set with a simple bit test +operation. + +Fixes: 2843ff6f36db ("mptcp: remote addresses fullmesh") +Cc: stable@vger.kernel.org +Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/435 +Signed-off-by: Paolo Abeni +Reviewed-by: Mat Martineau +Signed-off-by: Matthieu Baerts (NGI0) +Signed-off-by: David S. Miller +Signed-off-by: Greg Kroah-Hartman +--- + net/mptcp/pm_netlink.c | 33 ++++++++++++++++++--------------- + 1 file changed, 18 insertions(+), 15 deletions(-) + +--- a/net/mptcp/pm_netlink.c ++++ b/net/mptcp/pm_netlink.c +@@ -396,19 +396,6 @@ void mptcp_pm_free_anno_list(struct mptc + } + } + +-static bool lookup_address_in_vec(const struct mptcp_addr_info *addrs, unsigned int nr, +- const struct mptcp_addr_info *addr) +-{ +- int i; +- +- for (i = 0; i < nr; i++) { +- if (addrs[i].id == addr->id) +- return true; +- } +- +- return false; +-} +- + /* Fill all the remote addresses into the array addrs[], + * and return the array size. + */ +@@ -440,6 +427,16 @@ static unsigned int fill_remote_addresse + msk->pm.subflows++; + addrs[i++] = remote; + } else { ++ DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1); ++ ++ /* Forbid creation of new subflows matching existing ++ * ones, possibly already created by incoming ADD_ADDR ++ */ ++ bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1); ++ mptcp_for_each_subflow(msk, subflow) ++ if (READ_ONCE(subflow->local_id) == local->id) ++ __set_bit(subflow->remote_id, unavail_id); ++ + mptcp_for_each_subflow(msk, subflow) { + ssk = mptcp_subflow_tcp_sock(subflow); + remote_address((struct sock_common *)ssk, &addrs[i]); +@@ -447,11 +444,17 @@ static unsigned int fill_remote_addresse + if (deny_id0 && !addrs[i].id) + continue; + ++ if (test_bit(addrs[i].id, unavail_id)) ++ continue; ++ + if (!mptcp_pm_addr_families_match(sk, local, &addrs[i])) + continue; + +- if (!lookup_address_in_vec(addrs, i, &addrs[i]) && +- msk->pm.subflows < subflows_max) { ++ if (msk->pm.subflows < subflows_max) { ++ /* forbid creating multiple address towards ++ * this id ++ */ ++ __set_bit(addrs[i].id, unavail_id); + msk->pm.subflows++; + i++; + } diff --git a/queue-6.7/mptcp-fix-lockless-access-in-subflow-ulp-diag.patch b/queue-6.7/mptcp-fix-lockless-access-in-subflow-ulp-diag.patch new file mode 100644 index 00000000000..bb42f71ffd0 --- /dev/null +++ b/queue-6.7/mptcp-fix-lockless-access-in-subflow-ulp-diag.patch @@ -0,0 +1,88 @@ +From b8adb69a7d29c2d33eb327bca66476fb6066516b Mon Sep 17 00:00:00 2001 +From: Paolo Abeni +Date: Thu, 15 Feb 2024 19:25:30 +0100 +Subject: mptcp: fix lockless access in subflow ULP diag + +From: Paolo Abeni + +commit b8adb69a7d29c2d33eb327bca66476fb6066516b upstream. + +Since the introduction of the subflow ULP diag interface, the +dump callback accessed all the subflow data with lockless. + +We need either to annotate all the read and write operation accordingly, +or acquire the subflow socket lock. Let's do latter, even if slower, to +avoid a diffstat havoc. + +Fixes: 5147dfb50832 ("mptcp: allow dumping subflow context to userspace") +Cc: stable@vger.kernel.org +Signed-off-by: Paolo Abeni +Reviewed-by: Mat Martineau +Signed-off-by: Matthieu Baerts (NGI0) +Signed-off-by: David S. Miller +Signed-off-by: Greg Kroah-Hartman +--- + include/net/tcp.h | 2 +- + net/mptcp/diag.c | 6 +++++- + net/tls/tls_main.c | 2 +- + 3 files changed, 7 insertions(+), 3 deletions(-) + +--- a/include/net/tcp.h ++++ b/include/net/tcp.h +@@ -2502,7 +2502,7 @@ struct tcp_ulp_ops { + /* cleanup ulp */ + void (*release)(struct sock *sk); + /* diagnostic */ +- int (*get_info)(const struct sock *sk, struct sk_buff *skb); ++ int (*get_info)(struct sock *sk, struct sk_buff *skb); + size_t (*get_info_size)(const struct sock *sk); + /* clone ulp */ + void (*clone)(const struct request_sock *req, struct sock *newsk, +--- a/net/mptcp/diag.c ++++ b/net/mptcp/diag.c +@@ -13,17 +13,19 @@ + #include + #include "protocol.h" + +-static int subflow_get_info(const struct sock *sk, struct sk_buff *skb) ++static int subflow_get_info(struct sock *sk, struct sk_buff *skb) + { + struct mptcp_subflow_context *sf; + struct nlattr *start; + u32 flags = 0; ++ bool slow; + int err; + + start = nla_nest_start_noflag(skb, INET_ULP_INFO_MPTCP); + if (!start) + return -EMSGSIZE; + ++ slow = lock_sock_fast(sk); + rcu_read_lock(); + sf = rcu_dereference(inet_csk(sk)->icsk_ulp_data); + if (!sf) { +@@ -69,11 +71,13 @@ static int subflow_get_info(const struct + } + + rcu_read_unlock(); ++ unlock_sock_fast(sk, slow); + nla_nest_end(skb, start); + return 0; + + nla_failure: + rcu_read_unlock(); ++ unlock_sock_fast(sk, slow); + nla_nest_cancel(skb, start); + return err; + } +--- a/net/tls/tls_main.c ++++ b/net/tls/tls_main.c +@@ -1003,7 +1003,7 @@ static u16 tls_user_config(struct tls_co + return 0; + } + +-static int tls_get_info(const struct sock *sk, struct sk_buff *skb) ++static int tls_get_info(struct sock *sk, struct sk_buff *skb) + { + u16 version, cipher_type; + struct tls_context *ctx; diff --git a/queue-6.7/series b/queue-6.7/series index 549bf74ae2c..4d3e1752a44 100644 --- a/queue-6.7/series +++ b/queue-6.7/series @@ -213,3 +213,7 @@ usb-roles-fix-null-pointer-issue-when-put-module-s-reference.patch usb-roles-don-t-get-set_role-when-usb_role_switch-is-unregistered.patch mptcp-add-needs_id-for-userspace-appending-addr.patch mptcp-add-needs_id-for-netlink-appending-addr.patch +mptcp-fix-lockless-access-in-subflow-ulp-diag.patch +mptcp-fix-data-races-on-local_id.patch +mptcp-fix-data-races-on-remote_id.patch +mptcp-fix-duplicate-subflow-creation.patch