[PATCH mptcp-net v6 0/5] Fix some mptcp syncookie process bugs

[PATCH mptcp-net v6 0/5] Fix some mptcp syncookie process bugs

[wujianguo106]

From: Jianguo Wu <wujianguo@chinatelecom.cn>

v5->v6:
 - patch1: describe the testing tools in the commit log
 - patch4: add return value to mptcp_incoming_options(), return 0 if
	   a subflow has been reset, else return 1, and drop the packet
	   in tcp_data_queue()/tcp_rcv_state_process() if the return
	   value is 0
 - patch5: update selftest case "multiple subflows limited by server",
	   since the expected behavior is changed by patch4

v4->v5:
 - patch4: add comment 

v3->v4:
 - patch1: using seq and sport/dport for hashing, and ignore network
	   headers altogether, as suggest by Florian

v2->v3:
 - patch1: directly use inet6_ehashfn() for IPv6
 - patch4: add Fixes tag.

v1->v2:
 - patch1: handle ipv6 sockets/addresses,
	   always use 4-tuple drived hash and never look at skb->hash
 - patch3: split into 2 patches.
 - patch4: new added.

Jianguo Wu (5):
  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
  selftests: mptcp: fix case multiple subflows limited by server

 include/net/mptcp.h                             |  2 +-
 net/ipv4/tcp_input.c                            | 19 +++++++++++++++----
 net/mptcp/options.c                             | 22 ++++++++++++++--------
 net/mptcp/subflow.c                             | 11 +++--------
 net/mptcp/syncookies.c                          | 16 +++++++++++++++-
 tools/testing/selftests/net/mptcp/mptcp_join.sh |  2 +-
 6 files changed, 49 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[PATCH mptcp-net v6 1/5] mptcp: fix warning in __skb_flow_dissect() when do syn cookie for subflow join

[wujianguo106]

From: Jianguo Wu <wujianguo@chinatelecom.cn>

