All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
@ 2026-10-27  6:27 Kalpan Jani
  2026-10-27  6:51 ` sashiko-bot
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Kalpan Jani @ 2026-10-27  6:27 UTC (permalink / raw)
  To: mptcp; +Cc: shardul.b, janak, kalpanjani009, Kalpan Jani

Normalize the MPTCP sequence fields relative to the initial data sequence
number baselines.

Keep the normalized values available to privileged diagnostic requests,
while zeroing them in INET_DIAG_INFO for callers without CAP_NET_ADMIN.

Normalize MPTCP_SUBFLOW_ATTR_MAP_SEQ against remote_idsn as well, so
subflow mapping sequence numbers use the same remote sequence space as
mptcpi_rcv_nxt.

Pass requester capability state to idiag_get_info callbacks. Other
protocol callbacks retain their existing behavior.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
---
Changes since v9:
- Keep normalized sequence values available to privileged diagnostic
  requests and zero the three sequence fields for unprivileged
  INET_DIAG_INFO callers.
- Pass requester capability state to idiag_get_info callbacks while
  preserving existing behavior for other protocol callbacks.
- Normalize MPTCP_SUBFLOW_ATTR_MAP_SEQ relative to remote_idsn so it
  uses the same remote sequence-number baseline as mptcpi_rcv_nxt.

v9: https://lore.kernel.org/all/20261015101303.144541-1-kalpan.jani@mpiricsoftware.com/
---
 include/linux/inet_diag.h |  2 +-
 net/ipv4/inet_diag.c      |  6 +++---
 net/ipv4/raw_diag.c       |  2 +-
 net/ipv4/tcp_diag.c       |  2 +-
 net/ipv4/udp_diag.c       |  2 +-
 net/mptcp/diag.c          |  8 +++++++-
 net/mptcp/mptcp_diag.c    |  9 ++++++++-
 net/mptcp/protocol.c      |  4 ++++
 net/mptcp/protocol.h      |  2 ++
 net/mptcp/sockopt.c       | 14 ++++++++++----
 net/mptcp/subflow.c       |  5 ++++-
 net/sctp/diag.c           |  6 +++---
 12 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
index 704fd415c2b49..85f6278d5db1d 100644
--- a/include/linux/inet_diag.h
+++ b/include/linux/inet_diag.h
@@ -18,7 +18,7 @@ struct inet_diag_handler {
 
 	void		(*idiag_get_info)(struct sock *sk,
 					  struct inet_diag_msg *r,
-					  void *info);
+					  void *info, bool net_admin);
 
 	int		(*idiag_get_aux)(struct sock *sk,
 					 bool net_admin,
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 34b77aa87d0a4..362161bf39fe3 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -276,7 +276,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 	}
 
 	if (!icsk) {
-		handler->idiag_get_info(sk, r, NULL);
+		handler->idiag_get_info(sk, r, NULL, net_admin);
 		goto out;
 	}
 
@@ -327,7 +327,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 			goto errout;
 	}
 
-	handler->idiag_get_info(sk, r, info);
+	handler->idiag_get_info(sk, r, info, net_admin);
 
 	if (ext & (1 << (INET_DIAG_INFO - 1)) && handler->idiag_get_aux)
 		if (handler->idiag_get_aux(sk, net_admin, skb) < 0)
@@ -1027,7 +1027,7 @@ int inet_diag_handler_get_info(struct sk_buff *skb, struct sock *sk)
 	if (attr)
 		info = nla_data(attr);
 
-	handler->idiag_get_info(sk, r, info);
+	handler->idiag_get_info(sk, r, info, false);
 	inet_diag_unlock_handler(handler);
 
 	nlmsg_end(skb, nlh);
diff --git a/net/ipv4/raw_diag.c b/net/ipv4/raw_diag.c
index 943e5998e0ad5..264912d0cb9fc 100644
--- a/net/ipv4/raw_diag.c
+++ b/net/ipv4/raw_diag.c
@@ -185,7 +185,7 @@ static void raw_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 }
 
 static void raw_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			      void *info)
+			      void *info, bool net_admin)
 {
 	r->idiag_rqueue = sk_rmem_alloc_get(sk);
 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
index ba1fdbe9807fd..cac32575282e3 100644
--- a/net/ipv4/tcp_diag.c
+++ b/net/ipv4/tcp_diag.c
@@ -19,7 +19,7 @@
 #include <net/tcp.h>
 
 static void tcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			      void *_info)
+			      void *_info, bool net_admin)
 {
 	struct tcp_info *info = _info;
 
diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
index f4b24e628cf8d..5a1fa627cbee8 100644
--- a/net/ipv4/udp_diag.c
+++ b/net/ipv4/udp_diag.c
@@ -142,7 +142,7 @@ static void udp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 }
 
 static void udp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-		void *info)
+		void *info, bool net_admin)
 {
 	r->idiag_rqueue = udp_rqueue_get(sk);
 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
diff --git a/net/mptcp/diag.c b/net/mptcp/diag.c
index 70cf9ebce8338..590cdee419ef9 100644
--- a/net/mptcp/diag.c
+++ b/net/mptcp/diag.c
@@ -16,6 +16,7 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin
 {
 	struct mptcp_subflow_context *sf;
 	struct nlattr *start;
+	u64 map_seq;
 	u32 flags = 0;
 	bool slow;
 	int err;
@@ -35,6 +36,11 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin
 		goto nla_failure;
 	}
 
+	map_seq = 0;
+	if (sf->conn && sf->map_seq)
+		map_seq = sf->map_seq -
+			  READ_ONCE(mptcp_sk(sf->conn)->remote_idsn);
+
 	if (sf->mp_capable)
 		flags |= MPTCP_SUBFLOW_FLAG_MCAP_REM;
 	if (sf->request_mptcp)
@@ -67,7 +73,7 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin
 	if (net_admin &&
 	    (nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ,
 			 sf->rel_write_seq) ||
-	     nla_put_u64_64bit(skb, MPTCP_SUBFLOW_ATTR_MAP_SEQ, sf->map_seq,
+	     nla_put_u64_64bit(skb, MPTCP_SUBFLOW_ATTR_MAP_SEQ, map_seq,
 			       MPTCP_SUBFLOW_ATTR_PAD) ||
 	     nla_put_u32(skb, MPTCP_SUBFLOW_ATTR_MAP_SFSEQ,
 			 sf->map_subflow_seq) ||
diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index 136c2d05c0eeb..d526b1015da16 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -190,7 +190,7 @@ static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 }
 
 static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-				void *_info)
+				void *_info, bool net_admin)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct mptcp_info *info = _info;
@@ -215,6 +215,13 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
 		return;
 
 	mptcp_diag_fill_info(msk, info);
+
+	if (!net_admin) {
+		/* Hide data sequence numbers from unprivileged users. */
+		info->mptcpi_write_seq = 0;
+		info->mptcpi_snd_una = 0;
+		info->mptcpi_rcv_nxt = 0;
+	}
 }
 
 static const struct inet_diag_handler mptcp_diag_handler = {
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f22d64ab1c53b..e59209b6fc41c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3216,6 +3216,8 @@ static void __mptcp_init_sock(struct sock *sk)
 	INIT_WORK(&msk->work, mptcp_worker);
 	msk->out_of_order_queue = RB_ROOT;
 	msk->first_pending = NULL;
+	WRITE_ONCE(msk->local_idsn, 0);
+	WRITE_ONCE(msk->remote_idsn, 0);
 
 	/* msk does not go through tcp_init_sock(); seed RTO bounds. */
 	icsk->icsk_rto_min =
@@ -3763,6 +3765,7 @@ struct sock *mptcp_sk_clone_init(const struct sock *sk,
 
 	msk = mptcp_sk(nsk);
 	WRITE_ONCE(msk->local_key, subflow_req->local_key);
+	WRITE_ONCE(msk->local_idsn, subflow_req->idsn);
 	WRITE_ONCE(msk->token, subflow_req->token);
 	msk->in_accept_queue = 1;
 	WRITE_ONCE(msk->fully_established, false);
@@ -4201,6 +4204,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 					     MPTCP_MIB_TOKENFALLBACKINIT);
 	}
 
+	WRITE_ONCE(msk->local_idsn, subflow->idsn);
 	WRITE_ONCE(msk->write_seq, subflow->idsn);
 	WRITE_ONCE(msk->snd_nxt, subflow->idsn);
 	WRITE_ONCE(msk->snd_una, subflow->idsn);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3d250e8204d52..fc6858863fa89 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -288,6 +288,8 @@ struct mptcp_sock {
 						 * lockless access read
 						 */
 	u64		remote_key;		/* same as above */
+	u64		local_idsn;
+	u64		remote_idsn;
 	u64		write_seq;
 	u64		bytes_sent;
 	u64		snd_nxt;
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 922f6ae5c80cb..b20263b4cd55c 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1047,6 +1047,7 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
 void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 {
 	struct sock *sk = (struct sock *)msk;
+	u64 local_idsn = 0, remote_idsn = 0;
 	u32 flags = 0;
 	bool slow;
 	u32 now;
@@ -1084,9 +1085,11 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	info->mptcpi_flags = flags;
 
 	slow = lock_sock_fast(sk);
+	local_idsn = READ_ONCE(msk->local_idsn);
+
 	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
 	info->mptcpi_token = msk->token;
-	info->mptcpi_write_seq = msk->write_seq;
+	info->mptcpi_write_seq = msk->write_seq - local_idsn;
 	info->mptcpi_retransmits = inet_csk(sk)->icsk_retransmits;
 	info->mptcpi_bytes_sent = msk->bytes_sent;
 	info->mptcpi_bytes_received = msk->bytes_received;
@@ -1099,17 +1102,20 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	unlock_sock_fast(sk, slow);
 
 	mptcp_data_lock(sk);
+	if (READ_ONCE(msk->can_ack))
+		remote_idsn = READ_ONCE(msk->remote_idsn);
 	info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);
-	info->mptcpi_snd_una = msk->snd_una;
-	info->mptcpi_rcv_nxt = msk->ack_seq;
+	info->mptcpi_snd_una = msk->snd_una - local_idsn;
+	info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn;
 	info->mptcpi_bytes_acked = msk->bytes_acked;
 	mptcp_data_unlock(sk);
+
 }
 EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
 
 static int mptcp_getsockopt_info(struct mptcp_sock *msk, char __user *optval, int __user *optlen)
 {
-	struct mptcp_info m_info;
+	struct mptcp_info m_info = {};
 	int len;
 
 	if (get_user(len, optlen))
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 2d7ccb01d2342..39c7422afa779 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -482,7 +482,6 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
 	if (subflow->remote_key_valid)
 		return;
 
-	subflow->remote_key_valid = 1;
 	subflow->remote_key = mp_opt->sndr_key;
 	mptcp_crypto_key_sha(subflow->remote_key, NULL, &subflow->iasn);
 	subflow->iasn++;
@@ -491,9 +490,13 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
 	subflow->map_seq = subflow->iasn;
 
 	WRITE_ONCE(msk->remote_key, subflow->remote_key);
+	WRITE_ONCE(msk->remote_idsn, subflow->iasn - 1);
 	WRITE_ONCE(msk->ack_seq, subflow->iasn);
 	WRITE_ONCE(msk->can_ack, true);
 	atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
+
+	/* publish last, once iasn and the fields above are fully populated */
+	subflow->remote_key_valid = 1;
 }
 
 static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
diff --git a/net/sctp/diag.c b/net/sctp/diag.c
index c2a0de2adf6fd..ab6c560fde053 100644
--- a/net/sctp/diag.c
+++ b/net/sctp/diag.c
@@ -20,7 +20,7 @@
 #include <net/sctp/sctp.h>
 
 static void sctp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			       void *info);
+			       void *info, bool net_admin);
 
 /* define some functions to make asoc/ep fill look clean */
 static void inet_diag_msg_sctpasoc_fill(struct inet_diag_msg *r,
@@ -200,7 +200,7 @@ static int inet_sctp_diag_fill(struct sock *sk, struct sctp_association *asoc,
 	}
 	infox.sctpinfo = (struct sctp_info *)info;
 	infox.asoc = asoc;
-	sctp_diag_get_info(sk, r, &infox);
+	sctp_diag_get_info(sk, r, &infox, net_admin);
 
 	addr_list = asoc ? &asoc->base.bind_addr.address_list
 			 : &ep->base.bind_addr.address_list;
@@ -412,7 +412,7 @@ static int sctp_ep_dump(struct sctp_endpoint *ep, void *p)
 
 /* define the functions for sctp_diag_handler*/
 static void sctp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			       void *info)
+			       void *info, bool net_admin)
 {
 	struct sctp_infox *infox = (struct sctp_infox *)info;
 
-- 
2.43.0


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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27  6:27 [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
@ 2026-10-27  6:51 ` sashiko-bot
  2026-10-27  7:33 ` Matthieu Baerts
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-10-27  6:51 UTC (permalink / raw)
  To: Kalpan Jani, mptcp

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mptcp: stale remote_idsn after mptcp_disconnect() corrupts subflow map_seq
- [Medium] mptcp: torn local_idsn and snd_una snapshot in mptcp_diag_fill_info()
--

commit 7fad37bbb6af298d7d24854d3ee0023c92539f7c
Author: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

mptcp: normalize seq numbers reported in mptcp_info

This patch normalizes the sequence number fields in mptcp_info and
MPTCP_SUBFLOW_ATTR_MAP_SEQ relative to the initial data sequence numbers. It
also passes the requester capability state to idiag_get_info callbacks so that
sequence numbers in INET_DIAG_INFO are zeroed for unprivileged callers.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445

[ ... ]

> diff --git a/net/mptcp/diag.c b/net/mptcp/diag.c
> index 70cf9ebce8338..590cdee419ef9 100644
> --- a/net/mptcp/diag.c
> +++ b/net/mptcp/diag.c
> @@ -16,6 +16,7 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin
>  {
>  	struct mptcp_subflow_context *sf;
>  	struct nlattr *start;
> +	u64 map_seq;
>  	u32 flags = 0;
>  	bool slow;
>  	int err;
> @@ -35,6 +36,11 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin
>  		goto nla_failure;
>  	}
>  
> +	map_seq = 0;
> +	if (sf->conn && sf->map_seq)
> +		map_seq = sf->map_seq -
> +			  READ_ONCE(mptcp_sk(sf->conn)->remote_idsn);

[Severity: Medium]
Can this underflow map_seq if a connected MPTCP socket is disconnected via
mptcp_disconnect() and then reconnected with a fallback to TCP?

When an initial connection completes the MP_CAPABLE handshake,
subflow_set_remote_key() sets msk->remote_idsn and msk->can_ack = true. If the
socket is subsequently disconnected, mptcp_disconnect() resets msk->can_ack,
msk->ack_seq, and the byte counters, while __mptcp_subflow_disconnect() resets
the subflow context (zeroing subflow->iasn and subflow->map_seq), but
msk->remote_idsn (as well as msk->local_idsn, msk->write_seq, msk->snd_nxt,
and msk->snd_una) is left holding the previous connection's values:

net/mptcp/protocol.c:mptcp_disconnect() {
	...
	WRITE_ONCE(msk->can_ack, false);
	...
	/* for fallback's sake */
	WRITE_ONCE(msk->ack_seq, 0);
	atomic64_set(&msk->rcv_wnd_sent, 0);
}

If the socket is then reconnected via mptcp_connect() and falls back to plain
TCP before receiving a peer key (for example via mptcp_early_fallback() or
when the SYN-ACK lacks MP_CAPABLE in subflow_finish_connect()),
subflow_set_remote_key() is not called. When data arrives on the fallback
connection, subflow_check_data_avail() computes subflow->map_seq relative to
subflow->iasn == 0.

In mptcp_diag_fill_info(), reading msk->remote_idsn is guarded by
READ_ONCE(msk->can_ack), so mptcpi_rcv_nxt uses remote_idsn == 0. However,
subflow_get_info() does not check can_ack and subtracts the stale
msk->remote_idsn of the previous connection from the 0-based sf->map_seq.

Should mptcp_disconnect() reset the sequence and IDSN fields, and should
subflow_get_info() also gate remote_idsn on READ_ONCE(msk->can_ack)?

[ ... ]

> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 922f6ae5c80cb..b20263b4cd55c 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -1047,6 +1047,7 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
>  void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  {
>  	struct sock *sk = (struct sock *)msk;
> +	u64 local_idsn = 0, remote_idsn = 0;
>  	u32 flags = 0;
>  	bool slow;
>  	u32 now;
> @@ -1084,9 +1085,11 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  	info->mptcpi_flags = flags;
>  
>  	slow = lock_sock_fast(sk);
> +	local_idsn = READ_ONCE(msk->local_idsn);
> +
>  	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
>  	info->mptcpi_token = msk->token;
> -	info->mptcpi_write_seq = msk->write_seq;
> +	info->mptcpi_write_seq = msk->write_seq - local_idsn;
>  	info->mptcpi_retransmits = inet_csk(sk)->icsk_retransmits;
>  	info->mptcpi_bytes_sent = msk->bytes_sent;
>  	info->mptcpi_bytes_received = msk->bytes_received;
> @@ -1099,17 +1102,20 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  	unlock_sock_fast(sk, slow);
>  
>  	mptcp_data_lock(sk);
> +	if (READ_ONCE(msk->can_ack))
> +		remote_idsn = READ_ONCE(msk->remote_idsn);
>  	info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);
> -	info->mptcpi_snd_una = msk->snd_una;
> -	info->mptcpi_rcv_nxt = msk->ack_seq;
> +	info->mptcpi_snd_una = msk->snd_una - local_idsn;
> +	info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn;

[Severity: Medium]
Could mptcp_diag_fill_info() observe a torn pair of local_idsn and
msk->snd_una if it races with mptcp_connect()?

In mptcp_getsockopt(), the socket lock is released before calling
mptcp_getsockopt_sol_mptcp() -> mptcp_getsockopt_info() ->
mptcp_diag_fill_info(). In mptcp_diag_fill_info(), local_idsn is sampled under
lock_sock_fast(sk) and then the socket lock is released via
unlock_sock_fast(sk, slow) before acquiring mptcp_data_lock(sk).

If another thread concurrently calls connect() -> inet_stream_connect() ->
mptcp_connect() right after unlock_sock_fast(sk, slow), mptcp_connect()
initializes msk->local_idsn and msk->snd_una under lock_sock(sk) without
holding mptcp_data_lock(sk):

net/mptcp/protocol.c:mptcp_connect() {
	...
	WRITE_ONCE(msk->local_idsn, subflow->idsn);
	WRITE_ONCE(msk->write_seq, subflow->idsn);
	WRITE_ONCE(msk->snd_nxt, subflow->idsn);
	WRITE_ONCE(msk->snd_una, subflow->idsn);
}

When mptcp_diag_fill_info() then acquires mptcp_data_lock(sk), msk->snd_una
will be subflow->idsn while the previously sampled local_idsn is still 0,
causing info->mptcpi_snd_una to report the raw unnormalized subflow->idsn to
userspace.

Would it be safer to re-read READ_ONCE(msk->local_idsn) or sample msk->snd_una
using READ_ONCE() within the same critical section?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027062750.2095515-1-kalpan.jani@mpiricsoftware.com?part=1

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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27  6:27 [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
  2026-10-27  6:51 ` sashiko-bot
