* [PATCH v3 0/4] Fix some mptcp syncookie process bugs
@ 2021-06-11 7:34 wujianguo106
2021-06-11 7:34 ` [PATCH 1/4] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join wujianguo106
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: wujianguo106 @ 2021-06-11 7:34 UTC (permalink / raw)
To: mptcp; +Cc: pabeni
From: Jianguo Wu <wujianguo@chinatelecom.cn>
v1->v2:
patch1: handle ipv6 sockets/addresses,
always use 4-tuple drived hash and never look at skb->hash
patch2: no changes
patch3: split into 2 patches.
patch4: new added.
v2->v3:
patch1: directly use inet6_ehashfn() for IPv6
patch2: no changes
patch3: no changes
patch4: add Fixes tag.
Jianguo Wu (4):
mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join
mptcp: remove redundant req destruct in subflow_check_req()
mptcp: fix syncookie process if mptcp can not_accept new subflow
mptcp: avoid processing packet if a subflow reset
net/mptcp/options.c | 2 ++
net/mptcp/subflow.c | 11 +++--------
net/mptcp/syncookies.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 47 insertions(+), 12 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/4] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join
2021-06-11 7:34 [PATCH v3 0/4] Fix some mptcp syncookie process bugs wujianguo106
@ 2021-06-11 7:34 ` wujianguo106
2021-06-11 7:34 ` [PATCH 2/4] mptcp: remove redundant req destruct in subflow_check_req() wujianguo106
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: wujianguo106 @ 2021-06-11 7:34 UTC (permalink / raw)
To: mptcp; +Cc: pabeni
From: Jianguo Wu <wujianguo@chinatelecom.cn>
I got the following warning message while doing the test:
[ 55.552626] TCP: request_sock_subflow: Possible SYN flooding on port 8099. Sending cookies. Check SNMP counters.
[ 55.553024] ------------[ cut here ]------------
[ 55.553027] WARNING: CPU: 0 PID: 10 at net/core/flow_dissector.c:984 __skb_flow_dissect+0x280/0x1650
...
[ 55.553117] CPU: 0 PID: 10 Comm: ksoftirqd/0 Not tainted 5.12.0+ #18
[ 55.553121] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020
[ 55.553124] RIP: 0010:__skb_flow_dissect+0x280/0x1650
...
[ 55.553133] RSP: 0018:ffffb79580087770 EFLAGS: 00010246
[ 55.553137] RAX: 0000000000000000 RBX: ffffffff8ddb58e0 RCX: ffffb79580087888
[ 55.553139] RDX: ffffffff8ddb58e0 RSI: ffff8f7e4652b600 RDI: 0000000000000000
[ 55.553141] RBP: ffffb79580087858 R08: 0000000000000000 R09: 0000000000000008
[ 55.553143] R10: 000000008c622965 R11: 00000000d3313a5b R12: ffff8f7e4652b600
[ 55.553146] R13: ffff8f7e465c9062 R14: 0000000000000000 R15: ffffb79580087888
[ 55.553149] FS: 0000000000000000(0000) GS:ffff8f7f75e00000(0000) knlGS:0000000000000000
[ 55.553152] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 55.553154] CR2: 00007f73d1d19000 CR3: 0000000135e10004 CR4: 00000000003706f0
[ 55.553160] Call Trace:
[ 55.553166] ? __sha256_final+0x67/0xd0
[ 55.553173] ? sha256+0x7e/0xa0
[ 55.553177] __skb_get_hash+0x57/0x210
[ 55.553182] subflow_init_req_cookie_join_save+0xac/0xc0
[ 55.553189] subflow_check_req+0x474/0x550
[ 55.553195] ? ip_route_output_key_hash+0x67/0x90
[ 55.553200] ? xfrm_lookup_route+0x1d/0xa0
[ 55.553207] subflow_v4_route_req+0x8e/0xd0
[ 55.553212] tcp_conn_request+0x31e/0xab0
[ 55.553218] ? selinux_socket_sock_rcv_skb+0x116/0x210
[ 55.553224] ? tcp_rcv_state_process+0x179/0x6d0
[ 55.553229] tcp_rcv_state_process+0x179/0x6d0
[ 55.553235] tcp_v4_do_rcv+0xaf/0x220
[ 55.553239] tcp_v4_rcv+0xce4/0xd80
[ 55.553243] ? ip_route_input_rcu+0x246/0x260
[ 55.553248] ip_protocol_deliver_rcu+0x35/0x1b0
[ 55.553253] ip_local_deliver_finish+0x44/0x50
[ 55.553258] ip_local_deliver+0x6c/0x110
[ 55.553262] ? ip_rcv_finish_core.isra.19+0x5a/0x400
[ 55.553267] ip_rcv+0xd1/0xe0
...
After debugging, I found in __skb_flow_dissect(), skb->dev and skb->sk are both NULL,
then net is NULL, and trigger WARN_ON_ONCE(!net), actually net is always NULL in this
code path, as skb->dev is set to NULL in tcp_v4_rcv(), and skb->sk is never set.
Code snippet in __skb_flow_dissect() that trigger warning:
975 if (skb) {
976 if (!net) {
977 if (skb->dev)
978 net = dev_net(skb->dev);
979 else if (skb->sk)
980 net = sock_net(skb->sk);
981 }
982 }
983
984 WARN_ON_ONCE(!net);
So, use 4-tuple derived hash.
Fixes: 9466a1ccebbe("mptcp: enable JOIN requests even if cookies are in use").
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/syncookies.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
index abe0fd0..2ba09a7 100644
--- a/net/mptcp/syncookies.c
+++ b/net/mptcp/syncookies.c
@@ -35,13 +35,51 @@ struct join_entry {
static struct join_entry join_entries[COOKIE_JOIN_SLOTS] __cacheline_aligned_in_smp;
static spinlock_t join_entry_locks[COOKIE_JOIN_SLOTS] __cacheline_aligned_in_smp;
-static u32 mptcp_join_entry_hash(struct sk_buff *skb, struct net *net)
+static u32 mptcp_join_hashfn(const struct net *net, const __be32 laddr,
+ const __be16 lport, const __be32 faddr,
+ const __be16 fport)
{
- u32 i = skb_get_hash(skb) ^ net_hash_mix(net);
+ static u32 mptcp_join_hash_secret __read_mostly;
+ u32 i;
+
+ net_get_random_once(&mptcp_join_hash_secret,
+ sizeof(mptcp_join_hash_secret));
+
+ i = jhash_3words((__force __u32)laddr,
+ (__force __u32)faddr,
+ ((__u32)lport) << 16 | (__force __u32)fport,
+ mptcp_join_hash_secret + net_hash_mix(net));
return i % ARRAY_SIZE(join_entries);
}
+static u32 mptcp_join_hashfn_inet6(const struct net *net,
+ const struct in6_addr *laddr, const __be16 lport,
+ const struct in6_addr *faddr, const __be16 fport)
+{
+ u32 i;
+
+ i = inet6_ehashfn(net, laddr, (__force __u16)lport, faddr, fport);
+
+ return i % ARRAY_SIZE(join_entries);
+}
+
+static u32 mptcp_join_entry_hash(struct sk_buff *skb, struct net *net,
+ unsigned short family)
+{
+ struct tcphdr *th = tcp_hdr(skb);
+
+#if IS_ENABLED(CONFIG_IPV6)
+ if (family == AF_INET6 &&
+ !ipv6_addr_v4mapped(&ipv6_hdr(skb)->saddr))
+ return mptcp_join_hashfn_inet6(net,
+ &ipv6_hdr(skb)->daddr, th->dest,
+ &ipv6_hdr(skb)->saddr, th->source);
+#endif
+ return mptcp_join_hashfn(net, ip_hdr(skb)->daddr, th->dest,
+ ip_hdr(skb)->saddr, th->source);
+}
+
static void mptcp_join_store_state(struct join_entry *entry,
const struct mptcp_subflow_request_sock *subflow_req)
{
@@ -58,7 +96,7 @@ void subflow_init_req_cookie_join_save(const struct mptcp_subflow_request_sock *
struct sk_buff *skb)
{
struct net *net = read_pnet(&subflow_req->sk.req.ireq_net);
- u32 i = mptcp_join_entry_hash(skb, net);
+ u32 i = mptcp_join_entry_hash(skb, net, subflow_req->sk.req.ireq_family);
/* No use in waiting if other cpu is already using this slot --
* would overwrite the data that got stored.
@@ -79,7 +117,7 @@ bool mptcp_token_join_cookie_init_state(struct mptcp_subflow_request_sock *subfl
struct sk_buff *skb)
{
struct net *net = read_pnet(&subflow_req->sk.req.ireq_net);
- u32 i = mptcp_join_entry_hash(skb, net);
+ u32 i = mptcp_join_entry_hash(skb, net, subflow_req->sk.req.ireq_family);
struct mptcp_sock *msk;
struct join_entry *e;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] mptcp: remove redundant req destruct in subflow_check_req()
2021-06-11 7:34 [PATCH v3 0/4] Fix some mptcp syncookie process bugs wujianguo106
2021-06-11 7:34 ` [PATCH 1/4] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join wujianguo106
@ 2021-06-11 7:34 ` wujianguo106
2021-06-11 7:34 ` [PATCH 3/4] mptcp: fix syncookie process if mptcp can not_accept new subflow wujianguo106
2021-06-11 7:34 ` [PATCH 4/4] mptcp: avoid processing packet if a subflow reset wujianguo106
3 siblings, 0 replies; 5+ messages in thread
From: wujianguo106 @ 2021-06-11 7:34 UTC (permalink / raw)
To: mptcp; +Cc: pabeni
From: Jianguo Wu <wujianguo@chinatelecom.cn>
In subflow_check_req(), if subflow sport is mismatch, will put msk,
destroy token, and destruct req, then return -EPERM, which can be
done by subflow_req_destructor() via:
tcp_conn_request()
|--__reqsk_free()
|--subflow_req_destructor()
So we should remove these redundant code, otherwise will call
tcp_v4_reqsk_destructor() twice, and may double free inet_rsk(req)->ireq_opt.
Fixes: 5bc56388c74f ("mptcp: add port number check for MP_JOIN")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/subflow.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 6b1cd42..75ed530 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -213,11 +213,6 @@ static int subflow_check_req(struct request_sock *req,
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;
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTSYNRX);
return -EPERM;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] mptcp: fix syncookie process if mptcp can not_accept new subflow
2021-06-11 7:34 [PATCH v3 0/4] Fix some mptcp syncookie process bugs wujianguo106
2021-06-11 7:34 ` [PATCH 1/4] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join wujianguo106
2021-06-11 7:34 ` [PATCH 2/4] mptcp: remove redundant req destruct in subflow_check_req() wujianguo106
@ 2021-06-11 7:34 ` wujianguo106
2021-06-11 7:34 ` [PATCH 4/4] mptcp: avoid processing packet if a subflow reset wujianguo106
3 siblings, 0 replies; 5+ messages in thread
From: wujianguo106 @ 2021-06-11 7:34 UTC (permalink / raw)
To: mptcp; +Cc: pabeni
From: Jianguo Wu <wujianguo@chinatelecom.cn>
Lots of "TCP: tcp_fin: Impossible, sk->sk_state=7" in client side
when doing stress testing.
There are at least two cases may trigger this warning:
1.mptcp is in syncookie, and server recv MP_JOIN SYN request,
in subflow_check_req(), the mptcp_can_accept_new_subflow()
return false, so subflow_init_req_cookie_join_save() isn't
called, i.e. not store the data present in the MP_JOIN syn
request and the random nonce in hash table - join_entries[],
but still send synack. When recv 3rd-ack,
mptcp_token_join_cookie_init_state() will return false, and
3rd-ack is dropped, then if mptcp conn is closed by client,
client will send a DATA_FIN and a MPTCP FIN, the DATA_FIN
doesn't have MP_CAPABLE or MP_JOIN,
so mptcp_subflow_init_cookie_req() will return 0, and pass
the cookie check, MP_JOIN request is fallback to normal TCP.
Server will send a TCP FIN if closed, in client side,
when process TCP FIN, it will do reset, the code path is:
tcp_data_queue()->mptcp_incoming_options()
->check_fully_established()->mptcp_subflow_reset().
mptcp_subflow_reset() will set sock state to TCP_CLOSE,
so tcp_fin will hit TCP_CLOSE, and print the warning.
2.mptcp is in syncookie, and server recv 3rd-ack, in
mptcp_subflow_init_cookie_req(), mptcp_can_accept_new_subflow()
return false, and subflow_req->mp_join is not set to 1,
so in subflow_syn_recv_sock() will not reset the MP_JOIN
subflow, but fallback to normal TCP, and then the same thing
happens when server will send a TCP FIN if closed.
For case1, subflow_check_req() return -EPERM,
then tcp_conn_request() will drop MP_JOIN SYN.
For case2, let subflow_syn_recv_sock() call mptcp_can_accept_new_subflow(),
and do fatal fallback, send reset.
Fixes: 9466a1ccebbe("mptcp: enable JOIN requests even if cookies are in use")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/subflow.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 75ed530..6d98e19 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -224,6 +224,8 @@ static int subflow_check_req(struct request_sock *req,
if (unlikely(req->syncookie)) {
if (mptcp_can_accept_new_subflow(subflow_req->msk))
subflow_init_req_cookie_join_save(subflow_req, skb);
+ else
+ return -EPERM;
}
pr_debug("token=%u, remote_nonce=%u msk=%p", subflow_req->token,
@@ -263,9 +265,7 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
if (!mptcp_token_join_cookie_init_state(subflow_req, skb))
return -EINVAL;
- if (mptcp_can_accept_new_subflow(subflow_req->msk))
- subflow_req->mp_join = 1;
-
+ subflow_req->mp_join = 1;
subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq - 1;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] mptcp: avoid processing packet if a subflow reset
2021-06-11 7:34 [PATCH v3 0/4] Fix some mptcp syncookie process bugs wujianguo106
` (2 preceding siblings ...)
2021-06-11 7:34 ` [PATCH 3/4] mptcp: fix syncookie process if mptcp can not_accept new subflow wujianguo106
@ 2021-06-11 7:34 ` wujianguo106
3 siblings, 0 replies; 5+ messages in thread
From: wujianguo106 @ 2021-06-11 7:34 UTC (permalink / raw)
To: mptcp; +Cc: pabeni
From: Jianguo Wu <wujianguo@chinatelecom.cn>
If check_fully_established() causes a subflow reset, it should not
continue to process the packet in tcp_data_queue().
setting:
TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq;
so that the following check will drop the pkt in
tcp_data_queue():
if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
__kfree_skb(skb);
return;
}
Fixes: d582484726c4 ("mptcp: fix fallback for MP_JOIN subflows")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/options.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 1aec016..01a1bf6 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -926,6 +926,8 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
return true;
reset:
+ TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq;
+
mptcp_subflow_reset(ssk);
return false;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-11 7:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 7:34 [PATCH v3 0/4] Fix some mptcp syncookie process bugs wujianguo106
2021-06-11 7:34 ` [PATCH 1/4] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join wujianguo106
2021-06-11 7:34 ` [PATCH 2/4] mptcp: remove redundant req destruct in subflow_check_req() wujianguo106
2021-06-11 7:34 ` [PATCH 3/4] mptcp: fix syncookie process if mptcp can not_accept new subflow wujianguo106
2021-06-11 7:34 ` [PATCH 4/4] mptcp: avoid processing packet if a subflow reset wujianguo106
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).