I did stress test with wrk(https://github.com/wg/wrk) and webfsd(https://github.com/ourway/webfsd)
with the assistance of mptcp-tools(https://github.com/pabeni/mptcp-tools):
  Server side:
      ./use_mptcp.sh webfsd -4 -R /tmp/ -p 8099
  Client side:
      ./use_mptcp.sh wrk -c 200 -d 30 -t 4 http://192.168.174.129:8099/

and got the following warning message:

[   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, using seq and transport header derived hash.

Fixes: 9466a1ccebbe("mptcp: enable JOIN requests even if cookies are in use").
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
 net/mptcp/syncookies.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
index abe0fd099746..37127781aee9 100644
--- a/net/mptcp/syncookies.c
+++ b/net/mptcp/syncookies.c
@@ -37,7 +37,21 @@ struct join_entry {
 
 static u32 mptcp_join_entry_hash(struct sk_buff *skb, struct net *net)
 {
-	u32 i = skb_get_hash(skb) ^ net_hash_mix(net);
+	static u32 mptcp_join_hash_secret __read_mostly;
+	struct tcphdr *th = tcp_hdr(skb);
+	u32 seq, i;
+
+	net_get_random_once(&mptcp_join_hash_secret,
+			    sizeof(mptcp_join_hash_secret));
+
+	if (th->syn)
+		seq = TCP_SKB_CB(skb)->seq;
+	else
+		seq = TCP_SKB_CB(skb)->seq - 1;
+
+	i = jhash_3words(seq, net_hash_mix(net),
+			 (__force __u32)th->source << 16 | (__force __u32)th->dest,
+			 mptcp_join_hash_secret);
 
 	return i % ARRAY_SIZE(join_entries);
 }
-- 
1.8.3.1

[PATCH mptcp-net v6 2/5] mptcp: remove redundant req destruct in subflow_check_req()

[wujianguo106]

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 6b1cd4257edf..75ed530706c0 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

[PATCH mptcp-net v6 3/5] mptcp: fix syncookie process if mptcp can not_accept new subflow

[wujianguo106]

From: Jianguo Wu <wujianguo@chinatelecom.cn>

Lots of "TCP: tcp_fin: Impossible, sk->sk_state=7" in client side
when doing stress testing using wrk and webfsd.

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 75ed530706c0..6d98e19a20aa 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

[PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset

[wujianguo106]

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().
Add a return value to mptcp_incoming_options(), and return 0 if a
subflow has been reset, else return 1. Then drop the packet in
tcp_data_queue()/tcp_rcv_state_process() if mptcp_incoming_options()
return 0.

Fixes: d582484726c4 ("mptcp: fix fallback for MP_JOIN subflows")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
 include/net/mptcp.h  |  2 +-
 net/ipv4/tcp_input.c | 19 +++++++++++++++----
 net/mptcp/options.c  | 22 ++++++++++++++--------
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index d61bbbf11979..b32333f95738 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -104,7 +104,7 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
 bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
 			       unsigned int *size, unsigned int remaining,
 			       struct mptcp_out_options *opts);
-void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb);
+int mptcp_incoming_options(struct sock *sk, struct sk_buff *skb);
 
 void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 			 struct mptcp_out_options *opts);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 7d5e59f688de..4bacd7d2abd7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4247,6 +4247,9 @@ void tcp_reset(struct sock *sk, struct sk_buff *skb)
 {
 	trace_tcp_receive_reset(sk);
 
+	/* mptcp can't tell us to ignore reset pkts,
+	 * so just ignore the return value of mptcp_incoming_options().
+	 */
 	if (sk_is_mptcp(sk))
 		mptcp_incoming_options(sk, skb);
 
@@ -4941,8 +4944,13 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
 	bool fragstolen;
 	int eaten;
 
-	if (sk_is_mptcp(sk))
-		mptcp_incoming_options(sk, skb);
+	/* If a subflow has been reset, the packet should not continue
+	 * to be processed, drop the packet.
+	 */
+	if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
+		__kfree_skb(skb);
+		return;
+	}
 
 	if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
 		__kfree_skb(skb);
@@ -6523,8 +6531,11 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 	case TCP_CLOSING:
 	case TCP_LAST_ACK:
 		if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
-			if (sk_is_mptcp(sk))
-				mptcp_incoming_options(sk, skb);
+			/* If a subflow has been reset, the packet should not
+			 * continue to be processed, drop the packet.
+			 */
+			if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb))
+				goto discard;
 			break;
 		}
 		fallthrough;
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 1aec01686c1a..8a386556a23c 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -847,7 +847,8 @@ bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
 static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 				    struct mptcp_subflow_context *subflow,
 				    struct sk_buff *skb,
