Re: [PATCH v3] mptcp: do not use Fast Open on MP_JOIN subflows

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: disabled TCP timestamps on MP_JOIN with TFO due to wrong logic order
--

commit 7c8131d455e6d16e5fa896b1f57ebf371c13798d
Author: Yilin Zhang <yilinzhang@moonshot.ai>

mptcp: do not use Fast Open on MP_JOIN subflows

This commit disables TCP Fast Open on MP_JOIN subflows, as data on a subflow
requires a completed HMAC exchange. It updates the SYN processing path to
fallback to a regular handshake and strips the TFO cookie from MP_JOIN
SYN/ACKs.

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index f0a6725d2c376..7bf94aea5f681 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -348,6 +348,10 @@ static void subflow_prep_synack(const struct sock *sk, struct request_sock *req,
>  	if (foc && foc->len > -1)
>  		ireq->tstamp_ok = 0;
>  
> +	/* MP_JOIN cannot use TFO, do not send a cookie in the SYN/ACK */
> +	if (foc && mptcp_subflow_rsk(req)->mp_join)
> +		foc->len = -1;
> +
[Severity: Medium]
Does this logic order introduce a regression that unnecessarily disables
TCP timestamps on MP_JOIN subflows?

When a client initiates an MP_JOIN connection with a TCP Fast Open cookie
request, the original code in subflow_prep_synack() evaluates
foc && foc->len > -1 and sets ireq->tstamp_ok = 0.

Right after that, the newly added patch logic checks mp_join and sets
foc->len = -1 to reject TFO.

Because the timestamp check happens before the cookie is disabled, the SYN/ACK
is sent without both the TFO cookie and the TCP timestamp. Any remote client
can trigger this by simply adding a TFO option to their MP_JOIN SYN packet.

Could reversing the order of these checks preserve TCP timestamps by causing
the timestamp condition to evaluate to false when foc->len is updated to -1?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920061904.3575780-1-yilinzhang@moonshot.ai?part=1