This patch created a listening socket when an address with a port-number is added by PM netlink. Then binded the new port to the socket, and listened for the connection. Signed-off-by: Geliang Tang <geliangtang(a)gmail.com&gt; --- net/mptcp/pm_netlink.c | 58 ++++++++++++++++++++++++++++++++++++++++++ net/mptcp/protocol.c | 2 +- net/mptcp/protocol.h | 3 +++ net/mptcp/subflow.c | 4 +-- 4 files changed, 64 insertions(+), 3 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 5151cfcd6962..c296927bf167 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -26,6 +26,7 @@ struct mptcp_pm_addr_entry { struct list_head list; struct mptcp_addr_info addr; struct rcu_head rcu; + struct socket *lsk; }; struct mptcp_pm_add_entry { @@ -732,6 +733,53 @@ static struct pm_nl_pernet *genl_info_pm_nl(struct genl_info *info) return net_generic(genl_info_net(info), pm_nl_pernet_id); } +static int mptcp_pm_nl_create_listen_socket(struct sock *sk, + struct mptcp_pm_addr_entry *entry) +{ + struct sockaddr_storage addr; + struct mptcp_sock *msk; + struct socket *ssock; + int backlog = 20; + int err; + + err = sock_create_kern(sock_net(sk), entry->addr.family, + SOCK_STREAM, IPPROTO_MPTCP, &entry->lsk); + if (err) + return err; + + msk = mptcp_sk(entry->lsk->sk); + if (!msk) { + err = -EINVAL; + goto out; + } + + ssock = __mptcp_nmpc_socket(msk); + if (!ssock) { + err = -EINVAL; + goto out; + } + + mptcp_info2sockaddr(&entry->addr, &addr); + err = kernel_bind(ssock, (struct sockaddr *)&addr, + sizeof(struct sockaddr_in)); + if (err) { + pr_warn("kernel_bind error, err=%d", err); + goto out; + } + + err = kernel_listen(ssock, backlog); + if (err) { + pr_warn("kernel_listen error, err=%d", err); + goto out; + } + + return 0; + +out: + sock_release(entry->lsk); + return err; +} + static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info) { struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR]; @@ -750,9 +798,19 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info) } *entry = addr; + if (entry->addr.port) { + ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry); + if (ret) { + GENL_SET_ERR_MSG(info, "create listen socket error"); + kfree(entry); + return ret; + } + } ret = mptcp_pm_nl_append_new_local_addr(pernet, entry); if (ret < 0) { GENL_SET_ERR_MSG(info, "too many addresses or duplicate one"); + if (entry->lsk) + sock_release(entry->lsk); kfree(entry); return ret; } diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 4c36969873b9..5e464dfc0f6f 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -49,7 +49,7 @@ static void __mptcp_check_send_data_fin(struct sock *sk); * completed yet or has failed, return the subflow socket. * Otherwise return NULL. */ -static struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk) +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk) { if (!msk->subflow || READ_ONCE(msk->can_ack)) return NULL; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 9d8f01aac91c..ec179f3a6b4b 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -466,11 +466,14 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how); void __mptcp_close_ssk(struct sock *sk, struct sock *ssk, struct mptcp_subflow_context *subflow); void mptcp_subflow_reset(struct sock *ssk); +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk); /* called with sk socket lock held */ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, const struct mptcp_addr_info *remote); int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock); +void mptcp_info2sockaddr(const struct mptcp_addr_info *info, + struct sockaddr_storage *addr); static inline void mptcp_subflow_tcp_fallback(struct sock *sk, struct mptcp_subflow_context *ctx) diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 96c585f003f8..43cc5e2c3234 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1035,8 +1035,8 @@ void mptcpv6_handle_mapped(struct sock *sk, bool mapped) } #endif -static void mptcp_info2sockaddr(const struct mptcp_addr_info *info, - struct sockaddr_storage *addr) +void mptcp_info2sockaddr(const struct mptcp_addr_info *info, + struct sockaddr_storage *addr) { memset(addr, 0, sizeof(*addr)); addr->ss_family = info->family; -- 2.26.2