-				    struct mptcp_options_received *mp_opt)
+				    struct mptcp_options_received *mp_opt,
+				    bool *subflow_is_rst)
 {
 	/* here we can process OoO, in-window pkts, only in-sequence 4th ack
 	 * will make the subflow fully established
@@ -926,6 +927,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 	return true;
 
 reset:
+	*subflow_is_rst = true;
 	mptcp_subflow_reset(ssk);
 	return false;
 }
@@ -1022,12 +1024,14 @@ static bool add_addr_hmac_valid(struct mptcp_sock *msk,
 	return hmac == mp_opt->ahmac;
 }
 
-void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
+/* Return 0 if a subflow has been reset, else return 1 */
+int mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
 	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
 	struct mptcp_options_received mp_opt;
 	struct mptcp_ext *mpext;
+	bool subflow_is_rst = false;
 
 	if (__mptcp_check_fallback(msk)) {
 		/* Keep it simple and unconditionally trigger send data cleanup and
@@ -1040,12 +1044,12 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 			__mptcp_check_push(subflow->conn, sk);
 		__mptcp_data_acked(subflow->conn);
 		mptcp_data_unlock(subflow->conn);
-		return;
+		return 1;
 	}
 
 	mptcp_get_options(sk, skb, &mp_opt);
-	if (!check_fully_established(msk, sk, subflow, skb, &mp_opt))
-		return;
+	if (!check_fully_established(msk, sk, subflow, skb, &mp_opt, &subflow_is_rst))
+		return subflow_is_rst ? 0 : 1;
 
 	if (mp_opt.fastclose &&
 	    msk->local_key == mp_opt.rcvr_key) {
@@ -1087,7 +1091,7 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 	}
 
 	if (!mp_opt.dss)
-		return;
+		return 1;
 
 	/* we can't wait for recvmsg() to update the ack_seq, otherwise
 	 * monodirectional flows will stuck
@@ -1106,12 +1110,12 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 		    schedule_work(&msk->work))
 			sock_hold(subflow->conn);
 
-		return;
+		return 1;
 	}
 
 	mpext = skb_ext_add(skb, SKB_EXT_MPTCP);
 	if (!mpext)
-		return;
+		return 1;
 
 	memset(mpext, 0, sizeof(*mpext));
 
@@ -1140,6 +1144,8 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 		if (mpext->csum_reqd)
 			mpext->csum = mp_opt.csum;
 	}
+
+	return 1;
 }
 
 static void mptcp_set_rwin(const struct tcp_sock *tp)
-- 
1.8.3.1

[PATCH mptcp-net v6 5/5] selftests: mptcp: update case multiple subflows limited by server

[wujianguo106]

From: Jianguo Wu <wujianguo@chinatelecom.cn>

After patch "mptcp: fix syncookie process if mptcp can not_accept new subflow",
If a new subflow is limited, MP_JOIN SYN is dropped, and no SYN/ACK will be replied.
So in case "multiple subflows limited by server", the expected SYN/ACK number
should be 1.

Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 523c7797f30a..37b7da3cd5ca 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1398,7 +1398,7 @@ syncookies_tests()
 	ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 	ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
 	run_tests $ns1 $ns2 10.0.1.1
-	chk_join_nr "subflows limited by server w cookies" 2 2 1
+	chk_join_nr "subflows limited by server w cookies" 2 1 1
 
 	# test signal address with cookies
 	reset_with_cookies
-- 
1.8.3.1

Re: [PATCH mptcp-net v6 5/5] selftests: mptcp: update case multiple subflows limited by server

[Jianguo Wu]

On 2021/6/25 10:48, wujianguo106@163.com wrote:
> From: Jianguo Wu <wujianguo@chinatelecom.cn>
> 
> After patch "mptcp: fix syncookie process if mptcp can not_accept new subflow",
> If a new subflow is limited, MP_JOIN SYN is dropped, and no SYN/ACK will be replied.
> So in case "multiple subflows limited by server", the expected SYN/ACK number
> should be 1.
> 

Sorry, forgot to add:
Reported-by: kernel test robot <oliver.sang@intel.com>

> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 523c7797f30a..37b7da3cd5ca 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1398,7 +1398,7 @@ syncookies_tests()
>  	ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
>  	ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
>  	run_tests $ns1 $ns2 10.0.1.1
> -	chk_join_nr "subflows limited by server w cookies" 2 2 1
> +	chk_join_nr "subflows limited by server w cookies" 2 1 1
>  
>  	# test signal address with cookies
>  	reset_with_cookies
> 

Re: [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5448 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mptcp/export]
[also build test ERROR on next-20210624]
[cannot apply to kselftest/next linus/master v5.13-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: i386-randconfig-a011-20210622 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/8bac278b64d6949fede4d0f383ca5f01fca11ef7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
        git checkout 8bac278b64d6949fede4d0f383ca5f01fca11ef7
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/ipv4/tcp_input.c: In function 'tcp_data_queue':
>> net/ipv4/tcp_input.c:4950:25: error: invalid use of void expression
    4950 |  if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
         |                         ^
   net/ipv4/tcp_input.c: In function 'tcp_rcv_state_process':
   net/ipv4/tcp_input.c:6537:27: error: invalid use of void expression
    6537 |    if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb))
         |                           ^


vim +4950 net/ipv4/tcp_input.c

  4940	
  4941	static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
  4942	{
  4943		struct tcp_sock *tp = tcp_sk(sk);
  4944		bool fragstolen;
  4945		int eaten;
  4946	
  4947		/* If a subflow has been reset, the packet should not continue
  4948		 * to be processed, drop the packet.
  4949		 */
> 4950		if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
  4951			__kfree_skb(skb);
  4952			return;
  4953		}
  4954	
  4955		if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
  4956			__kfree_skb(skb);
  4957			return;
  4958		}
  4959		skb_dst_drop(skb);
  4960		__skb_pull(skb, tcp_hdr(skb)->doff * 4);
  4961	
  4962		tp->rx_opt.dsack = 0;
  4963	
  4964		/*  Queue data for delivery to the user.
  4965		 *  Packets in sequence go to the receive queue.
  4966		 *  Out of sequence packets to the out_of_order_queue.
  4967		 */
  4968		if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
  4969			if (tcp_receive_window(tp) == 0) {
  4970				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
  4971				goto out_of_window;
  4972			}
  4973	
  4974			/* Ok. In sequence. In window. */
  4975	queue_and_out:
  4976			if (skb_queue_len(&sk->sk_receive_queue) == 0)
  4977				sk_forced_mem_schedule(sk, skb->truesize);
  4978			else if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
  4979				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
  4980				sk->sk_data_ready(sk);
  4981				goto drop;
  4982			}
  4983	
  4984			eaten = tcp_queue_rcv(sk, skb, &fragstolen);
  4985			if (skb->len)
  4986				tcp_event_data_recv(sk, skb);
  4987			if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
  4988				tcp_fin(sk);
  4989	
  4990			if (!RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
  4991				tcp_ofo_queue(sk);
  4992	
  4993				/* RFC5681. 4.2. SHOULD send immediate ACK, when
  4994				 * gap in queue is filled.
  4995				 */
  4996				if (RB_EMPTY_ROOT(&tp->out_of_order_queue))
  4997					inet_csk(sk)->icsk_ack.pending |= ICSK_ACK_NOW;
  4998			}
  4999	
  5000			if (tp->rx_opt.num_sacks)
  5001				tcp_sack_remove(tp);
  5002	
  5003			tcp_fast_path_check(sk);
  5004	
  5005			if (eaten > 0)
  5006				kfree_skb_partial(skb, fragstolen);
  5007			if (!sock_flag(sk, SOCK_DEAD))
  5008				tcp_data_ready(sk);
  5009			return;
  5010		}
  5011	
  5012		if (!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt)) {
  5013			tcp_rcv_spurious_retrans(sk, skb);
  5014			/* A retransmit, 2nd most common case.  Force an immediate ack. */
  5015			NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
  5016			tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
  5017	
  5018	out_of_window:
  5019			tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
  5020			inet_csk_schedule_ack(sk);
  5021	drop:
  5022			tcp_drop(sk, skb);
  5023			return;
  5024		}
  5025	
  5026		/* Out of window. F.e. zero window probe. */
  5027		if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
  5028			goto out_of_window;
  5029	
  5030		if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
  5031			/* Partial packet, seq < rcv_next < end_seq */
  5032			tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, tp->rcv_nxt);
  5033	
  5034			/* If window is closed, drop tail of packet. But after
  5035			 * remembering D-SACK for its head made in previous line.
  5036			 */
  5037			if (!tcp_receive_window(tp)) {
  5038				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
  5039				goto out_of_window;
  5040			}
  5041			goto queue_and_out;
  5042		}
  5043	
  5044		tcp_data_queue_ofo(sk, skb);
  5045	}
  5046	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47885 bytes --]