@ 2026-10-27  7:33 ` Matthieu Baerts
  2026-10-27  7:38 ` MPTCP CI
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Matthieu Baerts @ 2026-10-27  7:33 UTC (permalink / raw)
  To: Kalpan Jani, mptcp; +Cc: shardul.b, janak, kalpanjani009

Hi Kalpan,

On 27/10/2026 07:27, Kalpan Jani wrote:
> Normalize the MPTCP sequence fields relative to the initial data sequence
> number baselines.
> 
> Keep the normalized values available to privileged diagnostic requests,
> while zeroing them in INET_DIAG_INFO for callers without CAP_NET_ADMIN.
> 
> Normalize MPTCP_SUBFLOW_ATTR_MAP_SEQ against remote_idsn as well, so
> subflow mapping sequence numbers use the same remote sequence space as
> mptcpi_rcv_nxt.
> 
> Pass requester capability state to idiag_get_info callbacks. Other
> protocol callbacks retain their existing behavior.
> 
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445
> Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
> ---
> Changes since v9:

Thank you for the new version!

> - Keep normalized sequence values available to privileged diagnostic
>   requests and zero the three sequence fields for unprivileged
>   INET_DIAG_INFO callers.
> - Pass requester capability state to idiag_get_info callbacks while
>   preserving existing behavior for other protocol callbacks.

I think it should be one or the other: the suggestion to disable the
different fields for non net-admin is a sort of workaround not to have
to deal with the possible issues reported by Sashiko.

If we take this route, the only modifications on MPTCP side would be in
the diag files (or if you have fixes, they can be in separated patches).
Also, it would be easier to split this change in two: exposing the
net_admin boolean, then modifying MPTCP to hide data.

BTW, using the bytes sent/received was not something that could be used
here?

> - Normalize MPTCP_SUBFLOW_ATTR_MAP_SEQ relative to remote_idsn so it
>   uses the same remote sequence-number baseline as mptcpi_rcv_nxt.

I guess it would be 0 for non net-admin.

(...)

> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> index 34b77aa87d0a4..362161bf39fe3 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -276,7 +276,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
>  	}
>  
>  	if (!icsk) {
> -		handler->idiag_get_info(sk, r, NULL);
> +		handler->idiag_get_info(sk, r, NULL, net_admin);
>  		goto out;
>  	}
>  
> @@ -327,7 +327,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
>  			goto errout;
>  	}
>  
> -	handler->idiag_get_info(sk, r, info);
> +	handler->idiag_get_info(sk, r, info, net_admin);
>  
>  	if (ext & (1 << (INET_DIAG_INFO - 1)) && handler->idiag_get_aux)
>  		if (handler->idiag_get_aux(sk, net_admin, skb) < 0)
> @@ -1027,7 +1027,7 @@ int inet_diag_handler_get_info(struct sk_buff *skb, struct sock *sk)
>  	if (attr)
>  		info = nla_data(attr);
>  
> -	handler->idiag_get_info(sk, r, info);
> +	handler->idiag_get_info(sk, r, info, false);

