All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options
@ 2026-10-13  8:56 T S Rameshkumar
  2026-10-13  9:07 ` Matthieu Baerts
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: T S Rameshkumar @ 2026-10-13  8:56 UTC (permalink / raw)
  To: mptcp; +Cc: matttbe, T S Rameshkumar

In mptcp_parse_option(), invalid MPTCP options or unsupported suboptions
and malformed option lengths are silently ignored via break statements.
As noted in GitHub issue #628, tracking these invalid options with a MIB
counter is useful to detect potential attacks, network corruption, or
misbehaving peers.

Introduce MPTCP_MIB_RCVINVALIDOPT ("RcvInvalidOpt") to track invalid
MPTCP options encountered during parsing. When mptcp_parse_option()
encounters invalid combinations, lengths, versions, or unknown subtypes,
jump to the bad_opt label to increment MPTCP_MIB_RCVINVALIDOPT.

To increment the MIB counter, mptcp_parse_option() requires a pointer to
struct net. Safely resolve net in mptcp_get_options() via mptcp_skb_net()
from skb->sk, skb->dev, or skb_dst, with NULL guards to prevent NULL
pointer dereferences when sk, dev, or dst is not set.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/628
Signed-off-by: T S Rameshkumar <rameshkumar.t@phytecembedded.in>
---
 net/mptcp/mib.c     |  1 +
 net/mptcp/mib.h     |  1 +
 net/mptcp/options.c | 68 ++++++++++++++++++++++++++++-----------------
 3 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index 608cb5688..0ab1ddac8 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -95,6 +95,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("BacklogDrop", MPTCP_MIB_BACKLOGDROP),
 	SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
 	SNMP_MIB_ITEM("OFOPruned", MPTCP_MIB_OFOPRUNED),
+	SNMP_MIB_ITEM("RcvInvalidOpt", MPTCP_MIB_RCVINVALIDOPT),
 };
 
 /* mptcp_mib_alloc - allocate percpu mib counters
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 1ebdb55e9..483f79dab 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -98,6 +98,7 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_BACKLOGDROP,		/* Backlog over memory limit */
 	MPTCP_MIB_RCVPRUNED,		/* Dropped due to memory constraints */
 	MPTCP_MIB_OFOPRUNED,		/* MPTCP-level OoO queue pruned */
+	MPTCP_MIB_RCVINVALIDOPT,	/* Received invalid MPTCP option */
 	__MPTCP_MIB_MAX
 };
 
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index adbf8e077..a1af2acfa 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -20,7 +20,8 @@ static bool mptcp_cap_flag_sha256(u8 flags)
 	return (flags & MPTCP_CAP_FLAG_MASK) == MPTCP_CAP_HMAC_SHA256;
 }
 
-static void mptcp_parse_option(const struct sk_buff *skb,
+static void mptcp_parse_option(struct net *net,
+			       const struct sk_buff *skb,
 			       const unsigned char *ptr, int opsize,
 			       struct mptcp_options_received *mp_opt)
 {
@@ -53,9 +54,9 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		/* Only the MPC + ACK can be used with a RM_ADDR */
 		if (subopt == OPTION_MPTCP_MPC_ACK) {
 			if ((mp_opt->suboptions & ~OPTION_MPTCP_RM_ADDR) != 0)
-				break;
+				goto bad_opt;
 		} else if (mp_opt->suboptions != 0) {
-			break;
+			goto bad_opt;
 		}
 
 		/* Cfr RFC 8684 Section 3.3.0:
@@ -71,21 +72,21 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		if (opsize != expected_opsize &&
 		    (expected_opsize != TCPOLEN_MPTCP_MPC_ACK_DATA ||
 		     opsize != TCPOLEN_MPTCP_MPC_ACK_DATA_CSUM))
-			break;
+			goto bad_opt;
 
 		/* try to be gentle vs future versions on the initial syn */
 		version = *ptr++ & MPTCP_VERSION_MASK;
 		if (opsize != TCPOLEN_MPTCP_MPC_SYN) {
 			if (version != MPTCP_SUPPORTED_VERSION)
-				break;
+				goto bad_opt;
 		} else if (version < MPTCP_SUPPORTED_VERSION) {
-			break;
+			goto bad_opt;
 		}
 
 		flags = *ptr++;
 		if (!mptcp_cap_flag_sha256(flags) ||
 		    (flags & MPTCP_CAP_EXTENSIBILITY))
-			break;
+			goto bad_opt;
 
 		/* RFC 6824, Section 3.1:
 		 * "For the Checksum Required bit (labeled "A"), if either
@@ -137,7 +138,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		/* Can be used with a restricted number of other options */
 		if ((mp_opt->suboptions & ~(OPTION_MPTCP_RM_ADDR |
 					    OPTION_MPTCP_PRIO)) != 0)
-			break;
+			goto bad_opt;
 
 		if (opsize == TCPOLEN_MPTCP_MPJ_SYN) {
 			mp_opt->suboptions |= OPTION_MPTCP_MPJ_SYN;
@@ -166,6 +167,8 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 			ptr += 2;
 			memcpy(mp_opt->hmac, ptr, MPTCPOPT_HMAC_LEN);
 			pr_debug("MP_JOIN hmac\n");
+		} else {
+			goto bad_opt;
 		}
 		break;
 
@@ -176,7 +179,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 					    OPTION_MPTCP_PRIO |
 					    OPTION_MPTCP_FASTCLOSE |
 					    OPTION_MPTCP_FAIL)) != 0)
-			break;
+			goto bad_opt;
 
 		pr_debug("DSS\n");
 		ptr++;
@@ -219,7 +222,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 			mp_opt->ack64 = 0;
 			mp_opt->use_ack = 0;
 			mp_opt->data_fin = 0;
-			break;
+			goto bad_opt;
 		}
 
 		mp_opt->suboptions |= OPTION_MPTCP_DSS;
@@ -269,7 +272,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		if ((mp_opt->suboptions & ~(OPTIONS_MPTCP_DSS |
 					    OPTION_MPTCP_RM_ADDR |
 					    OPTION_MPTCP_PRIO)) != 0)
-			break;
+			goto bad_opt;
 
 		mp_opt->echo = (*ptr++) & MPTCP_ADDR_ECHO;
 		if (!mp_opt->echo) {
@@ -282,7 +285,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 				mp_opt->addr.family = AF_INET6;
 #endif
 			else
-				break;
+				goto bad_opt;
 		} else {
 			if (opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE ||
 			    opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT)
@@ -293,7 +296,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 				mp_opt->addr.family = AF_INET6;
 #endif
 			else
-				break;
+				goto bad_opt;
 		}
 
 		mp_opt->suboptions |= OPTION_MPTCP_ADD_ADDR;
@@ -336,11 +339,11 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 					    OPTIONS_MPTCP_DSS |
 					    OPTION_MPTCP_ADD_ADDR |
 					    OPTION_MPTCP_PRIO)) != 0)
-			break;
+			goto bad_opt;
 
 		if (opsize < TCPOLEN_MPTCP_RM_ADDR_BASE + 1 ||
 		    opsize > TCPOLEN_MPTCP_RM_ADDR_BASE + MPTCP_RM_IDS_MAX)
-			break;
+			goto bad_opt;
 
 		ptr++;
 
@@ -357,10 +360,10 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 					    OPTIONS_MPTCP_DSS |
 					    OPTION_MPTCP_ADD_ADDR |
 					    OPTION_MPTCP_RM_ADDR)) != 0)
-			break;
+			goto bad_opt;
 
 		if (opsize != TCPOLEN_MPTCP_PRIO)
-			break;
+			goto bad_opt;
 
 		mp_opt->suboptions |= OPTION_MPTCP_PRIO;
 		mp_opt->backup = *ptr++ & MPTCP_PRIO_BKUP;
@@ -371,10 +374,10 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		/* Can be used with a restricted number of other options */
 		if ((mp_opt->suboptions & ~(OPTIONS_MPTCP_DSS |
 					    OPTION_MPTCP_RST)) != 0)
-			break;
+			goto bad_opt;
 
 		if (opsize != TCPOLEN_MPTCP_FASTCLOSE)
-			break;
+			goto bad_opt;
 
 		ptr += 2;
 		mp_opt->fc_recv_key = get_unaligned_be64(ptr);
@@ -387,13 +390,13 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		/* Can be used with a restricted number of other options */
 		if ((mp_opt->suboptions & ~(OPTION_MPTCP_FAIL |
 					    OPTION_MPTCP_FASTCLOSE)) != 0)
-			break;
+			goto bad_opt;
 
 		if (opsize != TCPOLEN_MPTCP_RST)
-			break;
+			goto bad_opt;
 
 		if (!(TCP_SKB_CB(skb)->tcp_flags & TCPHDR_RST))
-			break;
+			goto bad_opt;
 
 		mp_opt->suboptions |= OPTION_MPTCP_RST;
 		flags = *ptr++;
@@ -407,10 +410,10 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		/* Can be used with a restricted number of other options */
 		if ((mp_opt->suboptions & ~(OPTIONS_MPTCP_DSS |
 					    OPTION_MPTCP_RST)) != 0)
-			break;
+			goto bad_opt;
 
 		if (opsize != TCPOLEN_MPTCP_FAIL)
-			break;
+			goto bad_opt;
 
 		ptr += 2;
 		mp_opt->suboptions |= OPTION_MPTCP_FAIL;
@@ -419,13 +422,28 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		break;
 
 	default:
+bad_opt:
+		if (net)
+			MPTCP_INC_STATS(net, MPTCP_MIB_RCVINVALIDOPT);
 		break;
 	}
 }
 
+static inline struct net *mptcp_skb_net(const struct sk_buff *skb)
+{
+	if (skb->sk)
+		return sock_net(skb->sk);
+	if (skb->dev)
+		return dev_net_rcu(skb->dev);
+	if (skb_dst(skb) && skb_dst_dev_rcu(skb))
+		return skb_dst_dev_net_rcu(skb);
+	return NULL;
+}
+
 void mptcp_get_options(const struct sk_buff *skb,
 		       struct mptcp_options_received *mp_opt)
 {
+	struct net *net = mptcp_skb_net(skb);
 	const struct tcphdr *th = tcp_hdr(skb);
 	const unsigned char *ptr;
 	int length;
@@ -458,7 +476,7 @@ void mptcp_get_options(const struct sk_buff *skb,
 			if (opsize > length)
 				return;	/* don't parse partial options */
 			if (opcode == TCPOPT_MPTCP)
-				mptcp_parse_option(skb, ptr, opsize, mp_opt);
+				mptcp_parse_option(net, skb, ptr, opsize, mp_opt);
 			ptr += opsize - 2;
 			length -= opsize;
 		}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options
  2026-10-13  8:56 [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options T S Rameshkumar
@ 2026-10-13  9:07 ` Matthieu Baerts
  2026-10-13  9:35 ` sashiko-bot
  2026-10-13  9:57 ` MPTCP CI
  2 siblings, 0 replies; 4+ messages in thread
