Re: [PATCH net] mptcp: reject a DSS option that follows an incompatible suboption

[Matthieu Baerts <matttbe@kernel.org>]

Hi Fourie,

Thank you for the patch.

On 10/08/2026 13:46, Fourie Zhang wrote:
> All file:line references below are against v7.2-rc4 (ac5b0e5651b1). The
> KMSAN trace was captured on 7.2.0-rc6-kmsan72rc6 (075b74841bd0), where the
> same lines apply.
> 
> mptcp_parse_option() writes the DSS status flags before it validates the
> option length. For MPTCPOPT_DSS it assigns dsn64, use_map, ack64 and
> use_ack at net/mptcp/options.c:160-163, computes expected_opsize at
> :165-180, and only then rejects a bad length with a plain break at
> :190-192. That break leaves use_map set.
> 
> OPTION_MPTCP_DSS is a separate, sticky bit set at :194, and the mapping
> fields data_seq, subflow_seq and data_len are written only inside the
> use_map branch at :207-220, i.e. after the length check.
> 
> mptcp_get_options() clears only the four-byte status group
> ("*(u32 *)&mp_opt->status = 0", :370). data_seq, subflow_seq and data_len
> are declared ahead of struct_group(status, ...) in struct
> mptcp_options_received (net/mptcp/protocol.h:145-177), so they are left
> uninitialised, and the caller declares "struct mptcp_options_received
> mp_opt;" on the stack (:1137).
> 
> So a single segment carrying two DSS options -- first a well-formed
> ACK32-only DSS, which sets OPTION_MPTCP_DSS but no mapping, then a
> truncated DSS whose flags claim a mapping, which sets use_map and then
> fails the length check -- ends parsing with OPTION_MPTCP_DSS set and
> use_map == 1 while the mapping fields were never written.
> mptcp_incoming_options() passes the OPTION_MPTCP_DSS test at :1210 and
> copies them into the skb extension at :1253-1258; get_mapping_status()
> then branches on the uninitialised data_len:
> 
>   BUG: KMSAN: uninit-value in mptcp_subflow_data_available+0x2428/0x4c70
>    get_mapping_status (net/mptcp/subflow.c:1152)
>    subflow_check_data_avail (net/mptcp/subflow.c:1369)
>    mptcp_subflow_data_available (net/mptcp/subflow.c:1466)
>    subflow_data_ready
>    tcp_data_queue
>    tcp_rcv_established
>    tcp_v4_do_rcv
> 
>   Uninit was stored to memory at:
>    mptcp_incoming_options (net/mptcp/options.c:1258)
>    tcp_data_queue
>    tcp_rcv_established
>    tcp_v4_do_rcv
> 
>   Local variable mp_opt created at:
>    mptcp_incoming_options (net/mptcp/options.c:1137)
> 
>   CPU: 0 UID: 1000 PID: 137 Comm: poc 7.2.0-rc6-kmsan72rc6 #1
> 
> Reject a DSS option when an incompatible suboption is already present. The
> test runs before any DSS flag is read, so a malformed duplicate can no
> longer mutate state retained from an earlier valid option. ADD_ADDR,
> RM_ADDR, MP_PRIO and MP_FAIL stay permitted alongside DSS: once
> mptcp_established_options_dss() has run, those are the only options
> mptcp_established_options() can still add (:843-878).

The fix is correct, but this patch is targeting 'net', but it doesn't
apply there. That's because commit b6ee36152464 ("mptcp: avoid combining
some incoming suboptions") is already addressing this bug (and others),
the same one:


https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=b6ee36152464

Plus, see below...

> Fixes: 648ef4b88673 ("mptcp: Implement MPTCP receive path")
> Cc: stable@kernel.org
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Assisted-by: tencentos-corvus-ai:kimi-k3
> Signed-off-by: Fourie Zhang <fouriezhang@tencent.com>
> ---
> A KMSAN reproducer for this issue is available if requested.
> 
>  net/mptcp/options.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index c664023d37ba..7ec18fa1bef6 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -153,6 +153,13 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>  		break;
>  
>  	case MPTCPOPT_DSS:
> +		/* Can be used with a restricted number of other options */
> +		if ((mp_opt->suboptions & ~(OPTION_MPTCP_ADD_ADDR |
> +					    OPTION_MPTCP_RM_ADDR |
> +					    OPTION_MPTCP_PRIO |
> +					    OPTION_MPTCP_FAIL)) != 0)
> +			break;

This chunk with the exact same comment corresponds to what I sent in my
v1, a month ago:


https://lore.kernel.org/mptcp/20260709-mptcp-harden-combine-opt-v1-1-378b0a47c1b5@kernel.org

That's not because it is generated by AI it can copy code without
mentioning the source... Please avoid this next time.
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
pw-bot: rejected