RE: [PATCH net] tipc: require admin privileges for TIPCv2 mutators

[Tung Quang Nguyen]

>Subject: Re: [PATCH net] tipc: require admin privileges for TIPCv2 mutators
>
>Hi,
>
>On Fri, Jun 5, 2026 at 9:33 AM Tung Quang Nguyen
><tung.quang.nguyen@xxxxxxxx> wrote:
>>
>> >Subject: [PATCH net] tipc: require admin privileges for TIPCv2
>> >mutators
>> >
>> >TIPCv2 registers its generic-netlink mutating commands without
>> >permission flags. Generic netlink only checks CAP_NET_ADMIN when an
>> >operation carries GENL_ADMIN_PERM or GENL_UNS_ADMIN_PERM, so
>callers
>> >without CAP_NET_ADMIN can invoke these setters.
>> >
>> >This allows an unprivileged caller to change TIPC control-plane
>> >state, including bearer, link, network, monitor, peer and crypto-key state.
>> >TIPC_NL_MEDIA_SET is broader because it updates the shared media
>> >defaults returned by tipc_media_find(), including UDP MTU, priority,
>> >tolerance and window values, which can affect later bearer setup.
>> >
>> >Require CAP_NET_ADMIN for all TIPCv2 mutators. Use
>> >GENL_UNS_ADMIN_PERM for network-namespace scoped state, matching
>the
>> >legacy compat path and normal netns administration semantics. Use
>> >GENL_ADMIN_PERM for TIPC_NL_MEDIA_SET because it mutates global
>media
>> >defaults rather than state owned only by the target net namespace.
>>
>> A similar patch has been submitted using the same AI:
>> https://patchwork.kernel.org/project/netdevbpf/patch/20260602133555.76
>> 9727-2-michael.bommarito@xxxxxxxxx/
>>
>> Your patch suggests using GENL_ADMIN_PERM while above patch does not.
>> Did AI suggest using GENL_ADMIN_PERM or you suggested it yourself ?
>>
>
>Yeah, the GENL_ADMIN_PERM came from AI. The reasoning is that
>TIPC_NL_MEDIA_SET changes media_info entries within the global
>media_info_array [0, 1] so I believe it should require CAP_NET_ADMIN in
>init_user_ns rather than the userns owning the target netns.

There are many other global variables changed by TIPC_NL_BEARER_SET,  TIPC_NL_BEARER_DISABLE,
TIPC_NL_LINK_SET, TIPC_NL_NODE_SET etc. Given  the legacy TIPC netlink uses netlink_net_capable(),
I think it is better if we keep the same behavior for TIPC netlink (i.e, not using GENL_ADMIN_PERM) to better
support user namespaces or containers.

>
>However, since the legacy compat TIPC netlink path can reach the
>__tipc_nl_media_set functionality through the TIPC_CMD_SET_LINK_*
>operations, I agree that the GENL_UNS_ADMIN_PERM is the right flag for this
>fix. With my patch the TIPCv2's TIPC_NL_MEDIA_SET operation would be
>stricter than the compat path and one could still reach it by entering a new
>userns and using the legacy compat path.
>
>That said, the global nature of the `media_info_array` warrants a separate
>discussion: should both v1 and v2 paths require CAP_NET_ADMIN in
>init_user_ns for the media link operations since global data is modified? If you
>agree, I think this could be handled in a separate patch.
>
>[0] https://elixir.bootlin.com/linux/v7.0.11/source/net/tipc/bearer.c#L51-L60
>[1] https://elixir.bootlin.com/linux/v7.0.11/source/net/tipc/bearer.c#L1328-
>L1342
>
>> >
>> >Fixes: 0655f6a8635b ("tipc: add bearer disable/enable to new netlink
>> >api")
>> >Assisted-by: Codex:gpt-5.5-cyber-preview
>> >Signed-off-by: Dominik 'Disconnect3d' Czarnota
>> ><dominik.czarnota@xxxxxxxxxxxxxxx>
>> >---
>> > net/tipc/netlink.c | 12 ++++++++++++
>> > 1 file changed, 12 insertions(+)
>> >
>> >diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c index
>> >1a9a5bdaccf4..5bbe134284ac 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
>>