[PATCH net v2 0/4] tipc: fix netlink gate and receive-path bugs

[PATCH net v2 0/4] tipc: fix netlink gate and receive-path bugs

[Michael Bommarito]

This is v2 of the public TIPC series. Patches 1 and 2 are the same fixes
as before, with patch 1 updated to use GENL_ADMIN_PERM for
TIPC_NL_MEDIA_SET after the duplicate public patch pointed out that the
media defaults are global rather than per-netns state. Patches 3 and 4
address Tung's review feedback.

Changes in v2:

  - Patch 1 uses GENL_ADMIN_PERM for TIPC_NL_MEDIA_SET and
    GENL_UNS_ADMIN_PERM for the netns-scoped mutators.
  - Patch 3 validates msg_conn_ack() at the start of the CONN_ACK block
    and drops invalid messages instead of capping the value.
  - Patch 4 reorders the new u32 declarations in reverse-Xmas-tree order.

Michael Bommarito (4):
  tipc: require net admin for TIPCv2 netlink mutators
  tipc: validate discovery message length before reading media address
  tipc: prevent snt_unacked underflow on CONN_ACK
  tipc: reject inverted service ranges from peer bindings

 net/tipc/discover.c   | 14 ++++++++++++++
 net/tipc/name_distr.c | 11 ++++++++++-
 net/tipc/netlink.c    | 12 ++++++++++++
 net/tipc/socket.c     |  7 ++++++-
 4 files changed, 42 insertions(+), 2 deletions(-)


base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
-- 
2.53.0

[PATCH net v2 1/4] tipc: require net admin for TIPCv2 netlink mutators

[Michael Bommarito]

TIPCv2 registers mutating generic-netlink operations without admin
permission flags. Generic netlink only checks CAP_NET_ADMIN when an
operation sets GENL_ADMIN_PERM or GENL_UNS_ADMIN_PERM, so a local
unprivileged process can currently change TIPC state through commands
such as TIPC_NL_NET_SET, TIPC_NL_KEY_SET, TIPC_NL_KEY_FLUSH, and
bearer enable/disable.

The legacy TIPC netlink API already checks netlink_net_capable(...,
CAP_NET_ADMIN) for administrative commands. Give the TIPCv2 mutators
the equivalent generic-netlink gate. Use GENL_UNS_ADMIN_PERM for
network-namespace scoped operations and GENL_ADMIN_PERM for
TIPC_NL_MEDIA_SET, which updates the shared media defaults rather than
state owned only by the target network namespace.

A QEMU/KASAN repro run as uid/gid 65534 with zero effective
capabilities previously succeeded in changing the network id and node
identity, setting and flushing key material, and enabling/disabling a
UDP bearer. With this patch applied the same operations fail with
-EPERM.

Fixes: 0655f6a8635b ("tipc: add bearer disable/enable to new netlink api")
Link: https://lore.kernel.org/all/20260604163102.2658553-1-dominik.czarnota@trailofbits.com/
Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2:
- Use GENL_ADMIN_PERM for TIPC_NL_MEDIA_SET because it updates global
  media defaults, while keeping GENL_UNS_ADMIN_PERM for netns-scoped
  mutators.

 net/tipc/netlink.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c
index 1a9a5bdaccf4f..5bbe134284acc 100644
--- a/net/tipc/netlink.c
+++ b/net/tipc/netlink.c
@@ -152,11 +152,13 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_BEARER_DISABLE,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_bearer_disable,
 	},
 	{
 		.cmd	= TIPC_NL_BEARER_ENABLE,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_bearer_enable,
 	},
 	{
@@ -168,11 +170,13 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_BEARER_ADD,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_bearer_add,
 	},
 	{
 		.cmd	= TIPC_NL_BEARER_SET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_bearer_set,
 	},
 	{
@@ -197,11 +201,13 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_LINK_SET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_node_set_link,
 	},
 	{
 		.cmd	= TIPC_NL_LINK_RESET_STATS,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit   = tipc_nl_node_reset_link_stats,
 	},
 	{
@@ -213,6 +219,7 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_MEDIA_SET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_ADMIN_PERM,
 		.doit	= tipc_nl_media_set,
 	},
 	{
@@ -228,6 +235,7 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_NET_SET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_net_set,
 	},
 	{
@@ -238,6 +246,7 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_MON_SET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_node_set_monitor,
 	},
 	{
@@ -255,6 +264,7 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_PEER_REMOVE,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_peer_rm,
 	},
 #ifdef CONFIG_TIPC_MEDIA_UDP