When we received the MP_JOIN's SYN/ACK, we do the port number checks. If the subflow's source port number is different, we use a new helper mptcp_pm_sport_in_anno_list to check whether this port number is announced. If it isn't, we need to abort this connection. Signed-off-by: Geliang Tang <geliangtang(a)gmail.com&gt; --- net/mptcp/pm_netlink.c | 34 ++++++++++++++++++++++++++++++++++ net/mptcp/protocol.h | 1 + net/mptcp/subflow.c | 10 ++++++++++ 3 files changed, 45 insertions(+) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index c296927bf167..a41c41974eb9 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -111,6 +111,19 @@ static void remote_address(const struct sock_common *skc, #endif } +static void source_address(struct sock_common *skc, + struct mptcp_addr_info *addr) +{ + addr->family = skc->skc_family; + addr->port = htons(skc->skc_num); + if (addr->family == AF_INET) + addr->addr.s_addr = skc->skc_rcv_saddr; +#if IS_ENABLED(CONFIG_MPTCP_IPV6) + else if (addr->family == AF_INET6) + addr->addr6 = skc->skc_v6_rcv_saddr; +#endif +} + static bool lookup_subflow_by_saddr(const struct list_head *list, struct mptcp_addr_info *saddr) { @@ -202,6 +215,27 @@ lookup_anno_list_by_saddr(struct mptcp_sock *msk, return NULL; } +bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk) +{ + struct mptcp_pm_add_entry *entry; + struct mptcp_addr_info saddr; + bool ret = false; + + source_address((struct sock_common *)sk, &saddr); + + spin_lock_bh(&msk->pm.lock); + list_for_each_entry(entry, &msk->pm.anno_list, list) { + if (addresses_equal(&entry->addr, &saddr, true)) { + ret = true; + goto out; + } + } + +out: + spin_unlock_bh(&msk->pm.lock); + return ret; +} + static void mptcp_pm_add_timer(struct timer_list *timer) { struct mptcp_pm_add_entry *entry = from_timer(entry, timer, add_timer); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index ec179f3a6b4b..014acecae649 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -554,6 +554,7 @@ void mptcp_pm_add_addr_received(struct mptcp_sock *msk, void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk); void mptcp_pm_rm_addr_received(struct mptcp_sock *msk, u8 rm_id); 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); struct mptcp_pm_add_entry * mptcp_pm_del_add_timer(struct mptcp_sock *msk, struct mptcp_addr_info *addr); diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 89fe7c1744d5..7da6a00a35a6 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -184,6 +184,14 @@ static void subflow_init_req(struct request_sock *req, pr_debug("syn inet_sport=%d %d", ntohs(inet_sk(sk_listener)->inet_sport), ntohs(inet_sk((struct sock *)subflow_req->msk)->inet_sport)); + if (!mptcp_pm_sport_in_anno_list(subflow_req->msk, sk_listener)) { + sock_put((struct sock *)subflow_req->msk); + mptcp_token_destroy_request(req); + tcp_request_sock_ops.destructor(req); + subflow_req->msk = NULL; + subflow_req->mp_join = 0; + return; + } } if (unlikely(req->syncookie) && subflow_req->msk) { @@ -642,6 +650,8 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, pr_debug("ack inet_sport=%d %d", ntohs(inet_sk(sk)->inet_sport), ntohs(inet_sk((struct sock *)owner)->inet_sport)); + if (!mptcp_pm_sport_in_anno_list(owner, sk)) + goto out; } } } -- 2.26.2