Re: [PATCH mptcp-net v6 4/5] mptcp: avoid processing packet if a subflow reset

[Jianguo Wu]

Sorry for the mistake, please ignore this series, I will send v7.

On 2021/6/25 14:40, kernel test robot wrote:
> Hi,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on mptcp/export]
> [also build test ERROR on next-20210624]
> [cannot apply to kselftest/next linus/master v5.13-rc7]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
> base:   https://github.com/multipath-tcp/mptcp_net-next.git export
> config: i386-randconfig-a011-20210622 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         # https://github.com/0day-ci/linux/commit/8bac278b64d6949fede4d0f383ca5f01fca11ef7
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review wujianguo106-163-com/Fix-some-mptcp-syncookie-process-bugs/20210625-110557
>         git checkout 8bac278b64d6949fede4d0f383ca5f01fca11ef7
>         # save the attached .config to linux build tree
>         make W=1 ARCH=i386 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    net/ipv4/tcp_input.c: In function 'tcp_data_queue':
>>> net/ipv4/tcp_input.c:4950:25: error: invalid use of void expression
>     4950 |  if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
>          |                         ^
>    net/ipv4/tcp_input.c: In function 'tcp_rcv_state_process':
>    net/ipv4/tcp_input.c:6537:27: error: invalid use of void expression
>     6537 |    if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb))
>          |                           ^
> 
> 
> vim +4950 net/ipv4/tcp_input.c
> 
>   4940	
>   4941	static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
>   4942	{
>   4943		struct tcp_sock *tp = tcp_sk(sk);
>   4944		bool fragstolen;
>   4945		int eaten;
>   4946	
>   4947		/* If a subflow has been reset, the packet should not continue
>   4948		 * to be processed, drop the packet.
>   4949		 */
>> 4950		if (sk_is_mptcp(sk) && !mptcp_incoming_options(sk, skb)) {
>   4951			__kfree_skb(skb);
>   4952			return;
>   4953		}
>   4954	
>   4955		if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
>   4956			__kfree_skb(skb);
>   4957			return;
>   4958		}
>   4959		skb_dst_drop(skb);
>   4960		__skb_pull(skb, tcp_hdr(skb)->doff * 4);
>   4961	
>   4962		tp->rx_opt.dsack = 0;
>   4963	
>   4964		/*  Queue data for delivery to the user.
>   4965		 *  Packets in sequence go to the receive queue.
>   4966		 *  Out of sequence packets to the out_of_order_queue.
>   4967		 */
>   4968		if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
>   4969			if (tcp_receive_window(tp) == 0) {
>   4970				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
>   4971				goto out_of_window;
>   4972			}
>   4973	
>   4974			/* Ok. In sequence. In window. */
>   4975	queue_and_out:
>   4976			if (skb_queue_len(&sk->sk_receive_queue) == 0)
>   4977				sk_forced_mem_schedule(sk, skb->truesize);
>   4978			else if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
>   4979				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
>   4980				sk->sk_data_ready(sk);
>   4981				goto drop;
>   4982			}
>   4983	
>   4984			eaten = tcp_queue_rcv(sk, skb, &fragstolen);
>   4985			if (skb->len)
>   4986				tcp_event_data_recv(sk, skb);
>   4987			if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
>   4988				tcp_fin(sk);
>   4989	
>   4990			if (!RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
>   4991				tcp_ofo_queue(sk);
>   4992	
>   4993				/* RFC5681. 4.2. SHOULD send immediate ACK, when
>   4994				 * gap in queue is filled.
>   4995				 */
>   4996				if (RB_EMPTY_ROOT(&tp->out_of_order_queue))
>   4997					inet_csk(sk)->icsk_ack.pending |= ICSK_ACK_NOW;
>   4998			}
>   4999	
>   5000			if (tp->rx_opt.num_sacks)
>   5001				tcp_sack_remove(tp);
>   5002	
>   5003			tcp_fast_path_check(sk);
>   5004	
>   5005			if (eaten > 0)
>   5006				kfree_skb_partial(skb, fragstolen);
>   5007			if (!sock_flag(sk, SOCK_DEAD))
>   5008				tcp_data_ready(sk);
>   5009			return;
>   5010		}
>   5011	
>   5012		if (!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt)) {
>   5013			tcp_rcv_spurious_retrans(sk, skb);
>   5014			/* A retransmit, 2nd most common case.  Force an immediate ack. */
>   5015			NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
>   5016			tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
>   5017	
>   5018	out_of_window:
>   5019			tcp_enter_quickack_mode(sk, TCP_MAX_QUICKACKS);
>   5020			inet_csk_schedule_ack(sk);
>   5021	drop:
>   5022			tcp_drop(sk, skb);
>   5023			return;
>   5024		}
>   5025	
>   5026		/* Out of window. F.e. zero window probe. */
>   5027		if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
>   5028			goto out_of_window;
>   5029	
>   5030		if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
>   5031			/* Partial packet, seq < rcv_next < end_seq */
>   5032			tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, tp->rcv_nxt);
>   5033	
>   5034			/* If window is closed, drop tail of packet. But after
>   5035			 * remembering D-SACK for its head made in previous line.
>   5036			 */
>   5037			if (!tcp_receive_window(tp)) {
>   5038				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
>   5039				goto out_of_window;
>   5040			}
>   5041			goto queue_and_out;
>   5042		}
>   5043	
>   5044		tcp_data_queue_ofo(sk, skb);
>   5045	}
>   5046	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>