@@ -269,11 +279,13 @@ static const struct genl_ops tipc_genl_v2_ops[] = {
 	{
 		.cmd	= TIPC_NL_KEY_SET,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_node_set_key,
 	},
 	{
 		.cmd	= TIPC_NL_KEY_FLUSH,
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.flags	= GENL_UNS_ADMIN_PERM,
 		.doit	= tipc_nl_node_flush_key,
 	},
 #endif
-- 
2.53.0

[PATCH net v2 2/4] tipc: validate discovery message length before reading media address

[Michael Bommarito]

tipc_disc_rcv() reads the sender's media address from the fixed
media-info area of the header (msg_media_addr(), offset
TIPC_MEDIA_INFO_OFFSET) and, when the peer advertises 128-bit node
ids, copies a NODE_ID_LEN node id appended after the header. Neither
read is bounded against the actual received length: tipc_msg_validate()
only enforces a header size in the range [MIN_H_SIZE, MAX_H_SIZE], so a
LINK_CONFIG message as short as MIN_H_SIZE (24 bytes) passes validation
while the media-address read reaches up to MAX_H_SIZE and the node-id
read reaches MAX_H_SIZE + NODE_ID_LEN.

A node always builds discovery messages at MAX_H_SIZE + NODE_ID_LEN
(tipc_disc_init_msg()), so a shorter LINK_CONFIG message is malformed.
Drop such messages before the reads so the media address and node id
are taken from received data rather than from uninitialised tail room
or memory beyond the buffer.

A crafted short LINK_CONFIG datagram otherwise makes tipc_disc_rcv()
read past the received message data when a bearer is enabled.

Fixes: 3d749a6a26b0 ("tipc: Hide media-specific addressing details from generic bearer code")
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 net/tipc/discover.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/tipc/discover.c b/net/tipc/discover.c
index 3e54d2df5683a..daf5f11fc82b4 100644
--- a/net/tipc/discover.c
+++ b/net/tipc/discover.c
@@ -217,6 +217,20 @@ void tipc_disc_rcv(struct net *net, struct sk_buff *skb,
 	}
 	hdr = buf_msg(skb);
 
+	/* A discovery message carries the sender's media address within the
+	 * fixed-size header and, when 128-bit ids are advertised, a node id
+	 * appended after it.  A node always builds these messages at
+	 * MAX_H_SIZE + NODE_ID_LEN, so drop anything too short to hold what
+	 * is read below and keep msg2addr() and the node-id copy within the
+	 * received data.
+	 */
+	if (skb->len < MAX_H_SIZE ||
+	    ((caps & TIPC_NODE_ID128) && skb->len < MAX_H_SIZE + NODE_ID_LEN)) {
+		pr_warn_ratelimited("Rcv corrupt discovery message\n");
+		kfree_skb(skb);
+		return;
+	}
+
 	if (caps & TIPC_NODE_ID128)
 		memcpy(peer_id, msg_node_id(hdr), NODE_ID_LEN);
 	else
-- 
2.53.0

[PATCH net v2 3/4] tipc: prevent snt_unacked underflow on CONN_ACK

[Michael Bommarito]

tipc_sk_conn_proto_rcv() subtracts the peer-supplied connection ack count
from the unsigned 16-bit send counter snt_unacked without checking that it
does not exceed the number of messages actually outstanding:

	tsk->snt_unacked -= msg_conn_ack(hdr);

msg_conn_ack() is read straight from a received CONN_MANAGER/CONN_ACK
message. If the ack count is larger than snt_unacked, the subtraction
wraps to a near-maximum value, leaving tsk_conn_cong() permanently true
and starving the connection of further transmits.

Validate the ACK count at the start of the CONN_ACK block and drop the
message if it acknowledges more messages than are outstanding. A peer (or,
for a local connection, the connected peer socket) can otherwise wedge a
TIPC connection's send side by sending an oversized connection ack.

Fixes: 10724cc7bb78 ("tipc: redesign connection-level flow control")
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2:
- Validate msg_conn_ack() at the beginning of the CONN_ACK block and
  drop invalid messages instead of capping the peer-supplied value.

 net/tipc/socket.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 9329919fb07f0..80ad973cda16e 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1362,9 +1362,14 @@ static void tipc_sk_conn_proto_rcv(struct tipc_sock *tsk, struct sk_buff *skb,
 			__skb_queue_tail(xmitq, skb);
 		return;
 	} else if (mtyp == CONN_ACK) {
+		u16 conn_ack = msg_conn_ack(hdr);
+
+		if (conn_ack > tsk->snt_unacked)
+			goto exit;
+
 		was_cong = tsk_conn_cong(tsk);
 		tipc_sk_push_backlog(tsk, msg_nagle_ack(hdr));
-		tsk->snt_unacked -= msg_conn_ack(hdr);
+		tsk->snt_unacked -= conn_ack;
 		if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL)
 			tsk->snd_win = msg_adv_win(hdr);
 		if (was_cong && !tsk_conn_cong(tsk))
-- 
2.53.0

[PATCH net v2 4/4] tipc: reject inverted service ranges from peer bindings

[Michael Bommarito]

tipc_update_nametbl() inserts a binding advertised by a peer node using
the lower and upper service-range bounds taken directly from the wire,
without checking that lower <= upper. The local bind path validates the
ordering (tipc_uaddr_valid()), but the name-distribution path does not.

A binding with lower > upper is inserted at the far end of the
service-range rbtree (keyed on lower) where no lookup or withdrawal can
ever match it (service_range_foreach_match() requires sr->lower <= end).
The publication, its service_range node and the augmented rbtree entry
are then leaked for the lifetime of the namespace, and there is no
per-peer cap equivalent to TIPC_MAX_PUBL on locally created bindings.

Reject inverted ranges in the network path as well. A peer node can
otherwise leak unbounded binding-table memory by sending PUBLICATION
items with lower > upper.

Fixes: 37922ea4a310 ("tipc: permit overlapping service ranges in name table")
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2:
- Reorder the new u32 declarations in reverse-Xmas-tree order.

 net/tipc/name_distr.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 190b49c5cbc3e..da8a5eb6e63f1 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -282,10 +282,19 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
 	struct publication *p = NULL;
 	struct tipc_socket_addr sk;
 	struct tipc_uaddr ua;
+	u32 lower = ntohl(i->lower);
+	u32 upper = ntohl(i->upper);
 	u32 key = ntohl(i->key);
 
+	/* A peer-advertised binding with lower > upper can never be matched
+	 * or withdrawn and would leak the publication; the local bind path
+	 * rejects such ranges, so reject ranges learned from the network too.
+	 */
+	if (lower > upper)
+		return false;
+
 	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE,
-		   ntohl(i->type), ntohl(i->lower), ntohl(i->upper));
+		   ntohl(i->type), lower, upper);
 	sk.ref = ntohl(i->port);
 	sk.node = node;
 
-- 
2.53.0