Greg recently reported 3 patches that could not be applied without conflict in v6.1:
- e0266319413d ("mptcp: update local address flags when setting it") - f642c5c4d528 ("mptcp: hold pm lock when deleting entry") - db3eab8110bc ("mptcp: pm: use _rcu variant under rcu_read_lock")
Conflicts, if any, have been resolved, and documented in each patch.
Note that there are 3 extra patches added to avoid some conflicts:
- 14cb0e0bf39b ("mptcp: define more local variables sk") - 06afe09091ee ("mptcp: add userspace_pm_lookup_addr_by_id helper") - af250c27ea1c ("mptcp: drop lookup_by_id in lookup_addr")
The Stable-dep-of tags have been added to these patches.
1 extra patch has been included, it is supposed to be backported, but it was missing the Cc stable tag and it had conflicts:
- ce7356ae3594 ("mptcp: cope racing subflow creation in mptcp_rcv_space_adjust")
Geliang Tang (5): mptcp: define more local variables sk mptcp: add userspace_pm_lookup_addr_by_id helper mptcp: update local address flags when setting it mptcp: hold pm lock when deleting entry mptcp: drop lookup_by_id in lookup_addr
Matthieu Baerts (NGI0) (1): mptcp: pm: use _rcu variant under rcu_read_lock
Paolo Abeni (1): mptcp: cope racing subflow creation in mptcp_rcv_space_adjust
net/mptcp/pm_netlink.c | 15 ++++---- net/mptcp/pm_userspace.c | 77 ++++++++++++++++++++++++++-------------- net/mptcp/protocol.c | 3 +- 3 files changed, 60 insertions(+), 35 deletions(-)
From: Paolo Abeni pabeni@redhat.com
commit ce7356ae35943cc6494cc692e62d51a734062b7d upstream.
Additional active subflows - i.e. created by the in kernel path manager - are included into the subflow list before starting the 3whs.
A racing recvmsg() spooling data received on an already established subflow would unconditionally call tcp_cleanup_rbuf() on all the current subflows, potentially hitting a divide by zero error on the newly created ones.
Explicitly check that the subflow is in a suitable state before invoking tcp_cleanup_rbuf().
Fixes: c76c6956566f ("mptcp: call tcp_cleanup_rbuf on subflows") Signed-off-by: Paolo Abeni pabeni@redhat.com Reviewed-by: Matthieu Baerts (NGI0) matttbe@kernel.org Link: https://patch.msgid.link/02374660836e1b52afc91966b7535c8c5f7bafb0.1731060874... Signed-off-by: Jakub Kicinski kuba@kernel.org [ Conflicts in protocol.c, because commit f410cbea9f3d ("tcp: annotate data-races around tp->window_clamp") has not been backported to this version. The conflict is easy to resolve, because only the context is different, but not the line to modify. ] Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/protocol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 78ac5c538e13..1acd4e37a0ea 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2057,7 +2057,8 @@ static void mptcp_rcv_space_adjust(struct mptcp_sock *msk, int copied) slow = lock_sock_fast(ssk); WRITE_ONCE(ssk->sk_rcvbuf, rcvbuf); tcp_sk(ssk)->window_clamp = window_clamp; - tcp_cleanup_rbuf(ssk, 1); + if (tcp_can_send_ack(ssk)) + tcp_cleanup_rbuf(ssk, 1); unlock_sock_fast(ssk, slow); } }
From: Geliang Tang geliang.tang@suse.com
commit 14cb0e0bf39bd10429ba14e9e2f905f1144226fc upstream.
'(struct sock *)msk' is used several times in mptcp_nl_cmd_announce(), mptcp_nl_cmd_remove() or mptcp_userspace_pm_set_flags() in pm_userspace.c, it's worth adding a local variable sk to point it.
Reviewed-by: Matthieu Baerts matttbe@kernel.org Signed-off-by: Geliang Tang geliang.tang@suse.com Signed-off-by: Mat Martineau martineau@kernel.org Link: https://lore.kernel.org/r/20231025-send-net-next-20231025-v1-8-db8f25f798eb@... Signed-off-by: Jakub Kicinski kuba@kernel.org Stable-dep-of: 06afe09091ee ("mptcp: add userspace_pm_lookup_addr_by_id helper") Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/pm_userspace.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 748e3876ec6d..530f414e57d6 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -185,6 +185,7 @@ int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info) struct mptcp_pm_addr_entry addr_val; struct mptcp_sock *msk; int err = -EINVAL; + struct sock *sk; u32 token_val;
if (!addr || !token) { @@ -200,6 +201,8 @@ int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info) return err; }
+ sk = (struct sock *)msk; + if (!mptcp_pm_is_userspace(msk)) { GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected"); goto announce_err; @@ -223,7 +226,7 @@ int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info) goto announce_err; }
- lock_sock((struct sock *)msk); + lock_sock(sk); spin_lock_bh(&msk->pm.lock);
if (mptcp_pm_alloc_anno_list(msk, &addr_val.addr)) { @@ -233,11 +236,11 @@ int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info) }
spin_unlock_bh(&msk->pm.lock); - release_sock((struct sock *)msk); + release_sock(sk);
err = 0; announce_err: - sock_put((struct sock *)msk); + sock_put(sk); return err; }
@@ -284,6 +287,7 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) struct mptcp_sock *msk; LIST_HEAD(free_list); int err = -EINVAL; + struct sock *sk; u32 token_val; u8 id_val;
@@ -301,6 +305,8 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) return err; }
+ sk = (struct sock *)msk; + if (!mptcp_pm_is_userspace(msk)) { GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected"); goto remove_err; @@ -311,7 +317,7 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) goto remove_err; }
- lock_sock((struct sock *)msk); + lock_sock(sk);
list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) { if (entry->addr.id == id_val) { @@ -322,7 +328,7 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info)
if (!match) { GENL_SET_ERR_MSG(info, "address with specified id not found"); - release_sock((struct sock *)msk); + release_sock(sk); goto remove_err; }
@@ -330,15 +336,15 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info)
mptcp_pm_remove_addrs(msk, &free_list);
- release_sock((struct sock *)msk); + release_sock(sk);
list_for_each_entry_safe(match, entry, &free_list, list) { - sock_kfree_s((struct sock *)msk, match, sizeof(*match)); + sock_kfree_s(sk, match, sizeof(*match)); }
err = 0; remove_err: - sock_put((struct sock *)msk); + sock_put(sk); return err; }
@@ -560,6 +566,7 @@ int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token, { struct mptcp_sock *msk; int ret = -EINVAL; + struct sock *sk; u32 token_val;
token_val = nla_get_u32(token); @@ -568,6 +575,8 @@ int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token, if (!msk) return ret;
+ sk = (struct sock *)msk; + if (!mptcp_pm_is_userspace(msk)) goto set_flags_err;
@@ -575,11 +584,11 @@ int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token, rem->addr.family == AF_UNSPEC) goto set_flags_err;
- lock_sock((struct sock *)msk); + lock_sock(sk); ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc->addr, &rem->addr, bkup); - release_sock((struct sock *)msk); + release_sock(sk);
set_flags_err: - sock_put((struct sock *)msk); + sock_put(sk); return ret; }
From: Geliang Tang tanggeliang@kylinos.cn
commit 06afe09091ee69dc7ab058b4be9917ae59cc81e5 upstream.
Corresponding __lookup_addr_by_id() helper in the in-kernel netlink PM, this patch adds a new helper mptcp_userspace_pm_lookup_addr_by_id() to lookup the address entry with the given id on the userspace pm local address list.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Matthieu Baerts (NGI0) matttbe@kernel.org Reviewed-by: Mat Martineau martineau@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org Signed-off-by: David S. Miller davem@davemloft.net Stable-dep-of: f642c5c4d528 ("mptcp: hold pm lock when deleting entry") Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/pm_userspace.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 530f414e57d6..ca3e452d4edb 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -106,22 +106,29 @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, return -EINVAL; }
+static struct mptcp_pm_addr_entry * +mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id) +{ + struct mptcp_pm_addr_entry *entry; + + list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) { + if (entry->addr.id == id) + return entry; + } + return NULL; +} + int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id, u8 *flags, int *ifindex) { - struct mptcp_pm_addr_entry *entry, *match = NULL; + struct mptcp_pm_addr_entry *match;
*flags = 0; *ifindex = 0;
spin_lock_bh(&msk->pm.lock); - list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) { - if (id == entry->addr.id) { - match = entry; - break; - } - } + match = mptcp_userspace_pm_lookup_addr_by_id(msk, id); spin_unlock_bh(&msk->pm.lock); if (match) { *flags = match->flags; @@ -282,7 +289,7 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) { struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN]; struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID]; - struct mptcp_pm_addr_entry *match = NULL; + struct mptcp_pm_addr_entry *match; struct mptcp_pm_addr_entry *entry; struct mptcp_sock *msk; LIST_HEAD(free_list); @@ -319,13 +326,7 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info)
lock_sock(sk);
- list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) { - if (entry->addr.id == id_val) { - match = entry; - break; - } - } - + match = mptcp_userspace_pm_lookup_addr_by_id(msk, id_val); if (!match) { GENL_SET_ERR_MSG(info, "address with specified id not found"); release_sock(sk);
From: Geliang Tang tanggeliang@kylinos.cn
commit e0266319413d5d687ba7b6df7ca99e4b9724a4f2 upstream.
Just like in-kernel pm, when userspace pm does set_flags, it needs to send out MP_PRIO signal, and also modify the flags of the corresponding address entry in the local address list. This patch implements the missing logic.
Traverse all address entries on userspace_pm_local_addr_list to find the local address entry, if bkup is true, set the flags of this entry with FLAG_BACKUP, otherwise, clear FLAG_BACKUP.
Fixes: 892f396c8e68 ("mptcp: netlink: issue MP_PRIO signals from userspace PMs") Cc: stable@vger.kernel.org Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Matthieu Baerts (NGI0) matttbe@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org Link: https://patch.msgid.link/20241112-net-mptcp-misc-6-12-pm-v1-1-b835580cefa8@k... Signed-off-by: Jakub Kicinski kuba@kernel.org [ Conflicts in pm_userspace.c, because commit 6a42477fe449 ("mptcp: update set_flags interfaces"), is not in this version, and causes too many conflicts when backporting it. The same code can still be added at the same place, before sending the ACK. ] Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/pm_userspace.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index ca3e452d4edb..195f84f16b97 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -565,6 +565,7 @@ int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token, struct mptcp_pm_addr_entry *loc, struct mptcp_pm_addr_entry *rem, u8 bkup) { + struct mptcp_pm_addr_entry *entry; struct mptcp_sock *msk; int ret = -EINVAL; struct sock *sk; @@ -585,6 +586,17 @@ int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token, rem->addr.family == AF_UNSPEC) goto set_flags_err;
+ spin_lock_bh(&msk->pm.lock); + list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) { + if (mptcp_addresses_equal(&entry->addr, &loc->addr, false)) { + if (bkup) + entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP; + else + entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP; + } + } + spin_unlock_bh(&msk->pm.lock); + lock_sock(sk); ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc->addr, &rem->addr, bkup); release_sock(sk);
From: Geliang Tang tanggeliang@kylinos.cn
commit f642c5c4d528d11bd78b6c6f84f541cd3c0bea86 upstream.
When traversing userspace_pm_local_addr_list and deleting an entry from it in mptcp_pm_nl_remove_doit(), msk->pm.lock should be held.
This patch holds this lock before mptcp_userspace_pm_lookup_addr_by_id() and releases it after list_move() in mptcp_pm_nl_remove_doit().
Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Cc: stable@vger.kernel.org Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Matthieu Baerts (NGI0) matttbe@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org Link: https://patch.msgid.link/20241112-net-mptcp-misc-6-12-pm-v1-2-b835580cefa8@k... Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/pm_userspace.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 195f84f16b97..9016f8900c19 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -326,14 +326,17 @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info)
lock_sock(sk);
+ spin_lock_bh(&msk->pm.lock); match = mptcp_userspace_pm_lookup_addr_by_id(msk, id_val); if (!match) { GENL_SET_ERR_MSG(info, "address with specified id not found"); + spin_unlock_bh(&msk->pm.lock); release_sock(sk); goto remove_err; }
list_move(&match->list, &free_list); + spin_unlock_bh(&msk->pm.lock);
mptcp_pm_remove_addrs(msk, &free_list);
From: Geliang Tang tanggeliang@kylinos.cn
commit af250c27ea1c404e210fc3a308b20f772df584d6 upstream.
When the lookup_by_id parameter of __lookup_addr() is true, it's the same as __lookup_addr_by_id(), it can be replaced by __lookup_addr_by_id() directly. So drop this parameter, let __lookup_addr() only looks up address on the local address list by comparing addresses in it, not address ids.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Matthieu Baerts (NGI0) matttbe@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org Link: https://lore.kernel.org/r/20240305-upstream-net-next-20240304-mptcp-misc-cle... Signed-off-by: Jakub Kicinski kuba@kernel.org Stable-dep-of: db3eab8110bc ("mptcp: pm: use _rcu variant under rcu_read_lock") [ Conflicts in pm_netlink.c, because commit 6a42477fe449 ("mptcp: update set_flags interfaces") is not in this version, and causes too many conflicts when backporting it. The conflict is easy to resolve: addr is a pointer here here in mptcp_pm_nl_set_flags(), the rest of the code is the same. ] Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/pm_netlink.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 49e8156f5388..9b65d9360976 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -525,15 +525,12 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id) }
static struct mptcp_pm_addr_entry * -__lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info, - bool lookup_by_id) +__lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info) { struct mptcp_pm_addr_entry *entry;
list_for_each_entry(entry, &pernet->local_addr_list, list) { - if ((!lookup_by_id && - mptcp_addresses_equal(&entry->addr, info, entry->addr.port)) || - (lookup_by_id && entry->addr.id == info->id)) + if (mptcp_addresses_equal(&entry->addr, info, entry->addr.port)) return entry; } return NULL; @@ -564,7 +561,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
mptcp_local_address((struct sock_common *)msk->first, &mpc_addr); rcu_read_lock(); - entry = __lookup_addr(pernet, &mpc_addr, false); + entry = __lookup_addr(pernet, &mpc_addr); if (entry) { __clear_bit(entry->addr.id, msk->pm.id_avail_bitmap); msk->mpc_endpoint_id = entry->addr.id; @@ -2081,7 +2078,8 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info) token, &addr, &remote, bkup);
spin_lock_bh(&pernet->lock); - entry = __lookup_addr(pernet, &addr.addr, lookup_by_id); + entry = lookup_by_id ? __lookup_addr_by_id(pernet, addr.addr.id) : + __lookup_addr(pernet, &addr.addr); if (!entry) { spin_unlock_bh(&pernet->lock); return -EINVAL;
commit db3eab8110bc0520416101b6a5b52f44a43fb4cf upstream.
In mptcp_pm_create_subflow_or_signal_addr(), rcu_read_(un)lock() are used as expected to iterate over the list of local addresses, but list_for_each_entry() was used instead of list_for_each_entry_rcu() in __lookup_addr(). It is important to use this variant which adds the required READ_ONCE() (and diagnostic checks if enabled).
Because __lookup_addr() is also used in mptcp_pm_nl_set_flags() where it is called under the pernet->lock and not rcu_read_lock(), an extra condition is then passed to help the diagnostic checks making sure either the associated spin lock or the RCU lock is held.
Fixes: 86e39e04482b ("mptcp: keep track of local endpoint still available for each msk") Cc: stable@vger.kernel.org Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org Link: https://patch.msgid.link/20241112-net-mptcp-misc-6-12-pm-v1-3-b835580cefa8@k... Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/pm_netlink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 9b65d9360976..3fd7de56a30f 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -529,7 +529,8 @@ __lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info) { struct mptcp_pm_addr_entry *entry;
- list_for_each_entry(entry, &pernet->local_addr_list, list) { + list_for_each_entry_rcu(entry, &pernet->local_addr_list, list, + lockdep_is_held(&pernet->lock)) { if (mptcp_addresses_equal(&entry->addr, info, entry->addr.port)) return entry; }
On Tue, Nov 19, 2024 at 09:35:48AM +0100, Matthieu Baerts (NGI0) wrote:
Greg recently reported 3 patches that could not be applied without conflict in v6.1:
- e0266319413d ("mptcp: update local address flags when setting it")
- f642c5c4d528 ("mptcp: hold pm lock when deleting entry")
- db3eab8110bc ("mptcp: pm: use _rcu variant under rcu_read_lock")
Conflicts, if any, have been resolved, and documented in each patch.
Note that there are 3 extra patches added to avoid some conflicts:
- 14cb0e0bf39b ("mptcp: define more local variables sk")
- 06afe09091ee ("mptcp: add userspace_pm_lookup_addr_by_id helper")
- af250c27ea1c ("mptcp: drop lookup_by_id in lookup_addr")
The Stable-dep-of tags have been added to these patches.
1 extra patch has been included, it is supposed to be backported, but it was missing the Cc stable tag and it had conflicts:
- ce7356ae3594 ("mptcp: cope racing subflow creation in mptcp_rcv_space_adjust")
Geliang Tang (5): mptcp: define more local variables sk mptcp: add userspace_pm_lookup_addr_by_id helper mptcp: update local address flags when setting it mptcp: hold pm lock when deleting entry mptcp: drop lookup_by_id in lookup_addr
Matthieu Baerts (NGI0) (1): mptcp: pm: use _rcu variant under rcu_read_lock
Paolo Abeni (1): mptcp: cope racing subflow creation in mptcp_rcv_space_adjust
now queued up, thanks!
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Matthieu Baerts (NGI0)