This patch added a new argument for pm_nl_ctl tool. We can use it like this: # pm_nl_ctl add 10.0.2.1 flags signal port 10100 # pm_nl_ctl dump id 1 flags signal 10.0.2.1 10100 Signed-off-by: Geliang Tang <geliangtang(a)gmail.com&gt; --- tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c index b24a2f17d415..cf654a57552b 100644 --- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c +++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c @@ -176,8 +176,8 @@ int add_addr(int fd, int pm_family, int argc, char *argv[]) 1024]; struct rtattr *rta, *nest; struct nlmsghdr *nh; + u_int32_t flags = 0; u_int16_t family; - u_int32_t flags; int nest_start; u_int8_t id; int off = 0; @@ -223,7 +223,6 @@ int add_addr(int fd, int pm_family, int argc, char *argv[]) char *tok, *str; /* flags */ - flags = 0; if (++arg >= argc) error(1, 0, " missing flags value"); @@ -271,6 +270,20 @@ int add_addr(int fd, int pm_family, int argc, char *argv[]) rta->rta_len = RTA_LENGTH(4); memcpy(RTA_DATA(rta), &ifindex, 4); off += NLMSG_ALIGN(rta->rta_len); + } else if (!strcmp(argv[arg], "port")) { + u_int16_t port; + + if (++arg >= argc) + error(1, 0, " missing port value"); + if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) + error(1, 0, " flags must be signal when using port"); + + port = atoi(argv[arg]); + rta = (void *)(data + off); + rta->rta_type = MPTCP_PM_ADDR_ATTR_PORT; + rta->rta_len = RTA_LENGTH(2); + memcpy(RTA_DATA(rta), &port, 2); + off += NLMSG_ALIGN(rta->rta_len); } else error(1, 0, "unknown keyword %s", argv[arg]); } @@ -323,6 +336,7 @@ int del_addr(int fd, int pm_family, int argc, char *argv[]) static void print_addr(struct rtattr *attrs, int len) { uint16_t family = 0; + uint16_t port = 0; char str[1024]; uint32_t flags; uint8_t id; @@ -330,12 +344,16 @@ static void print_addr(struct rtattr *attrs, int len) while (RTA_OK(attrs, len)) { if (attrs->rta_type == MPTCP_PM_ADDR_ATTR_FAMILY) memcpy(&family, RTA_DATA(attrs), 2); + if (attrs->rta_type == MPTCP_PM_ADDR_ATTR_PORT) + memcpy(&port, RTA_DATA(attrs), 2); if (attrs->rta_type == MPTCP_PM_ADDR_ATTR_ADDR4) { if (family != AF_INET) error(1, errno, "wrong IP (v4) for family %d", family); inet_ntop(AF_INET, RTA_DATA(attrs), str, sizeof(str)); printf("%s", str); + if (port) + printf(" %d", port); } if (attrs->rta_type == MPTCP_PM_ADDR_ATTR_ADDR6) { if (family != AF_INET6) @@ -343,6 +361,8 @@ static void print_addr(struct rtattr *attrs, int len) family); inet_ntop(AF_INET6, RTA_DATA(attrs), str, sizeof(str)); printf("%s", str); + if (port) + printf(" %d", port); } if (attrs->rta_type == MPTCP_PM_ADDR_ATTR_ID) { memcpy(&id, RTA_DATA(attrs), 1); -- 2.26.2