I didn't check, but can you not extract the capability from the skb?

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


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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27  6:27 [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
  2026-10-27  6:51 ` sashiko-bot
  2026-10-27  7:33 ` Matthieu Baerts
@ 2026-10-27  7:38 ` MPTCP CI
  2026-10-27  9:54 ` Kalpan Jani
  2026-10-28 19:41 ` netdev-bot+sashiko
  4 siblings, 0 replies; 21+ messages in thread
From: MPTCP CI @ 2026-10-27  7:38 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp

Hi Kalpan,

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/40463062491

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


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] 21+ messages in thread

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27  6:27 [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
                   ` (2 preceding siblings ...)
  2026-10-27  7:38 ` MPTCP CI
@ 2026-10-27  9:54 ` Kalpan Jani
  2026-10-27 10:45   ` Matthieu Baerts
  2026-10-28 19:41 ` netdev-bot+sashiko
  4 siblings, 1 reply; 21+ messages in thread
From: Kalpan Jani @ 2026-10-27  9:54 UTC (permalink / raw)
  To: mptcp; +Cc: matttbe, martineau, pabeni

Hi Matt,

Thanks for reviewing the new version and for the suggestions.

I understand that v10 mixes two different approaches: normalizing the sequence values and hiding them from unprivileged diagnostic requests. I will separate these concerns rather than keep them bundled together.

I will first check whether the requester's CAP_NET_ADMIN status can be obtained reliably from the existing skb in inet_diag_handler_get_info(), so that we can avoid changing the idiag_get_info callback interface if possible. If that is not feasible, I will keep the generic callback change separate from the MPTCP-specific diagnostic change.

For MPTCP_SUBFLOW_ATTR_MAP_SEQ, I understand that the value should be zero for callers without CAP_NET_ADMIN. I will apply the same access-control policy consistently to the relevant MPTCP diagnostic fields.

I will also investigate whether the existing byte counters, such as bytes sent, received, and acknowledged, can provide the information needed for issue #445 without exposing the sequence counters. I will keep any required sequence-baseline fixes, including the two issues reported by Sashiko, separate from the diagnostic access-control change.

I will update the patch series after checking these points and validating the changes.

Cheers,
Kalpan

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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27  9:54 ` Kalpan Jani
@ 2026-10-27 10:45   ` Matthieu Baerts
  2026-10-27 12:29     ` Kalpan Jani
  0 siblings, 1 reply; 21+ messages in thread
From: Matthieu Baerts @ 2026-10-27 10:45 UTC (permalink / raw)
  To: Kalpan Jani, mptcp; +Cc: martineau, pabeni

Hi Kalpan,

Thank you for your reply.

On 27/10/2026 10:54, Kalpan Jani wrote:
> Hi Matt,
> 
> Thanks for reviewing the new version and for the suggestions.
> 
> I understand that v10 mixes two different approaches: normalizing the
> sequence values and hiding them from unprivileged diagnostic
> requests. I will separate these concerns rather than keep them
> bundled together.
> 
> I will first check whether the requester's CAP_NET_ADMIN status can
> be obtained reliably from the existing skb in
> inet_diag_handler_get_info(), so that we can avoid changing the
> idiag_get_info callback interface if possible. If that is not
> feasible, I will keep the generic callback change separate from the
> MPTCP-specific diagnostic change.
> 
> For MPTCP_SUBFLOW_ATTR_MAP_SEQ, I understand that the value should be
> zero for callers without CAP_NET_ADMIN. I will apply the same access-
> control policy consistently to the relevant MPTCP diagnostic fields.
> 
> I will also investigate whether the existing byte counters, such as
> bytes sent, received, and acknowledged, can provide the information
> needed for issue #445 without exposing the sequence counters. I will
> keep any required sequence-baseline fixes, including the two issues
> reported by Sashiko, separate from the diagnostic access-control
> change.

Just to be sure it is clear:

- If it is possible to use the bytes counters to provide the normalised
  seq counters, let's use them. If not, please say why before sending a
  new version, maybe we could add a similar counter instead of relying
  on the initial one.

- If that's not possible, set 0 for if no CAP_NET_ADMIN by looking at
  the skb (minimal changes only on MPTCP side).

- If that's not possible, add the net_admin parameter, and try to avoid
  setting "false" in inet_diag_handler_get_info → maybe we can check the
  capability linked to this skb? → to be tested of course

> I will update the patch series after checking these points and
> validating the changes.

Thank you for your patience!

Also, please reply to the right message with quote to add context,
otherwise it is hard to follow when switching between many series.

  https://subspace.kernel.org/etiquette.html

  https://docs.kernel.org/process/email-clients.html

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


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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27 10:45   ` Matthieu Baerts
@ 2026-10-27 12:29     ` Kalpan Jani
  2026-10-27 13:42       ` Matthieu Baerts
  0 siblings, 1 reply; 21+ messages in thread
From: Kalpan Jani @ 2026-10-27 12:29 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp, martineau, pabeni

Hi Matt,

I investigated the existing byte counters and the diagnostic capability path.

The existing byte counters are not direct replacements for the normalized sequence fields:

- write_seq advances when application data is queued and when DATA_FIN is added. bytes_sent tracks send progress, and the retransmission path can update snd_nxt directly without updating bytes_sent.
- snd_una is assigned directly during connection setup, outside __mptcp_snd_una_update(). Therefore, bytes_acked cannot be assumed to reproduce snd_una - local_idsn.
- ack_seq advances when processing DATA_FIN, whereas bytes_received counts payload bytes. Their initial values also differ.

Regarding the capability check, the normal MPTCP diagnostic dump paths have access to the original request skb and can check netlink_net_capable(cb->skb, CAP_NET_ADMIN).

However, inet_diag_handler_get_info() is also called for socket-destruction notifications. In that path, sock_diag_broadcast_destroy_work() creates a new skb and invokes the callback without an original diagnostic request. Therefore, the requester's capability cannot be recovered from that skb. Passing false in this notification path appears to be the appropriate conservative behavior.

Based on this investigation, I will keep the capability/privacy change separate from sequence normalization. I will also address the stale remote_idsn after disconnect and the inconsistent sequence snapshot reported by Sashiko before sending another normalization revision.

The existing counters are not direct substitutes for the sequence fields, although dedicated accounting remains a possible option if needed.

Cheers,
Kalpan Jani


From: Matthieu Baerts <matttbe@kernel.org>
To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com>, <mptcp@lists.linux.dev>
Cc: <martineau@kernel.org>, <pabeni@redhat.com>
Date: Tue, 27 Oct 2026 16:15:13 +0530
Subject: Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info

 > Hi Kalpan,
 > 
 > Thank you for your reply.
 > 
 > On 27/10/2026 10:54, Kalpan Jani wrote:
 > > Hi Matt,
 > > 
 > > Thanks for reviewing the new version and for the suggestions.
 > > 
 > > I understand that v10 mixes two different approaches: normalizing the
 > > sequence values and hiding them from unprivileged diagnostic
 > > requests. I will separate these concerns rather than keep them
 > > bundled together.
 > > 
 > > I will first check whether the requester's CAP_NET_ADMIN status can
 > > be obtained reliably from the existing skb in
 > > inet_diag_handler_get_info(), so that we can avoid changing the
 > > idiag_get_info callback interface if possible. If that is not
 > > feasible, I will keep the generic callback change separate from the
 > > MPTCP-specific diagnostic change.
 > > 
 > > For MPTCP_SUBFLOW_ATTR_MAP_SEQ, I understand that the value should be
 > > zero for callers without CAP_NET_ADMIN. I will apply the same access-
 > > control policy consistently to the relevant MPTCP diagnostic fields.
 > > 
 > > I will also investigate whether the existing byte counters, such as
 > > bytes sent, received, and acknowledged, can provide the information
 > > needed for issue #445 without exposing the sequence counters. I will
 > > keep any required sequence-baseline fixes, including the two issues
 > > reported by Sashiko, separate from the diagnostic access-control
 > > change.
 > 
 > Just to be sure it is clear:
 > 
 > - If it is possible to use the bytes counters to provide the normalised
 >   seq counters, let's use them. If not, please say why before sending a
 >   new version, maybe we could add a similar counter instead of relying
 >   on the initial one.
 > 
 > - If that's not possible, set 0 for if no CAP_NET_ADMIN by looking at
 >   the skb (minimal changes only on MPTCP side).
 > 
 > - If that's not possible, add the net_admin parameter, and try to avoid
 >   setting "false" in inet_diag_handler_get_info → maybe we can check the
 >   capability linked to this skb? → to be tested of course
 > 
 > > I will update the patch series after checking these points and
 > > validating the changes.
 > 
 > Thank you for your patience!
 > 
 > Also, please reply to the right message with quote to add context,
 > otherwise it is hard to follow when switching between many series.
 > 
 >   https://subspace.kernel.org/etiquette.html
 > 
 >   https://docs.kernel.org/process/email-clients.html
 > 
 > Cheers,
 > Matt
 > -- 
 > Sponsored by the NGI0 Core fund.
 > 
 > 



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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27 12:29     ` Kalpan Jani
@ 2026-10-27 13:42       ` Matthieu Baerts
  2026-10-28  8:26         ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters Kalpan Jani
  2026-10-28  9:43         ` [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Matthieu Baerts
  0 siblings, 2 replies; 21+ messages in thread
From: Matthieu Baerts @ 2026-10-27 13:42 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp, martineau, pabeni

Hi Kalpan,

Thank you for your reply.

On 27/10/2026 13:29, Kalpan Jani wrote:
> Hi Matt,
> 
> I investigated the existing byte counters and the diagnostic
> capability path.
> 
> The existing byte counters are not direct replacements for the
> normalized sequence fields:
> 
> - write_seq advances when application data is queued and when
> DATA_FIN is added. bytes_sent tracks send progress, and the
> retransmission path can update snd_nxt directly without updating
> bytes_sent. - snd_una is assigned directly during connection setup,
> outside __mptcp_snd_una_update(). Therefore, bytes_acked cannot be
> assumed to reproduce snd_una - local_idsn. - ack_seq advances when
> processing DATA_FIN, whereas bytes_received counts payload bytes.
> Their initial values also differ.

OK, let's not use the bytes counters then.

> Regarding the capability check, the normal MPTCP diagnostic dump
> paths have access to the original request skb and can check
> netlink_net_capable(cb->skb, CAP_NET_ADMIN).

Good!

> However, inet_diag_handler_get_info() is also called for socket-
> destruction notifications. In that path,
> sock_diag_broadcast_destroy_work() creates a new skb and invokes the
> callback without an original diagnostic request. Therefore, the
> requester's capability cannot be recovered from that skb. Passing
> false in this notification path appears to be the appropriate
> conservative behavior.

OK to pass false in this case.

> Based on this investigation, I will keep the capability/privacy
> change separate from sequence normalization. I will also address the
> stale remote_idsn after disconnect and the inconsistent sequence
> snapshot reported by Sashiko before sending another normalization
> revision.

Just to be sure it is clear: the main goal here is not to show the seq
counters to everybody from the same netns. This can be done either by
"normalising" them -- i.e. starting from 0 -- or by not presenting them
to some users.

Just thinking about that: for the normalisation, we can also just
add/remove an offset the userspace cannot predict and different per
connection. In fact, why not using the address of the msk pointer? I
guess that should prevent the different issues reported by Sashiko, no?

(In case of "other" existing issues, please split them and move them in
a dedicated patch not to mix the fix and the main patch here.)

If the "normalisation" is not possible, better not displaying them to
non cap-admin users. But we don't need both.

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


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

* [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters
  2026-10-27 13:42       ` Matthieu Baerts
@ 2026-10-28  8:26         ` Kalpan Jani
  2026-10-28  8:27           ` [RFC net-next v11 1/2] inet_diag: pass net_admin to get_info Kalpan Jani
                             ` (2 more replies)
  2026-10-28  9:43         ` [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Matthieu Baerts
  1 sibling, 3 replies; 21+ messages in thread
From: Kalpan Jani @ 2026-10-28  8:26 UTC (permalink / raw)
  To: mptcp; +Cc: matttbe, Kalpan Jani

This series updates v10 following the review discussion.

Instead of normalising sequence counters, it restricts their visibility
to users with CAP_NET_ADMIN.

Patch 1 passes the request's CAP_NET_ADMIN status through the generic
inet-diag callback. Socket-destruction notifications pass false because
there is no individual requesting user.

Patch 2 zeros mptcpi_write_seq, mptcpi_snd_una and mptcpi_rcv_nxt in
unprivileged MPTCP diagnostic responses. The MPTCP_INFO getsockopt path
remains unchanged.

Changes since v10:
- Drop IDSN-based sequence normalization.
- Split generic capability propagation and MPTCP visibility restriction.

Testing:
- W=1 build of all six affected diagnostic objects against the local
  mptcp/net-next reference.
- checkpatch.pl --strict: zero errors and zero warnings for both patches.

Kalpan Jani (2):
  inet_diag: pass net_admin to get_info
  mptcp: hide seq counters without net_admin

 include/linux/inet_diag.h | 3 ++-
 net/ipv4/inet_diag.c      | 6 +++---
 net/ipv4/raw_diag.c       | 2 +-
 net/ipv4/tcp_diag.c       | 2 +-
 net/ipv4/udp_diag.c       | 2 +-
 net/mptcp/mptcp_diag.c    | 8 +++++++-
 net/sctp/diag.c           | 6 +++---
 7 files changed, 18 insertions(+), 11 deletions(-)

-- 
2.43.0


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

* [RFC net-next v11 1/2] inet_diag: pass net_admin to get_info
  2026-10-28  8:26         ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters Kalpan Jani
@ 2026-10-28  8:27           ` Kalpan Jani
  2026-10-28  8:31             ` netdev-bot+sinfo
  2026-10-28  8:27           ` [RFC net-next v11 2/2] mptcp: hide seq counters without net_admin Kalpan Jani
  2026-10-28  9:32           ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters MPTCP CI
  2 siblings, 1 reply; 21+ messages in thread
From: Kalpan Jani @ 2026-10-28  8:27 UTC (permalink / raw)
  To: mptcp
  Cc: matttbe, Kalpan Jani, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Neal Cardwell,
	Kuniyuki Iwashima, Mat Martineau, Geliang Tang,
	Marcelo Ricardo Leitner, Xin Long, Kees Cook, David Ahern,
	Willem de Bruijn, netdev, linux-kernel, linux-sctp

Pass the request's CAP_NET_ADMIN status to idiag_get_info()
so protocol-specific diagnostic handlers can restrict privileged
information.

Socket-destruction notifications have no individual requesting user,
so the callback receives net_admin set to false for that path.

Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
---
 include/linux/inet_diag.h | 3 ++-
 net/ipv4/inet_diag.c      | 6 +++---
 net/ipv4/raw_diag.c       | 2 +-
 net/ipv4/tcp_diag.c       | 2 +-
 net/ipv4/udp_diag.c       | 2 +-
 net/mptcp/mptcp_diag.c    | 2 +-
 net/sctp/diag.c           | 6 +++---
 7 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
index 704fd415c2b49..8db96bfd3e67f 100644
--- a/include/linux/inet_diag.h
+++ b/include/linux/inet_diag.h
@@ -18,7 +18,8 @@ struct inet_diag_handler {
 
 	void		(*idiag_get_info)(struct sock *sk,
 					  struct inet_diag_msg *r,
-					  void *info);
+					  void *info,
+					  bool net_admin);
 
 	int		(*idiag_get_aux)(struct sock *sk,
 					 bool net_admin,
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 34b77aa87d0a4..362161bf39fe3 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -276,7 +276,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 	}
 
 	if (!icsk) {
-		handler->idiag_get_info(sk, r, NULL);
+		handler->idiag_get_info(sk, r, NULL, net_admin);
 		goto out;
 	}
 
@@ -327,7 +327,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 			goto errout;
 	}
 
-	handler->idiag_get_info(sk, r, info);
+	handler->idiag_get_info(sk, r, info, net_admin);
 
 	if (ext & (1 << (INET_DIAG_INFO - 1)) && handler->idiag_get_aux)
 		if (handler->idiag_get_aux(sk, net_admin, skb) < 0)
@@ -1027,7 +1027,7 @@ int inet_diag_handler_get_info(struct sk_buff *skb, struct sock *sk)
 	if (attr)
 		info = nla_data(attr);
 
-	handler->idiag_get_info(sk, r, info);
+	handler->idiag_get_info(sk, r, info, false);
 	inet_diag_unlock_handler(handler);
 
 	nlmsg_end(skb, nlh);
diff --git a/net/ipv4/raw_diag.c b/net/ipv4/raw_diag.c
index 943e5998e0ad5..264912d0cb9fc 100644
--- a/net/ipv4/raw_diag.c
+++ b/net/ipv4/raw_diag.c
@@ -185,7 +185,7 @@ static void raw_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 }
 
 static void raw_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			      void *info)
+			      void *info, bool net_admin)
 {
 	r->idiag_rqueue = sk_rmem_alloc_get(sk);
 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c
index ba1fdbe9807fd..cac32575282e3 100644
--- a/net/ipv4/tcp_diag.c
+++ b/net/ipv4/tcp_diag.c
@@ -19,7 +19,7 @@
 #include <net/tcp.h>
 
 static void tcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			      void *_info)
+			      void *_info, bool net_admin)
 {
 	struct tcp_info *info = _info;
 
diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
index f4b24e628cf8d..5a1fa627cbee8 100644
--- a/net/ipv4/udp_diag.c
+++ b/net/ipv4/udp_diag.c
@@ -142,7 +142,7 @@ static void udp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 }
 
 static void udp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-		void *info)
+		void *info, bool net_admin)
 {
 	r->idiag_rqueue = udp_rqueue_get(sk);
 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index 136c2d05c0eeb..9ed7a6be1e628 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -190,7 +190,7 @@ static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 }
 
 static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-				void *_info)
+				void *_info, bool net_admin)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct mptcp_info *info = _info;
diff --git a/net/sctp/diag.c b/net/sctp/diag.c
index c2a0de2adf6fd..ab6c560fde053 100644
--- a/net/sctp/diag.c
+++ b/net/sctp/diag.c
@@ -20,7 +20,7 @@
 #include <net/sctp/sctp.h>
 
 static void sctp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			       void *info);
+			       void *info, bool net_admin);
 
 /* define some functions to make asoc/ep fill look clean */
 static void inet_diag_msg_sctpasoc_fill(struct inet_diag_msg *r,
@@ -200,7 +200,7 @@ static int inet_sctp_diag_fill(struct sock *sk, struct sctp_association *asoc,
 	}
 	infox.sctpinfo = (struct sctp_info *)info;
 	infox.asoc = asoc;
-	sctp_diag_get_info(sk, r, &infox);
+	sctp_diag_get_info(sk, r, &infox, net_admin);
 
 	addr_list = asoc ? &asoc->base.bind_addr.address_list
 			 : &ep->base.bind_addr.address_list;
@@ -412,7 +412,7 @@ static int sctp_ep_dump(struct sctp_endpoint *ep, void *p)
 
 /* define the functions for sctp_diag_handler*/
 static void sctp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
-			       void *info)
+			       void *info, bool net_admin)
 {
 	struct sctp_infox *infox = (struct sctp_infox *)info;
 
-- 
2.43.0


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

* [RFC net-next v11 2/2] mptcp: hide seq counters without net_admin
  2026-10-28  8:26         ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters Kalpan Jani
  2026-10-28  8:27           ` [RFC net-next v11 1/2] inet_diag: pass net_admin to get_info Kalpan Jani
@ 2026-10-28  8:27           ` Kalpan Jani
  2026-10-28  9:32           ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters MPTCP CI
  2 siblings, 0 replies; 21+ messages in thread
From: Kalpan Jani @ 2026-10-28  8:27 UTC (permalink / raw)
  To: mptcp
  Cc: matttbe, Kalpan Jani, Mat Martineau, Geliang Tang,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, linux-kernel

The MPTCP inet-diag response includes data sequence numbers that
should not be exposed to users without CAP_NET_ADMIN.

Clear mptcpi_write_seq, mptcpi_snd_una and mptcpi_rcv_nxt in diagnostic
responses for unprivileged users. Apply this restriction in the
inet-diag callback, leaving the MPTCP_INFO getsockopt path unchanged.

Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
---
 net/mptcp/mptcp_diag.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index 9ed7a6be1e628..970f849f4345c 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -215,6 +215,12 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
 		return;
 
 	mptcp_diag_fill_info(msk, info);
+
+	if (!net_admin) {
+		info->mptcpi_write_seq = 0;
+		info->mptcpi_snd_una = 0;
+		info->mptcpi_rcv_nxt = 0;
+	}
 }
 
 static const struct inet_diag_handler mptcp_diag_handler = {
-- 
2.43.0


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

* Re: [RFC net-next v11 1/2] inet_diag: pass net_admin to get_info
  2026-10-28  8:27           ` [RFC net-next v11 1/2] inet_diag: pass net_admin to get_info Kalpan Jani
@ 2026-10-28  8:31             ` netdev-bot+sinfo
  0 siblings, 0 replies; 21+ messages in thread
From: netdev-bot+sinfo @ 2026-10-28  8:31 UTC (permalink / raw)
  To: Kalpan Jani
  Cc: mptcp, matttbe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Neal Cardwell, Kuniyuki Iwashima,
	Mat Martineau, Geliang Tang, Marcelo Ricardo Leitner, Xin Long,
	Kees Cook, David Ahern, Willem de Bruijn, netdev, linux-kernel,
	linux-sctp

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

* Re: [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters
  2026-10-28  8:26         ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters Kalpan Jani
  2026-10-28  8:27           ` [RFC net-next v11 1/2] inet_diag: pass net_admin to get_info Kalpan Jani
  2026-10-28  8:27           ` [RFC net-next v11 2/2] mptcp: hide seq counters without net_admin Kalpan Jani
@ 2026-10-28  9:32           ` MPTCP CI
  2 siblings, 0 replies; 21+ messages in thread
From: MPTCP CI @ 2026-10-28  9:32 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp

Hi Kalpan,

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/40658455981

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


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] 21+ messages in thread

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27 13:42       ` Matthieu Baerts
  2026-10-28  8:26         ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters Kalpan Jani
@ 2026-10-28  9:43         ` Matthieu Baerts
  2026-10-28 10:33           ` Kalpan Jani
  1 sibling, 1 reply; 21+ messages in thread
From: Matthieu Baerts @ 2026-10-28  9:43 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp, martineau, pabeni

Hi Kalpan,

On 27/10/2026 14:42, Matthieu Baerts wrote:
> Hi Kalpan,
> 
> Thank you for your reply.
> 
> On 27/10/2026 13:29, Kalpan Jani wrote:
>> Hi Matt,
>>
>> I investigated the existing byte counters and the diagnostic
>> capability path.
>>
>> The existing byte counters are not direct replacements for the
>> normalized sequence fields:
>>
>> - write_seq advances when application data is queued and when
>> DATA_FIN is added. bytes_sent tracks send progress, and the
>> retransmission path can update snd_nxt directly without updating
>> bytes_sent. - snd_una is assigned directly during connection setup,
>> outside __mptcp_snd_una_update(). Therefore, bytes_acked cannot be
>> assumed to reproduce snd_una - local_idsn. - ack_seq advances when
>> processing DATA_FIN, whereas bytes_received counts payload bytes.
>> Their initial values also differ.
> 
> OK, let's not use the bytes counters then.
> 
>> Regarding the capability check, the normal MPTCP diagnostic dump
>> paths have access to the original request skb and can check
>> netlink_net_capable(cb->skb, CAP_NET_ADMIN).
> 
> Good!
> 
>> However, inet_diag_handler_get_info() is also called for socket-
>> destruction notifications. In that path,
>> sock_diag_broadcast_destroy_work() creates a new skb and invokes the
>> callback without an original diagnostic request. Therefore, the
>> requester's capability cannot be recovered from that skb. Passing
>> false in this notification path appears to be the appropriate
>> conservative behavior.
> 
> OK to pass false in this case.
> 
>> Based on this investigation, I will keep the capability/privacy
>> change separate from sequence normalization. I will also address the
>> stale remote_idsn after disconnect and the inconsistent sequence
>> snapshot reported by Sashiko before sending another normalization
>> revision.
> 
> Just to be sure it is clear: the main goal here is not to show the seq
> counters to everybody from the same netns. This can be done either by
> "normalising" them -- i.e. starting from 0 -- or by not presenting them
> to some users.
> 
> Just thinking about that: for the normalisation, we can also just
> add/remove an offset the userspace cannot predict and different per
> connection. In fact, why not using the address of the msk pointer? I
> guess that should prevent the different issues reported by Sashiko, no?

Was this workaround using the msk address not working?

BTW, please next time don't send new series as a reply to an email,
start a new email thread instead.

> (In case of "other" existing issues, please split them and move them in
> a dedicated patch not to mix the fix and the main patch here.)
> 
> If the "normalisation" is not possible, better not displaying them to
> non cap-admin users. But we don't need both.
> 
> Cheers,
> Matt

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


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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-28  9:43         ` [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Matthieu Baerts
@ 2026-10-28 10:33           ` Kalpan Jani
  2026-10-28 10:41             ` Matthieu Baerts
  0 siblings, 1 reply; 21+ messages in thread
From: Kalpan Jani @ 2026-10-28 10:33 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp, martineau, pabeni

Hi Matt,

No, I haven't tried the msk-address offset approach yet.

I sent RFC v11 with the alternative approach of hiding the sequence fields from users without CAP_NET_ADMIN, based on your previous message. I did not mean to imply that the pointer-offset approach had been tried and found not to work.

The two patches applied and built successfully against my local mptcp/net-next tree, and both passed checkpatch. However, I haven't done a runtime test of the access restriction or tested the pointer-offset approach.

I'll look into the pointer-offset option before deciding whether another revision is needed. Since v11 has already been sent, I'll wait for your feedback before posting any further changes.

I also understand your point about email threading. I sent v11 as a reply to the existing thread, but I'll start a new thread for future patch series.

Sorry for the confusion.

Cheers,
Kalpan Jani


From: Matthieu Baerts <matttbe@kernel.org>
To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com>
Cc: "mptcp"<mptcp@lists.linux.dev>, "martineau"<martineau@kernel.org>, "pabeni"<pabeni@redhat.com>
Date: Wed, 28 Oct 2026 15:13:05 +0530
Subject: Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info

 > Hi Kalpan,
 > 
 > On 27/10/2026 14:42, Matthieu Baerts wrote:
 > > Hi Kalpan,
 > > 
 > > Thank you for your reply.
 > > 
 > > On 27/10/2026 13:29, Kalpan Jani wrote:
 > >> Hi Matt,
 > >>
 > >> I investigated the existing byte counters and the diagnostic
 > >> capability path.
 > >>
 > >> The existing byte counters are not direct replacements for the
 > >> normalized sequence fields:
 > >>
 > >> - write_seq advances when application data is queued and when
 > >> DATA_FIN is added. bytes_sent tracks send progress, and the
 > >> retransmission path can update snd_nxt directly without updating
 > >> bytes_sent. - snd_una is assigned directly during connection setup,
 > >> outside __mptcp_snd_una_update(). Therefore, bytes_acked cannot be
 > >> assumed to reproduce snd_una - local_idsn. - ack_seq advances when
 > >> processing DATA_FIN, whereas bytes_received counts payload bytes.
 > >> Their initial values also differ.
 > > 
 > > OK, let's not use the bytes counters then.
 > > 
 > >> Regarding the capability check, the normal MPTCP diagnostic dump
 > >> paths have access to the original request skb and can check
 > >> netlink_net_capable(cb->skb, CAP_NET_ADMIN).
 > > 
 > > Good!
 > > 
 > >> However, inet_diag_handler_get_info() is also called for socket-
 > >> destruction notifications. In that path,
 > >> sock_diag_broadcast_destroy_work() creates a new skb and invokes the
 > >> callback without an original diagnostic request. Therefore, the
 > >> requester's capability cannot be recovered from that skb. Passing
 > >> false in this notification path appears to be the appropriate
 > >> conservative behavior.
 > > 
 > > OK to pass false in this case.
 > > 
 > >> Based on this investigation, I will keep the capability/privacy
 > >> change separate from sequence normalization. I will also address the
 > >> stale remote_idsn after disconnect and the inconsistent sequence
 > >> snapshot reported by Sashiko before sending another normalization
 > >> revision.
 > > 
 > > Just to be sure it is clear: the main goal here is not to show the seq
 > > counters to everybody from the same netns. This can be done either by
 > > "normalising" them -- i.e. starting from 0 -- or by not presenting them
 > > to some users.
 > > 
 > > Just thinking about that: for the normalisation, we can also just
 > > add/remove an offset the userspace cannot predict and different per
 > > connection. In fact, why not using the address of the msk pointer? I
 > > guess that should prevent the different issues reported by Sashiko, no?
 > 
 > Was this workaround using the msk address not working?
 > 
 > BTW, please next time don't send new series as a reply to an email,
 > start a new email thread instead.
 > 
 > > (In case of "other" existing issues, please split them and move them in
 > > a dedicated patch not to mix the fix and the main patch here.)
 > > 
 > > If the "normalisation" is not possible, better not displaying them to
 > > non cap-admin users. But we don't need both.
 > > 
 > > Cheers,
 > > Matt
 > 
 > Cheers,
 > Matt
 > -- 
 > Sponsored by the NGI0 Core fund.
 > 
 > 



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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-28 10:33           ` Kalpan Jani
@ 2026-10-28 10:41             ` Matthieu Baerts
  2026-10-28 11:36               ` Kalpan Jani
  2026-10-29 10:05               ` Kalpan Jani
  0 siblings, 2 replies; 21+ messages in thread
From: Matthieu Baerts @ 2026-10-28 10:41 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp, martineau, pabeni

On 28/10/2026 11:33, Kalpan Jani wrote:> No, I haven't tried the
msk-address offset approach yet.
> 
> I sent RFC v11 with the alternative approach of hiding the sequence
> fields from users without CAP_NET_ADMIN, based on your previous
> message. I did not mean to imply that the pointer-offset approach had
> been tried and found not to work.
> 
> The two patches applied and built successfully against my local mptcp/
> net-next tree, and both passed checkpatch. However, I haven't done a
> runtime test of the access restriction or tested the pointer-offset
> approach.

Always better to have a runtime validation, especially here where you
Cc'ed netdev and the net maintainers ;)
(BTW, I think we can wait to settle on a working design before involving
them.)

> I'll look into the pointer-offset option before deciding whether
> another revision is needed. Since v11 has already been sent, I'll
> wait for your feedback before posting any further changes.

If this approach works, we could eventually consider that as a fix and
backport that.

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


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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-28 10:41             ` Matthieu Baerts
@ 2026-10-28 11:36               ` Kalpan Jani
  2026-10-29 10:05               ` Kalpan Jani
  1 sibling, 0 replies; 21+ messages in thread
From: Kalpan Jani @ 2026-10-28 11:36 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp, martineau, pabeni

Thanks, Matt.

Understood. I'll validate the `msk`-address offset approach first, including runtime testing, before deciding on another revision.

I've applied the approach in an isolated worktree, and the modified `mptcp_diag.o` builds successfully with `W=1`. Runtime validation is still pending.

If the results confirm that the approach works correctly and avoids exposing absolute sequence numbers, I'll share the findings so we can discuss whether it is suitable as a fix and for backporting.

I'll hold off on sending another patch series until the design has been validated.

Thanks for the guidance.

Cheers,
Kalpan Jani


From: Matthieu Baerts <matttbe@kernel.org>
To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com>
Cc: "mptcp"<mptcp@lists.linux.dev>, "martineau"<martineau@kernel.org>, "pabeni"<pabeni@redhat.com>
Date: Wed, 28 Oct 2026 16:11:38 +0530
Subject: Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info

 > On 28/10/2026 11:33, Kalpan Jani wrote:> No, I haven't tried the
 > msk-address offset approach yet.
 > > 
 > > I sent RFC v11 with the alternative approach of hiding the sequence
 > > fields from users without CAP_NET_ADMIN, based on your previous
 > > message. I did not mean to imply that the pointer-offset approach had
 > > been tried and found not to work.
 > > 
 > > The two patches applied and built successfully against my local mptcp/
 > > net-next tree, and both passed checkpatch. However, I haven't done a
 > > runtime test of the access restriction or tested the pointer-offset
 > > approach.
 > 
 > Always better to have a runtime validation, especially here where you
 > Cc'ed netdev and the net maintainers ;)
 > (BTW, I think we can wait to settle on a working design before involving
 > them.)
 > 
 > > I'll look into the pointer-offset option before deciding whether
 > > another revision is needed. Since v11 has already been sent, I'll
 > > wait for your feedback before posting any further changes.
 > 
 > If this approach works, we could eventually consider that as a fix and
 > backport that.
 > 
 > Cheers,
 > Matt
 > -- 
 > Sponsored by the NGI0 Core fund.
 > 
 > 



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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-27  6:27 [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
                   ` (3 preceding siblings ...)
  2026-10-27  9:54 ` Kalpan Jani
@ 2026-10-28 19:41 ` netdev-bot+sashiko
  4 siblings, 0 replies; 21+ messages in thread
From: netdev-bot+sashiko @ 2026-10-28 19:41 UTC (permalink / raw)
  To: kalpan.jani; +Cc: mptcp, shardul.b, janak, kalpanjani009, kuba

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] subflow_get_info() (net/mptcp/diag.c:39-42) adds a new read
  through the subflow context into the parent msk:…
- [Medium] The two places that normalize against msk->remote_idsn
  disagree on when it is valid, and remote_idsn is not reset when the…
- [Low] mptcp_diag_fill_info() (net/mptcp/sockopt.c) reads the local_idsn
  baseline (line 1012) inside a lock_sock_fast() section, releases the…
- [Low] subflow_get_info() (net/mptcp/diag.c:40) treats `sf->map_seq ==
  0` as 'no mapping' and exports 0 without normalizing.

--

mptcp: normalize seq numbers reported in mptcp_info

This patch adds local_idsn and remote_idsn baselines to struct mptcp_sock.
It uses them to normalize mptcpi_write_seq, mptcpi_snd_una and
mptcpi_rcv_nxt, and zeroes those fields in INET_DIAG_INFO for callers
without CAP_NET_ADMIN. It also normalizes MPTCP_SUBFLOW_ATTR_MAP_SEQ
against remote_idsn, and passes a net_admin flag to the idiag_get_info
callbacks.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445

> diff --git a/net/mptcp/diag.c b/net/mptcp/diag.c
> index 70cf9ebce8338..590cdee419ef9 100644
> --- a/net/mptcp/diag.c
> +++ b/net/mptcp/diag.c

[ ... ]

> @@ -35,6 +36,11 @@ static int subflow_get_info(struct sock *sk, struct sk_buff *skb, bool net_admin
>  		goto nla_failure;
>  	}
>  
> +	map_seq = 0;
> +	if (sf->conn && sf->map_seq)
> +		map_seq = sf->map_seq -
> +			  READ_ONCE(mptcp_sk(sf->conn)->remote_idsn);

[Severity: High]
Can this READ_ONCE() read from an mptcp_sock that has already been freed?

Before this patch, subflow_get_info() only read fields of the subflow
context, and that context is freed with kfree_rcu(). The new line follows
sf->conn into the parent msk. Nothing in the diag path seems to guarantee
that the ctx still holds its reference on sf->conn at this point.

The dump path reaches here like this:

inet_diag_dump_icsk()
  sk_diag_fill()          /* ssk held only by refcount, ehash lock dropped */
    inet_sk_diag_fill()
      tcp_diag_get_aux()  /* reads icsk->icsk_ulp_ops without the lock */
        tcp_diag_put_ulp()
          subflow_get_info()
            lock_sock_fast(sk);
            sf = rcu_dereference(inet_csk(sk)->icsk_ulp_data);

Suppose the subflow is destroyed between the icsk_ulp_ops read and
lock_sock_fast(). The teardown then runs:

tcp_done()
  inet_csk_destroy_sock()
    tcp_v4_destroy_sock()
      tcp_cleanup_ulp()
        subflow_ulp_release()

and subflow_ulp_release() does:

	sk = ctx->conn;
	if (sk) {
		...
		sock_put(sk);
	}

	mptcp_subflow_ops_undo_override(ssk);
	if (release)
		kfree_rcu(ctx, rcu);

tcp_cleanup_ulp() only clears icsk->icsk_ulp_ops. icsk_ulp_data and
ctx->conn are left in place, and subflow_get_info() does not check
icsk_ulp_ops again after it takes the lock.

If the user has already closed the msk, that sock_put() can drop the last
reference. Connected msks are not SOCK_RCU_FREE, because
mptcp_sk_clone_init() does:

	sock_reset_flag(nsk, SOCK_RCU_FREE);

So the rcu_read_lock() in subflow_get_info() would not keep the msk alive.

This read also happens before the net_admin check, so an unprivileged
dump such as ss -ti could reach it. A CAP_NET_ADMIN requester would also
get the stale value back as MAP_SEQ.

[Severity: Medium]
Should this be gated on msk->can_ack, as mptcp_diag_fill_info() does?

mptcp_diag_fill_info() only uses remote_idsn for mptcpi_rcv_nxt when
can_ack is set:

	if (READ_ONCE(msk->can_ack))
		remote_idsn = READ_ONCE(msk->remote_idsn);

Here, remote_idsn is subtracted whenever sf->map_seq is non-zero.

remote_idsn is also not reset on disconnect. mptcp_disconnect() clears
can_ack and ack_seq:

	WRITE_ONCE(msk->can_ack, false);
	...
	/* for fallback's sake */
	WRITE_ONCE(msk->ack_seq, 0);

local_idsn and remote_idsn are only zeroed in __mptcp_init_sock(), which
does not run on that path.

Consider this sequence:

socket(IPPROTO_MPTCP)
connect() to an MPTCP peer     /* subflow_set_remote_key() sets remote_idsn */
connect(AF_UNSPEC)             /* mptcp_disconnect() */
connect() to a non-MPTCP peer  /* fallback, remote key never set */

The first subflow's iasn and map_seq were reset, so in fallback
subflow_check_data_avail() rebuilds map_seq starting from iasn == 0:

	subflow->map_seq = __mptcp_expand_seq(subflow->map_seq,
					      subflow->iasn +
					      TCP_SKB_CB(skb)->seq -
					      subflow->ssn_offset - 1);

MAP_SEQ would then be a small value minus the stale remote_idsn, which
wraps. For the same socket, mptcpi_rcv_nxt is reported as ack_seq - 0.

That seems to contradict this part of the commit message:

    Normalize MPTCP_SUBFLOW_ATTR_MAP_SEQ against remote_idsn as well, so
    subflow mapping sequence numbers use the same remote sequence space as
    mptcpi_rcv_nxt.

Should mptcp_disconnect() also reset local_idsn and remote_idsn?

[Severity: Low]
Is sf->map_seq == 0 a reliable test for "no mapping"?

get_mapping_status() stores whatever mptcp_expand_seq() returns and marks
the mapping valid:

	subflow->map_seq = map_seq;
	subflow->map_subflow_seq = mpext->subflow_seq;
	subflow->map_data_len = data_len;
	subflow->map_valid = 1;

So once the 64-bit data sequence space wraps, a valid mapping can start
at absolute DSN 0. With remote_idsn = 2^64 - k, that mapping's relative
sequence is k, but MAP_SEQ would be reported as 0. The peer picks its own
key, so it can make its IDSN close to 2^64.

Could the check use something other than the numeric sequence value?

>  	if (sf->mp_capable)
>  		flags |= MPTCP_SUBFLOW_FLAG_MCAP_REM;
>  	if (sf->request_mptcp)

[ ... ]

> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index fcf6feb2a9eb5..b1e41c967157e 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c

[ ... ]

> @@ -1008,9 +1009,11 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  	info->mptcpi_flags = flags;
>  
>  	slow = lock_sock_fast(sk);
> +	local_idsn = READ_ONCE(msk->local_idsn);
> +
>  	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
>  	info->mptcpi_token = msk->token;
> -	info->mptcpi_write_seq = msk->write_seq;
> +	info->mptcpi_write_seq = msk->write_seq - local_idsn;

[ ... ]

> @@ -1023,17 +1026,20 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  	unlock_sock_fast(sk, slow);
>  
>  	mptcp_data_lock(sk);
> +	if (READ_ONCE(msk->can_ack))
> +		remote_idsn = READ_ONCE(msk->remote_idsn);
>  	info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);
> -	info->mptcpi_snd_una = msk->snd_una;
> -	info->mptcpi_rcv_nxt = msk->ack_seq;
> +	info->mptcpi_snd_una = msk->snd_una - local_idsn;
> +	info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn;

