* [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.
@ 2026-05-27 7:08 Maoyi Xie
2026-05-27 7:17 ` Kuniyuki Iwashima
0 siblings, 1 reply; 5+ messages in thread
From: Maoyi Xie @ 2026-05-27 7:08 UTC (permalink / raw)
To: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet
Cc: David Ahern, Kuniyuki Iwashima, Xiao Liang, Nikolaos Gkarlis,
netdev, linux-kernel, Maoyi Xie, stable
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.
2026-05-27 7:08 [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink Maoyi Xie
@ 2026-05-27 7:17 ` Kuniyuki Iwashima
2026-05-27 9:43 ` Maoyi Xie
0 siblings, 1 reply; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-27 7:17 UTC (permalink / raw)
To: Maoyi Xie
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
David Ahern, Xiao Liang, Nikolaos Gkarlis, netdev, linux-kernel,
stable
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.
2026-05-27 7:17 ` Kuniyuki Iwashima
@ 2026-05-27 9:43 ` Maoyi Xie
2026-05-27 16:57 ` Kuniyuki Iwashima
0 siblings, 1 reply; 5+ messages in thread
From: Maoyi Xie @ 2026-05-27 9:43 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
David Ahern, Xiao Liang, Nikolaos Gkarlis, netdev, linux-kernel,
stable
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.
2026-05-27 9:43 ` Maoyi Xie
@ 2026-05-27 16:57 ` Kuniyuki Iwashima
2026-05-29 6:52 ` Maoyi Xie
0 siblings, 1 reply; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-27 16:57 UTC (permalink / raw)
To: Maoyi Xie
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
David Ahern, Xiao Liang, Nikolaos Gkarlis, netdev, linux-kernel,
stable
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink.
2026-05-27 16:57 ` Kuniyuki Iwashima
@ 2026-05-29 6:52 ` Maoyi Xie
0 siblings, 0 replies; 5+ messages in thread
From: Maoyi Xie @ 2026-05-29 6:52 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, Eric Dumazet,
David Ahern, Xiao Liang, Nikolaos Gkarlis, netdev, linux-kernel,
stable
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-29 6:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-27 7:08 [PATCH net] rtnetlink: Require CAP_NET_ADMIN in link netns for changelink Maoyi Xie
2026-05-27 7:17 ` Kuniyuki Iwashima
2026-05-27 9:43 ` Maoyi Xie
2026-05-27 16:57 ` Kuniyuki Iwashima
2026-05-29 6:52 ` Maoyi Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®