All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Baerts <matttbe@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-next 3/3] mptcp: options: reset parsing in case of invalidity
Date: Thu, 3 Sep 2026 16:18:47 +0200	[thread overview]
Message-ID: <32c614b0-b103-4962-8206-4a25ee7cd766@kernel.org> (raw)
In-Reply-To: <20260903113800.968991F00A3A@smtp.kernel.org>

On 03/09/2026 13:38, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The `break` statement inside `mptcp_get_options` incorrectly exits the `switch` statement instead of the `while` loop, causing `length` and `ptr` to become desynchronized, leading to an infinite loop, out-of-bounds reads, and MPTCP option smuggling.
> --
> 
> commit 60e4bc5aa7cfc08049c3062323eeae263a824439
> Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> mptcp: options: reset parsing in case of invalidity
> 
> When an invalid option is detected, it is safer to drop any previously
> parsed MPTCP options. This ensures a fallback to TCP or a subflow reject
> instead of just ignoring the currently parsed option.
> 
>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index aaaabcab0f8f9..b6d2e39f8464a 100644
>> --- a/net/mptcp/options.c
>> +++ b/net/mptcp/options.c
> [ ... ]
>> @@ -461,9 +456,12 @@ void mptcp_get_options(const struct sock *sk, const struct sk_buff *skb,
>>  				return;
>>  			if (opsize > length)
>>  				return;	/* don't parse partial options */
>> -			if (opcode == TCPOPT_MPTCP)
>> -				mptcp_parse_option(sk, skb, ptr, opsize,
>> -						   mp_opt);
>> +			if (opcode == TCPOPT_MPTCP &&
>> +			    !mptcp_parse_option(sk, skb, ptr, opsize, mp_opt)) {
>> +				/* invalid option, drop previously parsed opt */
>> +				*(u32 *)&mp_opt->status = 0;
>> +				break;
> 
> [Severity: High]
> Does this break statement exit the switch rather than the while loop?

Arf, it was supposed to be a break for the while loop, but I missed the
switch... Let's use 'return' here, then.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


  reply	other threads:[~2026-09-03 14:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03 11:24 [PATCH mptcp-next 0/3] mptcp: options: MIB for invalid & drop suboptions Matthieu Baerts (NGI0)
2026-09-03 11:24 ` [PATCH mptcp-next 1/3] mptcp: options: use a dedicated bit for csum reqd Matthieu Baerts (NGI0)
2026-09-03 11:24 ` [PATCH mptcp-next 2/3] mptcp: options: track invalid suboptions Matthieu Baerts (NGI0)
2026-09-03 11:24 ` [PATCH mptcp-next 3/3] mptcp: options: reset parsing in case of invalidity Matthieu Baerts (NGI0)
2026-09-03 11:38   ` sashiko-bot
2026-09-03 14:18     ` Matthieu Baerts [this message]
2026-09-03 12:28 ` [PATCH mptcp-next 0/3] mptcp: options: MIB for invalid & drop suboptions MPTCP CI

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32c614b0-b103-4962-8206-4a25ee7cd766@kernel.org \
    --to=matttbe@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.