From 29211e7db28ab12a4a5aaab4bcc080a3ac67ec78 Mon Sep 17 00:00:00 2001 From: Tim Gardner Date: Fri, 15 Oct 2021 16:05:50 -0700 Subject: [PATCH 1/3] mptcp: Avoid NULL dereference in mptcp_getsockopt_subflow_addrs() Coverity complains of a possible NULL dereference in mptcp_getsockopt_subflow_addrs(): 861 } else if (sk->sk_family == AF_INET6) { 3. returned_null: inet6_sk returns NULL. [show details] 4. var_assigned: Assigning: np = NULL return value from inet6_sk. 862 const struct ipv6_pinfo *np = inet6_sk(sk); Fix this by checking for NULL. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/231 Fixes: c11c5906bc0a ("mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support") Cc: Florian Westphal Signed-off-by: Tim Gardner [mjm: Added WARN_ON_ONCE() to the unexpected case] Signed-off-by: Mat Martineau Signed-off-by: David S. Miller --- net/mptcp/sockopt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c index 8137cc3a42960..0f1e661c2032b 100644 --- a/net/mptcp/sockopt.c +++ b/net/mptcp/sockopt.c @@ -861,6 +861,9 @@ static void mptcp_get_sub_addrs(const struct sock *sk, struct mptcp_subflow_addr } else if (sk->sk_family == AF_INET6) { const struct ipv6_pinfo *np = inet6_sk(sk); + if (WARN_ON_ONCE(!np)) + return; + a->sin6_local.sin6_family = AF_INET6; a->sin6_local.sin6_port = inet->inet_sport; From 72bcbc46a5c385961b9c167db79ad1f38f04e6a7 Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Fri, 15 Oct 2021 16:05:51 -0700 Subject: [PATCH 2/3] mptcp: increase default max additional subflows to 2 The current default does not allowing additional subflows, mostly as a safety restriction to avoid uncontrolled resource consumption on busy servers. Still the system admin and/or the application have to opt-in to MPTCP explicitly. After that, they need to change (increase) the default maximum number of additional subflows. Let set that to reasonable default, and make end-users life easier. Additionally we need to update some self-tests accordingly. Signed-off-by: Paolo Abeni Signed-off-by: Mat Martineau Signed-off-by: David S. Miller --- net/mptcp/pm_netlink.c | 3 +++ tools/testing/selftests/net/mptcp/mptcp_join.sh | 5 ++++- tools/testing/selftests/net/mptcp/pm_netlink.sh | 6 +++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 050eea231528b..f7d33a9abd57d 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -2052,6 +2052,9 @@ static int __net_init pm_nl_init_net(struct net *net) struct pm_nl_pernet *pernet = net_generic(net, pm_nl_pernet_id); INIT_LIST_HEAD_RCU(&pernet->local_addr_list); + + /* Cit. 2 subflows ought to be enough for anybody. */ + pernet->subflows_max = 2; pernet->next_id = 1; pernet->stale_loss_cnt = 4; spin_lock_init(&pernet->lock); diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 255793c5ac4ff..293d349e21fef 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -945,12 +945,15 @@ subflows_tests() # subflow limited by client reset + ip netns exec $ns1 ./pm_nl_ctl limits 0 0 + ip netns exec $ns2 ./pm_nl_ctl limits 0 0 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow run_tests $ns1 $ns2 10.0.1.1 chk_join_nr "single subflow, limited by client" 0 0 0 # subflow limited by server reset + ip netns exec $ns1 ./pm_nl_ctl limits 0 0 ip netns exec $ns2 ./pm_nl_ctl limits 0 1 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow run_tests $ns1 $ns2 10.0.1.1 @@ -973,7 +976,7 @@ subflows_tests() run_tests $ns1 $ns2 10.0.1.1 chk_join_nr "multiple subflows" 2 2 2 - # multiple subflows limited by serverf + # multiple subflows limited by server reset ip netns exec $ns1 ./pm_nl_ctl limits 0 1 ip netns exec $ns2 ./pm_nl_ctl limits 0 2 diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh index 3c741abe034ef..cbacf9f6538bd 100755 --- a/tools/testing/selftests/net/mptcp/pm_netlink.sh +++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh @@ -70,7 +70,7 @@ check() check "ip netns exec $ns1 ./pm_nl_ctl dump" "" "defaults addr list" check "ip netns exec $ns1 ./pm_nl_ctl limits" "accept 0 -subflows 0" "defaults limits" +subflows 2" "defaults limits" ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.1 ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.2 flags subflow dev lo @@ -118,11 +118,11 @@ check "ip netns exec $ns1 ./pm_nl_ctl dump" "" "flush addrs" ip netns exec $ns1 ./pm_nl_ctl limits 9 1 check "ip netns exec $ns1 ./pm_nl_ctl limits" "accept 0 -subflows 0" "rcv addrs above hard limit" +subflows 2" "rcv addrs above hard limit" ip netns exec $ns1 ./pm_nl_ctl limits 1 9 check "ip netns exec $ns1 ./pm_nl_ctl limits" "accept 0 -subflows 0" "subflows above hard limit" +subflows 2" "subflows above hard limit" ip netns exec $ns1 ./pm_nl_ctl limits 8 8 check "ip netns exec $ns1 ./pm_nl_ctl limits" "accept 8 From 3828c514726fce7d97063155c4749eafefd9fbd2 Mon Sep 17 00:00:00 2001 From: Mat Martineau Date: Fri, 15 Oct 2021 16:05:52 -0700 Subject: [PATCH 3/3] mptcp: Make mptcp_pm_nl_mp_prio_send_ack() static This function is only used within pm_netlink.c now. Fixes: 067065422fcd ("mptcp: add the outgoing MP_PRIO support") Reviewed-by: Matthieu Baerts Signed-off-by: Mat Martineau Signed-off-by: David S. Miller --- net/mptcp/pm_netlink.c | 6 +++--- net/mptcp/protocol.h | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index f7d33a9abd57d..7b96be1e9f14a 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -654,9 +654,9 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk) } } -int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, - struct mptcp_addr_info *addr, - u8 bkup) +static int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, + struct mptcp_addr_info *addr, + u8 bkup) { struct mptcp_subflow_context *subflow; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 7379ab580a7e5..284fdcec067ed 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -736,9 +736,6 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk); void mptcp_pm_rm_addr_received(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list); void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup); -int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk, - struct mptcp_addr_info *addr, - u8 bkup); void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq); void mptcp_pm_free_anno_list(struct mptcp_sock *msk); bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk);