Commit 398b7c37 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'mptcp-fixes' 



Matthieu Baerts says:

====================
mptcp: misc. fixes for v6.8

This series includes 4 types of fixes:

Patches 1 and 2 force the path-managers not to allocate a new address
entry when dealing with the "special" ID 0, reserved to the address of
the initial subflow. These patches can be backported up to v5.19 and
v5.12 respectively.

Patch 3 to 6 fix the in-kernel path-manager not to create duplicated
subflows. Patch 6 is the main fix, but patches 3 to 5 are some kind of
pre-requisities: they fix some data races that could also lead to the
creation of unexpected subflows. These patches can be backported up to
v5.7, v5.10, v6.0, and v5.15 respectively.

Note that patch 3 modifies the existing ULP API. No better solutions
have been found for -net, and there is some similar prior art, see
commit 0df48c26 ("tcp: add tcpi_bytes_acked to tcp_info"). Please
also note that TLS ULP Diag has likely the same issue.

Patches 7 to 9 fix issues in the selftests, when executing them on older
kernels, e.g. when testing the last version of these kselftests on the
v5.15.148 kernel as it is done by LKFT when validating stable kernels.
These patches only avoid printing expected errors the console and
marking some tests as "OK" while they have been skipped. Patches 7 and 8
can be backported up to v6.6.

Patches 10 to 13 make sure all MPTCP selftests subtests have a unique
name. It is important to have a unique (sub)test name in TAP, because
that's the test identifier. Some CI environments might drop tests with
duplicated names. Patches 10 to 12 can be backported up to v6.6.
====================

Signed-off-by: default avatarMatthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 59a646d6 4103d848

include/net/tcp.h

+1 −1
Original line number Diff line number Diff line
@@ -2506,7 +2506,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,

net/mptcp/diag.c

+6 −2
Original line number Diff line number Diff line
@@ -13,17 +13,19 @@ 
#include <uapi/linux/mptcp.h>

#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) {
@@ -63,17 +65,19 @@ static int subflow_get_info(const struct sock *sk, struct sk_buff *skb) 
			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;

	}



	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;

}

net/mptcp/pm_netlink.c

+43 −26
Original line number Diff line number Diff line
@@ -396,19 +396,6 @@ void mptcp_pm_free_anno_list(struct mptcp_sock *msk) 
	}

}



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,18 +427,34 @@ static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk, 
		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]);

			addrs[i].id = subflow->remote_id;

			addrs[i].id = READ_ONCE(subflow->remote_id);

			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++;

			}
@@ -799,18 +802,18 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk, 


		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->local_id;

			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, subflow->local_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);
@@ -901,7 +904,8 @@ static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry) 
}



static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,

					     struct mptcp_pm_addr_entry *entry)

					     struct mptcp_pm_addr_entry *entry,

					     bool needs_id)

{

	struct mptcp_pm_addr_entry *cur, *del_entry = NULL;

	unsigned int addr_max;
@@ -949,7 +953,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet, 
		}

	}



	if (!entry->addr.id) {

	if (!entry->addr.id && needs_id) {

find_next:

		entry->addr.id = find_next_zero_bit(pernet->id_bitmap,

						    MPTCP_PM_MAX_ADDR_ID + 1,
@@ -960,7 +964,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet, 
		}

	}



	if (!entry->addr.id)

	if (!entry->addr.id && needs_id)

		goto out;



	__set_bit(entry->addr.id, pernet->id_bitmap);
@@ -1092,7 +1096,7 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc 
	entry->ifindex = 0;

	entry->flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;

	entry->lsk = NULL;

	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);

	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true);

	if (ret < 0)

		kfree(entry);
@@ -1285,6 +1289,18 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net) 
	return 0;

}



static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,

				      struct genl_info *info)

{

	struct nlattr *tb[MPTCP_PM_ADDR_ATTR_MAX + 1];



	if (!nla_parse_nested_deprecated(tb, MPTCP_PM_ADDR_ATTR_MAX, attr,

					 mptcp_pm_address_nl_policy, info->extack) &&

	    tb[MPTCP_PM_ADDR_ATTR_ID])

		return true;

	return false;

}



int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)

{

	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
@@ -1326,7 +1342,8 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info) 
			goto out_free;

		}

	}

	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);

	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,

						!mptcp_pm_has_addr_attr_id(attr, info));

	if (ret < 0) {

		GENL_SET_ERR_MSG_FMT(info, "too many addresses or duplicate one: %d", ret);

		goto out_free;
@@ -1980,7 +1997,7 @@ static int mptcp_event_add_subflow(struct sk_buff *skb, const struct sock *ssk) 
	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))

net/mptcp/pm_userspace.c

+8 −7
Original line number Diff line number Diff line
@@ -26,7 +26,8 @@ void mptcp_free_local_addr_list(struct mptcp_sock *msk) 
}



static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,

						    struct mptcp_pm_addr_entry *entry)

						    struct mptcp_pm_addr_entry *entry,

						    bool needs_id)

{

	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);

	struct mptcp_pm_addr_entry *match = NULL;
@@ -41,7 +42,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk, 
	spin_lock_bh(&msk->pm.lock);

	list_for_each_entry(e, &msk->pm.userspace_pm_local_addr_list, list) {

		addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true);

		if (addr_match && entry->addr.id == 0)

		if (addr_match && entry->addr.id == 0 && needs_id)

			entry->addr.id = e->addr.id;

		id_match = (e->addr.id == entry->addr.id);

		if (addr_match && id_match) {
@@ -64,7 +65,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk, 
		}



		*e = *entry;

		if (!e->addr.id)

		if (!e->addr.id && needs_id)

			e->addr.id = find_next_zero_bit(id_bitmap,

							MPTCP_PM_MAX_ADDR_ID + 1,

							1);
@@ -153,7 +154,7 @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, 
	if (new_entry.addr.port == msk_sport)

		new_entry.addr.port = 0;



	return mptcp_userspace_pm_append_new_local_addr(msk, &new_entry);

	return mptcp_userspace_pm_append_new_local_addr(msk, &new_entry, true);

}



int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
@@ -198,7 +199,7 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info) 
		goto announce_err;

	}



	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val);

	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val, false);

	if (err < 0) {

		GENL_SET_ERR_MSG(info, "did not match address and id");

		goto announce_err;
@@ -233,7 +234,7 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk, 


	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;

		}
@@ -378,7 +379,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info) 
	}



	local.addr = addr_l;

	err = mptcp_userspace_pm_append_new_local_addr(msk, &local);

	err = mptcp_userspace_pm_append_new_local_addr(msk, &local, false);

	if (err < 0) {

		GENL_SET_ERR_MSG(info, "did not match address and id");

		goto create_err;

net/mptcp/protocol.c

+1 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ static int __mptcp_socket_create(struct mptcp_sock *msk) 
	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));