rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.

[PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.

Posted by Maoyi Xie 1 week, 5 days ago

Commit 11b326fb0a37 ("ip6: vti: Use ip6_tnl.net in
vti6_changelink().") made vti6_changelink() and vti6_update()
mutate the vti6 hash of the device's creation netns. The
rtnetlink path into changelink never checks CAP_NET_ADMIN
against that netns. The only capability check on the link netns,
netlink_ns_capable() against link_net->user_ns, runs solely when
the RTM_NEWLINK message carries IFLA_LINK_NETNSID. A plain
"ip link set <name> type vti6 ..." does not carry it.

So an unprivileged user holding a migrated vti6 device can
rewrite an entry in the creation netns vti6 hash. They pick the
endpoint addresses. Commit 8b484efd5cb4 ("ip6: vti: Use
ip6_tnl.net in vti6_siocdevprivate().") already closed the
SIOCCHGTUNNEL path. This patch closes the RTM_NEWLINK path.

Other link_types are affected too. Any type that publishes
get_link_net and whose changelink touches t->net has the same
gap: ipip, gre, sit, ip_vti, ip6_tnl, ip6_gre, xfrm_interface.

Check netlink_ns_capable(CAP_NET_ADMIN) against the device's
link netns before dispatching to rtnl_changelink(). Types
without get_link_net are unaffected. The newlink path has long
checked capability in the link netns. The changelink path never
did.

Reported-by: Xiao Liang <shaw.leon@gmail.com>
Closes: https://lore.kernel.org/netdev/CABAhCOSzP1vaThGV35_VnsRCb=87_CPjPVsTHbq905k8A+BuUg@mail.gmail.com/
Fixes: 06615bed60c1 ("net: Verify permission to link_net in newlink")
Cc: stable@vger.kernel.org
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 net/core/rtnetlink.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index df042da422ef..ac7a3bf438d5 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3969,8 +3969,26 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		dev = NULL;
 	}
 
-	if (dev)
+	if (dev) {
+		/* changelink may mutate the link's creation netns.
+		 * rtnl_link_get_net_capable() above only checked
+		 * tgt_net. When the creation netns differs, also
+		 * require CAP_NET_ADMIN there. Otherwise a migrated
+		 * device lets a caller with caps only in its current
+		 * netns mutate the creation netns.
+		 */
+		if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
+			struct net *dev_link_net;
+
+			dev_link_net = dev->rtnl_link_ops->get_link_net(dev);
+			if (!net_eq(dev_link_net, tgt_net) &&
+			    !netlink_ns_capable(skb, dev_link_net->user_ns,
+						CAP_NET_ADMIN))
+				return -EPERM;
+		}
+
 		return rtnl_changelink(skb, nlh, ops, dev, tgt_net, tbs, data, extack);
+	}
 
 	if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
 		/* No dev found and NLM_F_CREATE not set. Requested dev does not exist,
-- 
2.34.1

Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.

Posted by Kuniyuki Iwashima 1 week, 5 days ago

On Wed, May 27, 2026 at 12:08 AM Maoyi Xie <maoyixie.tju@gmail.com> wrote:
>
> Commit 11b326fb0a37 ("ip6: vti: Use ip6_tnl.net in
> vti6_changelink().") made vti6_changelink() and vti6_update()
> mutate the vti6 hash of the device's creation netns. The
> rtnetlink path into changelink never checks CAP_NET_ADMIN
> against that netns. The only capability check on the link netns,
> netlink_ns_capable() against link_net->user_ns, runs solely when
> the RTM_NEWLINK message carries IFLA_LINK_NETNSID. A plain
> "ip link set <name> type vti6 ..." does not carry it.
>
> So an unprivileged user holding a migrated vti6 device can
> rewrite an entry in the creation netns vti6 hash. They pick the
> endpoint addresses. Commit 8b484efd5cb4 ("ip6: vti: Use
> ip6_tnl.net in vti6_siocdevprivate().") already closed the
> SIOCCHGTUNNEL path. This patch closes the RTM_NEWLINK path.
>
> Other link_types are affected too. Any type that publishes
> get_link_net and whose changelink touches t->net has the same
> gap: ipip, gre, sit, ip_vti, ip6_tnl, ip6_gre, xfrm_interface.
>
> Check netlink_ns_capable(CAP_NET_ADMIN) against the device's
> link netns before dispatching to rtnl_changelink(). Types
> without get_link_net are unaffected. The newlink path has long
> checked capability in the link netns. The changelink path never
> did.
>
> Reported-by: Xiao Liang <shaw.leon@gmail.com>
> Closes: https://lore.kernel.org/netdev/CABAhCOSzP1vaThGV35_VnsRCb=87_CPjPVsTHbq905k8A+BuUg@mail.gmail.com/
> Fixes: 06615bed60c1 ("net: Verify permission to link_net in newlink")
> Cc: stable@vger.kernel.org
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
> ---
>  net/core/rtnetlink.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index df042da422ef..ac7a3bf438d5 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -3969,8 +3969,26 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>                 dev = NULL;
>         }
>
> -       if (dev)
> +       if (dev) {
> +               /* changelink may mutate the link's creation netns.
> +                * rtnl_link_get_net_capable() above only checked
> +                * tgt_net. When the creation netns differs, also
> +                * require CAP_NET_ADMIN there. Otherwise a migrated
> +                * device lets a caller with caps only in its current
> +                * netns mutate the creation netns.
> +                */
> +               if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
> +                       struct net *dev_link_net;
> +
> +                       dev_link_net = dev->rtnl_link_ops->get_link_net(dev);
> +                       if (!net_eq(dev_link_net, tgt_net) &&
> +                           !netlink_ns_capable(skb, dev_link_net->user_ns,
> +                                               CAP_NET_ADMIN))
> +                               return -EPERM;

Do all other callers of ->get_link_net(), dev_get_iflink_dev()
and batadv_getlink_net(), require the same capability check ?


> +               }
> +
>                 return rtnl_changelink(skb, nlh, ops, dev, tgt_net, tbs, data, extack);
> +       }
>
>         if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
>                 /* No dev found and NLM_F_CREATE not set. Requested dev does not exist,
> --
> 2.34.1
>

Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.

Posted by Maoyi Xie 1 week, 5 days ago

Hi Kuniyuki,

Thanks for looking.

> Do all other callers of ->get_link_net(), dev_get_iflink_dev()
> and batadv_getlink_net(), require the same capability check ?

No. Those are read paths. get_link_net feeds IFLA_LINK_NETNSID, the
iflink lookup feeds IFLA_LINK, and batadv_getlink_net resolves a hard
interface's parent netns. None of them mutates state, so none needs a
capability check.

But your question points at a real problem in my patch. get_link_net
is the wrong gate. For the ip tunnels and xfrmi it returns t->net, the
netns changelink mutates, so the check is right there. For peer types
like netkit and veth it returns the peer netns instead. netkit has a
changelink, and its peer usually lives in another netns. My patch
would then require CAP_NET_ADMIN in the peer netns for a plain change
to a netkit device, which netkit does not require today.

So the check belongs in the changelink path of the types that mutate
t->net, against t->net->user_ns. That mirrors the ioctl side in
8b484efd5cb4. I will send a v2 along those lines.

Thanks,
Maoyi

Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.

Posted by Kuniyuki Iwashima 1 week, 5 days ago

On Wed, May 27, 2026 at 2:43 AM Maoyi Xie <maoyixie.tju@gmail.com> wrote:
>
> Hi Kuniyuki,
>
> Thanks for looking.
>
> > Do all other callers of ->get_link_net(), dev_get_iflink_dev()
> > and batadv_getlink_net(), require the same capability check ?
>
> No. Those are read paths.

See how netif_change_proto_down() uses dev_get_iflink_dev().


> get_link_net feeds IFLA_LINK_NETNSID, the
> iflink lookup feeds IFLA_LINK, and batadv_getlink_net resolves a hard
> interface's parent netns. None of them mutates state, so none needs a
> capability check.
>
> But your question points at a real problem in my patch. get_link_net
> is the wrong gate. For the ip tunnels and xfrmi it returns t->net, the
> netns changelink mutates, so the check is right there. For peer types
> like netkit and veth it returns the peer netns instead. netkit has a
> changelink, and its peer usually lives in another netns. My patch
> would then require CAP_NET_ADMIN in the peer netns for a plain change
> to a netkit device, which netkit does not require today.
>
> So the check belongs in the changelink path of the types that mutate
> t->net, against t->net->user_ns. That mirrors the ioctl side in
> 8b484efd5cb4. I will send a v2 along those lines.
>
> Thanks,
> Maoyi

Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.

Posted by Maoyi Xie 1 week, 3 days ago

Hi Kuniyuki,

> > > Do all other callers of ->get_link_net(), dev_get_iflink_dev()
> > > and batadv_getlink_net(), require the same capability check ?
> >
> > No. Those are read paths.
>
> See how netif_change_proto_down() uses dev_get_iflink_dev().

Thanks for catching that. You're right, "all read paths" was too
broad. netif_change_proto_down() is a mutation function and it calls
dev_get_iflink_dev() inside its logic.

I read through it. The resolved iflink_dev is only used there to test
reachability (the !iflink_dev return) and to read
netif_carrier_ok(iflink_dev) for the carrier_on conditional. The
mutations (proto_down, carrier_off/on) target dev, which is in the
caller's netns and was cap checked at the rtnl setlink entry. So I do
not see a parallel cap gap on that path.

If you agree, I would like to keep this series scoped to the
rtnl_changelink path Xiao reported. The per-type cap check on
t->net->user_ns mirrors 8b484efd5cb4. If you see another angle on the
dev_get_iflink_dev() callers, please tell me and I will look again.

Thanks,
Maoyi