[Severity: Low]
Can the baseline and the counter here come from different connections?

local_idsn is read under lock_sock_fast(), the lock is dropped, and
snd_una is read later under mptcp_data_lock(). mptcp_connect() writes all
of these while holding only the msk owner lock:

	WRITE_ONCE(msk->local_idsn, subflow->idsn);
	WRITE_ONCE(msk->write_seq, subflow->idsn);
	WRITE_ONCE(msk->snd_nxt, subflow->idsn);
	WRITE_ONCE(msk->snd_una, subflow->idsn);

Holding the owner lock does not exclude a data lock holder.

Suppose another thread calls connect() between the two critical sections
on a fresh socket. The reader then holds local_idsn == 0 and reports the
absolute IDSN as mptcpi_snd_una. Across a disconnect and reconnect, the
result is the difference between two unrelated baselines.

mptcp_disconnect() also clears can_ack, and later ack_seq, without the
data lock. mptcpi_rcv_nxt can therefore combine can_ack == false with the
old ack_seq, or the old remote_idsn with ack_seq == 0.

mptcp_getsockopt_info() calls mptcp_diag_fill_info() without the socket
lock and copies the result out as is. Could the baselines and the
counters be sampled under one consistent lock?

>  	info->mptcpi_bytes_acked = msk->bytes_acked;
>  	mptcp_data_unlock(sk);
> +
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261027062750.2095515-1-kalpan.jani%40mpiricsoftware.com

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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-28 10:41             ` Matthieu Baerts
  2026-10-28 11:36               ` Kalpan Jani
