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

Matthieu Baerts <[email protected]>
Newsgroups dev.linux.lists.mptcp,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Organization NGI0 Core
Message-ID <[email protected]>
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: [email protected]
> Reported-by: TencentOS Corvus AI <[email protected]>
> Assisted-by: tencentos-corvus-ai:kimi-k3
> Signed-off-by: Fourie Zhang <[email protected]>
> ---
> 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/[email protected]

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.