This patch added testcases for ADD_ADDR with port and the related MIB counters check in chk_add_nr. The output looks like this: 24 signal address with port syn[ ok ] - synack[ ok ] - ack[ ok ] add[ ok ] - echo [ ok ] - pt [ ok ] syn[ ok ] - synack[ ok ] - ack[ ok ] syn[ ok ] - ack [ ok ] 25 subflow and signal with port syn[ ok ] - synack[ ok ] - ack[ ok ] add[ ok ] - echo [ ok ] - pt [ ok ] syn[ ok ] - synack[ ok ] - ack[ ok ] syn[ ok ] - ack [ ok ] 26 remove single address with port syn[ ok ] - synack[ ok ] - ack[ ok ] add[ ok ] - echo [ ok ] - pt [ ok ] syn[ ok ] - synack[ ok ] - ack[ ok ] syn[ ok ] - ack [ ok ] rm [ ok ] - sf [ ok ] Signed-off-by: Geliang Tang <geliangtang(a)gmail.com&gt; --- .../testing/selftests/net/mptcp/mptcp_join.sh | 148 +++++++++++++++++- 1 file changed, 147 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 0eae628d1ffd..38d756f5e366 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -429,6 +429,12 @@ chk_add_nr() { local add_nr=$1 local echo_nr=$2 + local port_nr=${3:-0} + local syn_nr=${4:-$port_nr} + local syn_ack_nr=${5:-$port_nr} + local ack_nr=${6:-$port_nr} + local mis_syn_nr=${7:-0} + local mis_ack_nr=${8:-0} local count local dump_stats @@ -451,7 +457,87 @@ chk_add_nr() ret=1 dump_stats=1 else - echo "[ ok ]" + echo -n "[ ok ]" + fi + + if [ $port_nr -gt 0 ]; then + echo -n " - pt " + count=`ip netns exec $ns2 nstat -as | grep MPTcpExtPortAdd | awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$port_nr" ]; then + echo "[fail] got $count ADD_ADDR[s] with a port-number expected $port_nr" + ret=1 + dump_stats=1 + else + echo "[ ok ]" + fi + + printf "%-39s %s" " " "syn" + count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMPJoinPortSynRx | + awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$syn_nr" ]; then + echo "[fail] got $count JOIN[s] syn with a different \ + port-number expected $syn_nr" + ret=1 + dump_stats=1 + else + echo -n "[ ok ]" + fi + + echo -n " - synack" + count=`ip netns exec $ns2 nstat -as | grep MPTcpExtMPJoinPortSynAckRx | + awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$syn_ack_nr" ]; then + echo "[fail] got $count JOIN[s] synack with a different \ + port-number expected $syn_ack_nr" + ret=1 + dump_stats=1 + else + echo -n "[ ok ]" + fi + + echo -n " - ack" + count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMPJoinPortAckRx | + awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$ack_nr" ]; then + echo "[fail] got $count JOIN[s] ack with a different \ + port-number expected $ack_nr" + ret=1 + dump_stats=1 + else + echo "[ ok ]" + fi + + printf "%-39s %s" " " "syn" + count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMismatchPortSynRx | + awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$mis_syn_nr" ]; then + echo "[fail] got $count JOIN[s] syn with a mismatched \ + port-number expected $mis_syn_nr" + ret=1 + dump_stats=1 + else + echo -n "[ ok ]" + fi + + echo -n " - ack " + count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMismatchPortAckRx | + awk '{print $2}'` + [ -z "$count" ] && count=0 + if [ "$count" != "$mis_ack_nr" ]; then + echo "[fail] got $count JOIN[s] ack with a mismatched \ + port-number expected $mis_ack_nr" + ret=1 + dump_stats=1 + else + echo "[ ok ]" + fi + else + echo "" fi if [ "${dump_stats}" = 1 ]; then @@ -717,6 +803,66 @@ chk_join_nr "remove subflow and signal IPv6" 2 2 2 chk_add_nr 1 1 chk_rm_nr 1 1 +# signal address with port +reset +ip netns exec $ns1 ./pm_nl_ctl limits 0 1 +ip netns exec $ns2 ./pm_nl_ctl limits 1 1 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal port 10100 +run_tests $ns1 $ns2 10.0.1.1 +chk_join_nr "signal address with port" 1 1 1 +chk_add_nr 1 1 1 + +# subflow and signal with port +reset +ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal port 10100 +ip netns exec $ns1 ./pm_nl_ctl limits 0 2 +ip netns exec $ns2 ./pm_nl_ctl limits 1 2 +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 "subflow and signal with port" 2 2 2 +chk_add_nr 1 1 1 + +# single address with port, remove +reset +ip netns exec $ns1 ./pm_nl_ctl limits 0 1 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal port 10100 +ip netns exec $ns2 ./pm_nl_ctl limits 1 1 +run_tests $ns1 $ns2 10.0.1.1 0 1 0 slow +chk_join_nr "remove single address with port" 1 1 1 +chk_add_nr 1 1 1 +chk_rm_nr 0 0 + +# subflow and signal with port, remove +reset +ip netns exec $ns1 ./pm_nl_ctl limits 0 2 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal port 10100 +ip netns exec $ns2 ./pm_nl_ctl limits 1 2 +ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow +run_tests $ns1 $ns2 10.0.1.1 0 1 1 slow +chk_join_nr "remove subflow and signal with port" 2 2 2 +chk_add_nr 1 1 1 +chk_rm_nr 1 1 + +# multiple addresses with port +reset +ip netns exec $ns1 ./pm_nl_ctl limits 2 2 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal port 10100 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.3.1 flags signal port 10100 +ip netns exec $ns2 ./pm_nl_ctl limits 2 2 +run_tests $ns1 $ns2 10.0.1.1 +chk_join_nr "multiple addresses with port" 2 2 2 +chk_add_nr 2 2 2 + +# multiple addresses with ports +reset +ip netns exec $ns1 ./pm_nl_ctl limits 2 2 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal port 10100 +ip netns exec $ns1 ./pm_nl_ctl add 10.0.3.1 flags signal port 10101 +ip netns exec $ns2 ./pm_nl_ctl limits 2 2 +run_tests $ns1 $ns2 10.0.1.1 +chk_join_nr "multiple addresses with ports" 2 2 2 +chk_add_nr 2 2 2 + # single subflow, syncookies reset_with_cookies ip netns exec $ns1 ./pm_nl_ctl limits 0 1 -- 2.26.2

