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

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.