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

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

[Michael Bommarito]

This is v3 of the public TIPC series. The discovery-message length
patch (was patch 2/4) is dropped: tipc_msg_validate() already rejects
the short messages it was guarding against, so it was redundant. The
remaining three patches are unchanged in behaviour from v2 and address
the receive-path review feedback.

Patch 1 gives the TIPCv2 mutating generic-netlink operations the admin
gate the legacy API already has, so a local unprivileged process can no
longer change TIPC state. Patch 2 drops CONN_ACK messages that
acknowledge more outstanding sends than exist, preventing the
snt_unacked underflow. Patch 3 rejects peer bindings with lower > upper,
which would otherwise leak binding-table memory.

Changes in v3:

  - Drop the discovery-message length patch; tipc_msg_validate()
    already rejects the short messages it guarded against (Tung Quang
    Nguyen).
  - Patch 2 (snt_unacked): drop the conn_ack local and test
    tsk->snt_unacked against msg_conn_ack() inline (Tung Quang Nguyen).
  - Patch 3 (inverted ranges): restructure the declaration block, moving
    ua below key at the maintainer's request (Tung Quang Nguyen).

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 2 validates msg_conn_ack() at the start of the CONN_ACK block
    and drops invalid messages instead of capping the value.
  - Patch 3 reorders the new u32 declarations in reverse-Xmas-tree order.

Michael Bommarito (3):
  tipc: require net admin for TIPCv2 netlink mutators
  tipc: prevent snt_unacked underflow on CONN_ACK
  tipc: reject inverted service ranges from peer bindings

 net/tipc/name_distr.c | 13 +++++++++++--
 net/tipc/netlink.c    | 12 ++++++++++++
 net/tipc/socket.c     |  3 +++
 3 files changed, 26 insertions(+), 2 deletions(-)


base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
-- 
2.53.0

[PATCH net v3 1/3] 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

RE: [PATCH net v3 1/3] tipc: require net admin for TIPCv2 netlink mutators

[Tung Quang Nguyen]

>Subject: [PATCH net v3 1/3] tipc: require net admin for TIPCv2 netlink mutators
>
>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.

Given the legacy TIPC netlink uses netlink_net_capable(), we should keep the same behavior
for TIPC netlink (i.e, not using GENL_ADMIN_PERM) to better support user namespaces or containers.

>
>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 v3 2/3] 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>
---
v3:
- Drop the conn_ack local; test tsk->snt_unacked against
  msg_conn_ack() inline (Tung Quang Nguyen).

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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 9329919fb07f0..f64f7a35b5c91 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1362,6 +1362,9 @@ 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) {
+		if (tsk->snt_unacked < msg_conn_ack(hdr))
+			goto exit;
+
 		was_cong = tsk_conn_cong(tsk);
 		tipc_sk_push_backlog(tsk, msg_nagle_ack(hdr));
 		tsk->snt_unacked -= msg_conn_ack(hdr);
-- 
2.53.0

[PATCH net v3 3/3] 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>
---
v3:
- Restructure the declaration block (move ua below key) at the
  maintainer's request (Tung Quang Nguyen).

v2:
- Reorder the new u32 declarations in reverse-Xmas-tree order.

 net/tipc/name_distr.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 190b49c5cbc3e..ba4f4906e13b7 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -280,12 +280,21 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
 				u32 node, u32 dtype)
 {
 	struct publication *p = NULL;
+	u32 lower = ntohl(i->lower);
+	u32 upper = ntohl(i->upper);
 	struct tipc_socket_addr sk;
-	struct tipc_uaddr ua;
 	u32 key = ntohl(i->key);
+	struct tipc_uaddr ua;
+
+	/* 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