Hi Paolo, Mat, On Thu, Dec 03, 2020 at 05:36:08PM -0800, Mat Martineau wrote: I need some help about releasing the MPTCP type listening socket. When I use "sock_release(entry->lsk)" to release it, I'll get a deadlock warning like this: ---- [ 55.789592] ============================================ [ 55.789593] WARNING: possible recursive locking detected [ 55.789594] 5.10.0-rc6-mptcp+ #742 Not tainted [ 55.789595] -------------------------------------------- [ 55.789596] pm_nl_ctl/5583 is trying to acquire lock: [ 55.789597] ffff9ff9883cb960 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: __mptcp_close_ssk+0x52/0x160 [ 55.789604] but task is already holding lock: [ 55.789605] ffff9ff949c1c1a0 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x45/0x320 [ 55.789608] other info that might help us debug this: [ 55.789609] Possible unsafe locking scenario: [ 55.789610] CPU0 [ 55.789610] ---- [ 55.789611] lock(k-sk_lock-AF_INET); [ 55.789613] lock(k-sk_lock-AF_INET); [ 55.789614] *** DEADLOCK *** [ 55.789615] May be due to missing lock nesting notation [ 55.789616] 3 locks held by pm_nl_ctl/5583: [ 55.789617] #0: ffffffff8c5f9af0 (cb_lock){++++}-{3:3}, at: genl_rcv+0x15/0x40 [ 55.789621] #1: ffffffff8c5f9b88 (genl_mutex){+.+.}-{3:3}, at: genl_rcv_msg+0xf5/0x1c0 [ 55.789625] #2: ffff9ff949c1c1a0 (k-sk_lock-AF_INET){+.+.}-{0:0}, at: mptcp_close+0x45/0x320 [ 55.789629] stack backtrace: [ 55.789631] CPU: 1 PID: 5583 Comm: pm_nl_ctl Kdump: loaded Not tainted 5.10.0-rc6-mptcp+ #742 [ 55.789632] Hardware name: TIMI Mi Laptop Pro 15/TM1905, BIOS XMACM500P0301 04/08/2020 [ 55.789633] Call Trace: [ 55.789637] dump_stack+0x8b/0xb0 [ 55.789639] __lock_acquire.cold+0x159/0x2ab [ 55.789643] ? debug_object_assert_init+0x4b/0x130 [ 55.789646] lock_acquire+0x116/0x370 [ 55.789648] ? __mptcp_close_ssk+0x52/0x160 [ 55.789651] ? lock_sock_nested+0x51/0x90 [ 55.789653] lock_sock_nested+0x70/0x90 [ 55.789655] ? __mptcp_close_ssk+0x52/0x160 [ 55.789657] __mptcp_close_ssk+0x52/0x160 [ 55.789659] __mptcp_destroy_sock+0x119/0x210 [ 55.789661] mptcp_close+0x281/0x320 [ 55.789663] inet_release+0x99/0xa8 [ 55.789665] sock_release+0x20/0x70 [ 55.789667] mptcp_nl_cmd_add_addr+0x27c/0x2e0 [ 55.789670] genl_family_rcv_msg_doit+0xcd/0x110 [ 55.789675] genl_rcv_msg+0xce/0x1c0 [ 55.789677] ? mptcp_nl_cmd_get_limits+0x260/0x260 [ 55.789680] ? genl_get_cmd+0xd0/0xd0 [ 55.789683] netlink_rcv_skb+0x50/0xf0 [ 55.789687] genl_rcv+0x24/0x40 [ 55.789688] netlink_unicast+0x16d/0x230 [ 55.789690] netlink_sendmsg+0x23f/0x460 [ 55.789693] sock_sendmsg+0x5e/0x60 [ 55.789694] __sys_sendto+0xf1/0x160 [ 55.789698] ? do_user_addr_fault+0x215/0x440 [ 55.789701] ? lockdep_hardirqs_on_prepare+0xff/0x180 [ 55.789702] __x64_sys_sendto+0x25/0x30 [ 55.789704] do_syscall_64+0x33/0x40 [ 55.789707] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 55.789709] RIP: 0033:0x7fca52863efa [ 55.789710] Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 76 c3 0f 1f 44 00 00 55 48 83 ec 30 44 89 4c [ 55.789712] RSP: 002b:00007ffc6a45db88 EFLAGS: 00000246 ORIG_RAX: 000000000000002c [ 55.789714] RAX: ffffffffffffffda RBX: 00007ffc6a45dbf0 RCX: 00007fca52863efa [ 55.789715] RDX: 0000000000000038 RSI: 00007ffc6a45dbf0 RDI: 0000000000000003 [ 55.789716] RBP: 0000000000000038 R08: 00007ffc6a45db94 R09: 000000000000000c [ 55.789717] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 [ 55.789718] R13: 0000000000000003 R14: 00007ffc6a45e170 R15: 00007ffc6a45e138 [ 55.789751] MPTCP: msk=000000001cb8c5f2 [ 55.798357] MPTCP: subflow=0000000008e7e757 ---- I spent a few days trying to solve this problem, but it didn't go well. Please give some suggestions about it, thanks very much. -Geliang > > + return err; > > +} > > + > > static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info) > > { > > struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR]; > > @@ -750,9 +798,19 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info) > > } > > > > *entry = addr; > > + if (entry->addr.port) { > > + ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry); > > + if (ret) { > > + GENL_SET_ERR_MSG(info, "create listen socket error"); > > + kfree(entry); > > + return ret; > > + } > > + } > > ret = mptcp_pm_nl_append_new_local_addr(pernet, entry); > > if (ret < 0) { > > GENL_SET_ERR_MSG(info, "too many addresses or duplicate one"); > > + if (entry->lsk) > > + sock_release(entry->lsk); > > kfree(entry); > > return ret; > > } > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > > index 4c36969873b9..5e464dfc0f6f 100644 > > --- a/net/mptcp/protocol.c > > +++ b/net/mptcp/protocol.c > > @@ -49,7 +49,7 @@ static void __mptcp_check_send_data_fin(struct sock *sk); > > * completed yet or has failed, return the subflow socket. > > * Otherwise return NULL. > > */ > > -static struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk) > > +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk) > > { > > if (!msk->subflow || READ_ONCE(msk->can_ack)) > > return NULL; > > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h > > index 9d8f01aac91c..ec179f3a6b4b 100644 > > --- a/net/mptcp/protocol.h > > +++ b/net/mptcp/protocol.h > > @@ -466,11 +466,14 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how); > > void __mptcp_close_ssk(struct sock *sk, struct sock *ssk, > > struct mptcp_subflow_context *subflow); > > void mptcp_subflow_reset(struct sock *ssk); > > +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk); > > > > /* called with sk socket lock held */ > > int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, > > const struct mptcp_addr_info *remote); > > int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock); > > +void mptcp_info2sockaddr(const struct mptcp_addr_info *info, > > + struct sockaddr_storage *addr); > > > > static inline void mptcp_subflow_tcp_fallback(struct sock *sk, > > struct mptcp_subflow_context *ctx) > > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c > > index 96c585f003f8..43cc5e2c3234 100644 > > --- a/net/mptcp/subflow.c > > +++ b/net/mptcp/subflow.c > > @@ -1035,8 +1035,8 @@ void mptcpv6_handle_mapped(struct sock *sk, bool mapped) > > } > > #endif > > > > -static void mptcp_info2sockaddr(const struct mptcp_addr_info *info, > > - struct sockaddr_storage *addr) > > +void mptcp_info2sockaddr(const struct mptcp_addr_info *info, > > + struct sockaddr_storage *addr) > > { > > memset(addr, 0, sizeof(*addr)); > > addr->ss_family = info->family; > > -- > > 2.26.2 > > -- > Mat Martineau > Intel