@ 2026-10-29 10:05               ` Kalpan Jani
  2026-10-29 11:29                 ` Matthieu Baerts
  1 sibling, 1 reply; 21+ messages in thread
From: Kalpan Jani @ 2026-10-29 10:05 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: mptcp, martineau, pabeni, Mpiric Private Limited, Shardul B,
	Kalpanjani009

Hi Matt,

Following up on the V11 patch, I've proceeded with the runtime validation of the submitted approach.

Could V11 be considered a candidate fix for this issue, subject to your review? If you think the current approach is suitable, I'd be happy to proceed with the next steps.

Thanks for your guidance.

Cheers,  
Kalpan Jani

From: Matthieu Baerts <matttbe@kernel.org>
To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com>
Cc: "mptcp"<mptcp@lists.linux.dev>, "martineau"<martineau@kernel.org>, "pabeni"<pabeni@redhat.com>
Date: Wed, 28 Oct 2026 16:11:38 +0530
Subject: Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info

 > On 28/10/2026 11:33, Kalpan Jani wrote:> No, I haven't tried the
 > msk-address offset approach yet.
 > > 
 > > I sent RFC v11 with the alternative approach of hiding the sequence
 > > fields from users without CAP_NET_ADMIN, based on your previous
 > > message. I did not mean to imply that the pointer-offset approach had
 > > been tried and found not to work.
 > > 
 > > The two patches applied and built successfully against my local mptcp/
 > > net-next tree, and both passed checkpatch. However, I haven't done a
 > > runtime test of the access restriction or tested the pointer-offset
 > > approach.
 > 
 > Always better to have a runtime validation, especially here where you
 > Cc'ed netdev and the net maintainers ;)
 > (BTW, I think we can wait to settle on a working design before involving
 > them.)
 > 
 > > I'll look into the pointer-offset option before deciding whether
 > > another revision is needed. Since v11 has already been sent, I'll
 > > wait for your feedback before posting any further changes.
 > 
 > If this approach works, we could eventually consider that as a fix and
 > backport that.
 > 
 > Cheers,
 > Matt
 > -- 
 > Sponsored by the NGI0 Core fund.
 > 
 > 



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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-29 10:05               ` Kalpan Jani
@ 2026-10-29 11:29                 ` Matthieu Baerts
  2026-10-29 11:31                   ` Matthieu Baerts
  0 siblings, 1 reply; 21+ messages in thread
From: Matthieu Baerts @ 2026-10-29 11:29 UTC (permalink / raw)
  To: Kalpan Jani
  Cc: mptcp, martineau, pabeni, Mpiric Private Limited, Shardul B,
	Kalpanjani009

Hi Kalpan,

On 29/10/2026 11:05, Kalpan Jani wrote:
> Following up on the V11 patch, I've proceeded with the runtime
> validation of the submitted approach.

Thanks, good to know it can be an alternative.

> Could V11 be considered a candidate fix for this issue, subject to
> your review? If you think the current approach is suitable, I'd be
> happy to proceed with the next steps.

For me, the goal is still the same: the main goal here is not to show
the seq counters to everybody. This can be done either by adding an
offset -- i.e. starting from 0 or something else -- or by not presenting
them to some users. It seems better to normalise them, than reporting 0,
especially when looking at the v11 RFC modifying code a bit everywhere
"just for MPTCP": other net maintainers might not accept that.

So as mentioned before: try the normalisation/offset first, then if that
causes more troubles, hide the value to non-CAP_NET_ADMIN users but
that's more a workaround that might be hard to backport.

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


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

* Re: [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-29 11:29                 ` Matthieu Baerts
@ 2026-10-29 11:31                   ` Matthieu Baerts
  0 siblings, 0 replies; 21+ messages in thread
From: Matthieu Baerts @ 2026-10-29 11:31 UTC (permalink / raw)
  To: Kalpan Jani
  Cc: mptcp, martineau, pabeni, Mpiric Private Limited, Shardul B,
	Kalpanjani009

On 29/10/2026 12:29, Matthieu Baerts wrote:
> Hi Kalpan,
> 
> On 29/10/2026 11:05, Kalpan Jani wrote:
>> Following up on the V11 patch, I've proceeded with the runtime
>> validation of the submitted approach.
> 
> Thanks, good to know it can be an alternative.
> 
>> Could V11 be considered a candidate fix for this issue, subject to
>> your review? If you think the current approach is suitable, I'd be
>> happy to proceed with the next steps.
> 
> For me, the goal is still the same: the main goal here is not to show
> the seq counters to everybody. This can be done either by adding an
> offset -- i.e. starting from 0 or something else -- or by not presenting
> them to some users. It seems better to normalise them, than reporting 0,
> especially when looking at the v11 RFC modifying code a bit everywhere
> "just for MPTCP": other net maintainers might not accept that.
> 
> So as mentioned before: try the normalisation/offset first, then if that

Just to make sure that's clear: the normalisation (i.e. starting from 0)
seems more complex than initially thought, plus it seems that we need to
store more counters just for that. Using the msk address pointer as
offset seems easier, but we will see once there is a PoC.

> causes more troubles, hide the value to non-CAP_NET_ADMIN users but
> that's more a workaround that might be hard to backport.
> 
> Cheers,
> Matt

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


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

end of thread, other threads:[~2026-10-29 11:31 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27  6:27 [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
2026-10-27  6:51 ` sashiko-bot
2026-10-27  7:33 ` Matthieu Baerts
2026-10-27  7:38 ` MPTCP CI
2026-10-27  9:54 ` Kalpan Jani
2026-10-27 10:45   ` Matthieu Baerts
2026-10-27 12:29     ` Kalpan Jani
2026-10-27 13:42       ` Matthieu Baerts
2026-10-28  8:26         ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters Kalpan Jani
2026-10-28  8:27           ` [RFC net-next v11 1/2] inet_diag: pass net_admin to get_info Kalpan Jani
2026-10-28  8:31             ` netdev-bot+sinfo
2026-10-28  8:27           ` [RFC net-next v11 2/2] mptcp: hide seq counters without net_admin Kalpan Jani
2026-10-28  9:32           ` [RFC net-next v11 0/2] inet_diag: restrict MPTCP sequence counters MPTCP CI
2026-10-28  9:43         ` [RFC net-next v10] mptcp: normalize seq numbers reported in mptcp_info Matthieu Baerts
2026-10-28 10:33           ` Kalpan Jani
2026-10-28 10:41             ` Matthieu Baerts
2026-10-28 11:36               ` Kalpan Jani
2026-10-29 10:05               ` Kalpan Jani
2026-10-29 11:29                 ` Matthieu Baerts
2026-10-29 11:31                   ` Matthieu Baerts
2026-10-28 19:41 ` netdev-bot+sashiko

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.