From: Matthieu Baerts @ 2026-10-13  9:07 UTC (permalink / raw)
  To: T S Rameshkumar, mptcp; +Cc: T S Rameshkumar

Hi T S,

On 13/10/2026 10:56, T S Rameshkumar wrote:
> In mptcp_parse_option(), invalid MPTCP options or unsupported suboptions
> and malformed option lengths are silently ignored via break statements.
> As noted in GitHub issue #628, tracking these invalid options with a MIB
> counter is useful to detect potential attacks, network corruption, or
> misbehaving peers.

Thank you for the patches. Please next time add a comment on the GitHub
ticket: patches have already been shared and waiting for a review (don't
hesitate to review it). If a ticket is assigned, that means someone is
working on it.

https://lore.kernel.org/mptcp/20260904-mptcp-mib-inval-opt-v2-0-a72003dc4425@kernel.org/


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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options
  2026-10-13  8:56 [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options T S Rameshkumar
  2026-10-13  9:07 ` Matthieu Baerts
@ 2026-10-13  9:35 ` sashiko-bot
  2026-10-13  9:57 ` MPTCP CI
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-13  9:35 UTC (permalink / raw)
  To: T S Rameshkumar, mptcp

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: missed MIB increment for backlog packets due to mptcp_skb_net()
--

commit 69466d93fa353450c6162f23ad6457846e513a9d
Author: T S Rameshkumar <rameshsv06@gmail.com>

mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options

This commit introduces the MPTCP_MIB_RCVINVALIDOPT counter to track invalid
MPTCP options encountered during parsing. To perform the increment, it attempts
to resolve the net pointer from the socket buffer in mptcp_skb_net() by
checking skb->sk, skb->dev, and skb_dst.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/628

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index adbf8e0773dbb..a1af2acfa791b 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c

[ ... ]

> @@ -419,13 +422,28 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>  		break;
>  
>  	default:
> +bad_opt:
> +		if (net)
> +			MPTCP_INC_STATS(net, MPTCP_MIB_RCVINVALIDOPT);
>  		break;
>  	}
>  }
>  
> +static inline struct net *mptcp_skb_net(const struct sk_buff *skb)
> +{
> +	if (skb->sk)
> +		return sock_net(skb->sk);
> +	if (skb->dev)
> +		return dev_net_rcu(skb->dev);
> +	if (skb_dst(skb) && skb_dst_dev_rcu(skb))
> +		return skb_dst_dev_net_rcu(skb);
> +	return NULL;
> +}

[Severity: Medium]
For TCP packets processed from the socket backlog queue, is it possible for
mptcp_skb_net() to always return NULL?

When a packet is processed from the backlog (e.g. via tcp_v4_do_rcv()),
skb->dev is cleared unconditionally, skb_dst is dropped, and skb->sk is not
yet set for incoming packets.

Because all three pointers are NULL, the function will return NULL. This will
cause the MIB increment at the bad_opt label to be silently skipped for any
malformed options present in backlog packets.

Since the callers of mptcp_get_options() generally have access to the relevant
socket, could they pass the known net or sk directly instead of deriving it
from the skb?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013085604.115478-1-rameshkumar.t@phytecembedded.in?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options
  2026-10-13  8:56 [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options T S Rameshkumar
  2026-10-13  9:07 ` Matthieu Baerts
  2026-10-13  9:35 ` sashiko-bot
@ 2026-10-13  9:57 ` MPTCP CI
  2 siblings, 0 replies; 4+ messages in thread
From: MPTCP CI @ 2026-10-13  9:57 UTC (permalink / raw)
  To: T S Rameshkumar; +Cc: mptcp

Hi T,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/38375270540

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/248a54272fd4
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1184533


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-13  9:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13  8:56 [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options T S Rameshkumar
2026-10-13  9:07 ` Matthieu Baerts
2026-10-13  9:35 ` sashiko-bot
2026-10-13  9:57 ` MPTCP CI

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.