On Mon, 2020-12-07 at 14:30 +0800, Geliang Tang wrote: Uhm... this lock warning is quite strange. We already hit that lock sequence in several others places, with no splat. The lock sequence per se is safe, as the lock is for different 'struct sock' I'm wondering if you are get any others eariler warning, fooling lockdepth ?!? Thanks, Paolo

On Wed, Dec 09, 2020 at 07:24:42PM +0800, Geliang Tang wrote: Hi Paolo, The full log and the patch is attached. Apply this patch and run mptcp_join.sh can reproduce the warning. Thanks. -Geliang

Hi Paolo, On Wed, Dec 09, 2020 at 04:25:33PM +0100, Paolo Abeni wrote: Thanks for your help. v7 should be added the following code in pm_netlink.c to invoke sock_release to reproduce the issue: @@ -885,6 +968,8 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info) spin_unlock_bh(&pernet->lock); mptcp_nl_remove_subflow_and_signal_addr(sock_net(skb->sk), &entry->addr); + if (entry->lsk) + sock_release(entry->lsk); kfree_rcu(entry, rcu); return ret; Sorry, I forget the kconfig. I attached it in this letter. -Geliang

Hi Paolo, Paolo Abeni <pabeni(a)redhat.com&gt; 于2020年12月11日周五 下午11:21写道： Thanks for your help. I tested this patch, it works well. Please send this patch to the ML and add my reported-and-tested tag in it: Reported-and-tested-by: Geliang Tang <geliangtang(a)gmail.com&gt; Thanks very much. -Geliang > /P > >