[PATCH net v2] net: require CAP_NET_ADMIN in the device netns for tunnel changelink

Maoyi Xie posted 1 patch 1 week ago
There is a newer version of this series
net/ipv4/ip_tunnel.c           | 3 +++
net/ipv6/ip6_gre.c             | 6 ++++++
net/ipv6/ip6_tunnel.c          | 3 +++
net/ipv6/ip6_vti.c             | 3 +++
net/xfrm/xfrm_interface_core.c | 3 +++
5 files changed, 18 insertions(+)
[PATCH net v2] net: require CAP_NET_ADMIN in the device netns for tunnel changelink
Posted by Maoyi Xie 1 week ago
A tunnel changelink mutates the tunnel hash of the device's creation
netns. ip_tunnel_changelink(), ip6_tnl_changelink(), vti6_changelink(),
ip6gre_changelink(), ip6erspan_changelink() and xfrmi_changelink() all
look up and update through t->net.

The rtnl path into changelink only checks CAP_NET_ADMIN against
tgt_net. After IFLA_NET_NS_FD migration the creation netns differs from
the caller's netns. A caller with caps only in its current netns can
then rewrite an entry in the creation netns hash. They pick the
endpoint addresses. Commit 8b484efd5cb4 ("ip6: vti: Use ip6_tnl.net in
vti6_siocdevprivate().") added the same check on the ioctl path. This
adds it on the RTM_NEWLINK path.

Check ns_capable(t->net->user_ns, CAP_NET_ADMIN) in each changelink
before the lookup and update. The newlink path has long checked the
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: d0f418516022 ("net, ip_tunnel: fix namespaces move")
Fixes: 5311a69aaca3 ("net, ip6_tunnel: fix namespaces move")
Fixes: 690afc165bb3 ("net: ip6_gre: fix moving ip6gre between namespaces")
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Fixes: 11b326fb0a37 ("ip6: vti: Use ip6_tnl.net in vti6_changelink().")
Cc: stable@vger.kernel.org
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
v2: Reworked per Kuniyuki Iwashima's review. v1 gated the check on
    dev->rtnl_link_ops->get_link_net in __rtnl_newlink(). That gate is
    too broad. For peer types like netkit and veth get_link_net returns
    the peer netns, which changelink does not mutate, so the core check
    would wrongly require CAP_NET_ADMIN there. Move the check into the
    changelink path of the tunnel types that mutate t->net, against
    t->net->user_ns. This mirrors the ioctl side in 8b484efd5cb4.

v1: https://lore.kernel.org/netdev/20260527070824.2677331-1-maoyixie.tju@gmail.com/

 net/ipv4/ip_tunnel.c           | 3 +++
 net/ipv6/ip6_gre.c             | 6 ++++++
 net/ipv6/ip6_tunnel.c          | 3 +++
 net/ipv6/ip6_vti.c             | 3 +++
 net/xfrm/xfrm_interface_core.c | 3 +++
 5 files changed, 18 insertions(+)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 50d0f5fe4e4c..51d8787318f3 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -1251,6 +1251,9 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct net *net = tunnel->net;
 	struct ip_tunnel_net *itn = net_generic(net, tunnel->ip_tnl_net_id);
 
+	if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (dev == itn->fb_tunnel_dev)
 		return -EINVAL;
 
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 365b4059eb20..0de4994bc92f 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -2047,6 +2047,9 @@ static int ip6gre_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct ip6gre_net *ign = net_generic(t->net, ip6gre_net_id);
 	struct __ip6_tnl_parm p;
 
+	if (!ns_capable(t->net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
 	t = ip6gre_changelink_common(dev, tb, data, &p, extack);
 	if (IS_ERR(t))
 		return PTR_ERR(t);
@@ -2266,6 +2269,9 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct __ip6_tnl_parm p;
 	struct ip6gre_net *ign;
 
+	if (!ns_capable(t->net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
 	ign = net_generic(t->net, ip6gre_net_id);
 	t = ip6gre_changelink_common(dev, tb, data, &p, extack);
 	if (IS_ERR(t))
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 9d1037ac082f..2834004c7011 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -2102,6 +2102,9 @@ static int ip6_tnl_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
 	struct ip_tunnel_encap ipencap;
 
+	if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (dev == ip6n->fb_tnl_dev) {
 		if (ip_tunnel_netlink_encap_parms(data, &ipencap)) {
 			/* iproute2 always sets TUNNEL_ENCAP_FLAG_CSUM6, so
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index df793c8bfffb..7b05e0c491db 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -1044,6 +1044,9 @@ static int vti6_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct __ip6_tnl_parm p;
 	struct vti6_net *ip6n;
 
+	if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
 	ip6n = net_generic(net, vti6_net_id);
 	if (dev == ip6n->fb_tnl_dev)
 		return -EINVAL;
diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
index 330a05286a56..a1029a829406 100644
--- a/net/xfrm/xfrm_interface_core.c
+++ b/net/xfrm/xfrm_interface_core.c
@@ -869,6 +869,9 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct net *net = xi->net;
 	struct xfrm_if_parms p = {};
 
+	if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
 	xfrmi_netlink_parms(data, &p);
 	if (!p.if_id) {
 		NL_SET_ERR_MSG(extack, "if_id must be non zero");
-- 
2.34.1
Re: [PATCH net v2] net: require CAP_NET_ADMIN in the device netns for tunnel changelink
Posted by Kuniyuki Iwashima 3 days, 23 hours ago
On Sun, May 31, 2026 at 8:41 PM Maoyi Xie <maoyixie.tju@gmail.com> wrote:
>
> A tunnel changelink mutates the tunnel hash of the device's creation
> netns. ip_tunnel_changelink(), ip6_tnl_changelink(), vti6_changelink(),
> ip6gre_changelink(), ip6erspan_changelink() and xfrmi_changelink() all
> look up and update through t->net.
>
> The rtnl path into changelink only checks CAP_NET_ADMIN against
> tgt_net. After IFLA_NET_NS_FD migration the creation netns differs from
> the caller's netns. A caller with caps only in its current netns can
> then rewrite an entry in the creation netns hash. They pick the
> endpoint addresses. Commit 8b484efd5cb4 ("ip6: vti: Use ip6_tnl.net in
> vti6_siocdevprivate().") added the same check on the ioctl path. This
> adds it on the RTM_NEWLINK path.
>
> Check ns_capable(t->net->user_ns, CAP_NET_ADMIN) in each changelink
> before the lookup and update. The newlink path has long checked the
> 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: d0f418516022 ("net, ip_tunnel: fix namespaces move")
> Fixes: 5311a69aaca3 ("net, ip6_tunnel: fix namespaces move")
> Fixes: 690afc165bb3 ("net: ip6_gre: fix moving ip6gre between namespaces")
> Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
> Fixes: 11b326fb0a37 ("ip6: vti: Use ip6_tnl.net in vti6_changelink().")
> Cc: stable@vger.kernel.org
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
> ---
> v2: Reworked per Kuniyuki Iwashima's review. v1 gated the check on
>     dev->rtnl_link_ops->get_link_net in __rtnl_newlink(). That gate is
>     too broad. For peer types like netkit and veth get_link_net returns
>     the peer netns, which changelink does not mutate, so the core check
>     would wrongly require CAP_NET_ADMIN there. Move the check into the
>     changelink path of the tunnel types that mutate t->net, against
>     t->net->user_ns. This mirrors the ioctl side in 8b484efd5cb4.
>
> v1: https://lore.kernel.org/netdev/20260527070824.2677331-1-maoyixie.tju@gmail.com/
>
>  net/ipv4/ip_tunnel.c           | 3 +++
>  net/ipv6/ip6_gre.c             | 6 ++++++
>  net/ipv6/ip6_tunnel.c          | 3 +++
>  net/ipv6/ip6_vti.c             | 3 +++
>  net/xfrm/xfrm_interface_core.c | 3 +++
>  5 files changed, 18 insertions(+)
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 50d0f5fe4e4c..51d8787318f3 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -1251,6 +1251,9 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct net *net = tunnel->net;
>         struct ip_tunnel_net *itn = net_generic(net, tunnel->ip_tnl_net_id);
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))

Some attributes might have already changed before calling
ip_tunnel_changelink().

e.g. ipgre_netlink_parms() updates t->collect_md, which will
be visible once the device owner calls changelink.

Also, check net_eq(net, dev_net(dev)) to avoid unnecessary
LSM invocations.


> +               return -EPERM;
> +
>         if (dev == itn->fb_tunnel_dev)
>                 return -EINVAL;
>
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 365b4059eb20..0de4994bc92f 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -2047,6 +2047,9 @@ static int ip6gre_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct ip6gre_net *ign = net_generic(t->net, ip6gre_net_id);
>         struct __ip6_tnl_parm p;
>
> +       if (!ns_capable(t->net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         t = ip6gre_changelink_common(dev, tb, data, &p, extack);
>         if (IS_ERR(t))
>                 return PTR_ERR(t);
> @@ -2266,6 +2269,9 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct __ip6_tnl_parm p;
>         struct ip6gre_net *ign;
>
> +       if (!ns_capable(t->net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         ign = net_generic(t->net, ip6gre_net_id);
>         t = ip6gre_changelink_common(dev, tb, data, &p, extack);
>         if (IS_ERR(t))
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index 9d1037ac082f..2834004c7011 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -2102,6 +2102,9 @@ static int ip6_tnl_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
>         struct ip_tunnel_encap ipencap;
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         if (dev == ip6n->fb_tnl_dev) {
>                 if (ip_tunnel_netlink_encap_parms(data, &ipencap)) {
>                         /* iproute2 always sets TUNNEL_ENCAP_FLAG_CSUM6, so
> diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
> index df793c8bfffb..7b05e0c491db 100644
> --- a/net/ipv6/ip6_vti.c
> +++ b/net/ipv6/ip6_vti.c
> @@ -1044,6 +1044,9 @@ static int vti6_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct __ip6_tnl_parm p;
>         struct vti6_net *ip6n;
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         ip6n = net_generic(net, vti6_net_id);
>         if (dev == ip6n->fb_tnl_dev)
>                 return -EINVAL;
> diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
> index 330a05286a56..a1029a829406 100644
> --- a/net/xfrm/xfrm_interface_core.c
> +++ b/net/xfrm/xfrm_interface_core.c
> @@ -869,6 +869,9 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct net *net = xi->net;
>         struct xfrm_if_parms p = {};
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         xfrmi_netlink_parms(data, &p);
>         if (!p.if_id) {
>                 NL_SET_ERR_MSG(extack, "if_id must be non zero");
> --
> 2.34.1
>
Re: [PATCH net v2] net: require CAP_NET_ADMIN in the device netns for tunnel changelink
Posted by Xiao Liang 6 days, 4 hours ago
On Mon, Jun 1, 2026 at 11:41 AM Maoyi Xie <maoyixie.tju@gmail.com> wrote:
>
> A tunnel changelink mutates the tunnel hash of the device's creation
> netns. ip_tunnel_changelink(), ip6_tnl_changelink(), vti6_changelink(),
> ip6gre_changelink(), ip6erspan_changelink() and xfrmi_changelink() all
> look up and update through t->net.
>
> The rtnl path into changelink only checks CAP_NET_ADMIN against
> tgt_net. After IFLA_NET_NS_FD migration the creation netns differs from
> the caller's netns. A caller with caps only in its current netns can
> then rewrite an entry in the creation netns hash. They pick the
> endpoint addresses. Commit 8b484efd5cb4 ("ip6: vti: Use ip6_tnl.net in
> vti6_siocdevprivate().") added the same check on the ioctl path. This
> adds it on the RTM_NEWLINK path.
>
> Check ns_capable(t->net->user_ns, CAP_NET_ADMIN) in each changelink
> before the lookup and update. The newlink path has long checked the
> 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: d0f418516022 ("net, ip_tunnel: fix namespaces move")
> Fixes: 5311a69aaca3 ("net, ip6_tunnel: fix namespaces move")
> Fixes: 690afc165bb3 ("net: ip6_gre: fix moving ip6gre between namespaces")
> Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
> Fixes: 11b326fb0a37 ("ip6: vti: Use ip6_tnl.net in vti6_changelink().")
> Cc: stable@vger.kernel.org
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
> ---
> v2: Reworked per Kuniyuki Iwashima's review. v1 gated the check on
>     dev->rtnl_link_ops->get_link_net in __rtnl_newlink(). That gate is
>     too broad. For peer types like netkit and veth get_link_net returns
>     the peer netns, which changelink does not mutate, so the core check
>     would wrongly require CAP_NET_ADMIN there. Move the check into the
>     changelink path of the tunnel types that mutate t->net, against
>     t->net->user_ns. This mirrors the ioctl side in 8b484efd5cb4.
>
> v1: https://lore.kernel.org/netdev/20260527070824.2677331-1-maoyixie.tju@gmail.com/
>
>  net/ipv4/ip_tunnel.c           | 3 +++
>  net/ipv6/ip6_gre.c             | 6 ++++++
>  net/ipv6/ip6_tunnel.c          | 3 +++
>  net/ipv6/ip6_vti.c             | 3 +++
>  net/xfrm/xfrm_interface_core.c | 3 +++
>  5 files changed, 18 insertions(+)
>
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 50d0f5fe4e4c..51d8787318f3 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -1251,6 +1251,9 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct net *net = tunnel->net;
>         struct ip_tunnel_net *itn = net_generic(net, tunnel->ip_tnl_net_id);
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +

Should modifying params that don't affect tunnel lookup (e.g. GRE_CSUM
and GRE_SEQ) require CAP_NET_ADMIN in link netns?

>         if (dev == itn->fb_tunnel_dev)
>                 return -EINVAL;
>
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 365b4059eb20..0de4994bc92f 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -2047,6 +2047,9 @@ static int ip6gre_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct ip6gre_net *ign = net_generic(t->net, ip6gre_net_id);
>         struct __ip6_tnl_parm p;
>
> +       if (!ns_capable(t->net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         t = ip6gre_changelink_common(dev, tb, data, &p, extack);
>         if (IS_ERR(t))
>                 return PTR_ERR(t);
> @@ -2266,6 +2269,9 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct __ip6_tnl_parm p;
>         struct ip6gre_net *ign;
>
> +       if (!ns_capable(t->net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         ign = net_generic(t->net, ip6gre_net_id);
>         t = ip6gre_changelink_common(dev, tb, data, &p, extack);
>         if (IS_ERR(t))
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index 9d1037ac082f..2834004c7011 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -2102,6 +2102,9 @@ static int ip6_tnl_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
>         struct ip_tunnel_encap ipencap;
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         if (dev == ip6n->fb_tnl_dev) {
>                 if (ip_tunnel_netlink_encap_parms(data, &ipencap)) {
>                         /* iproute2 always sets TUNNEL_ENCAP_FLAG_CSUM6, so
> diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
> index df793c8bfffb..7b05e0c491db 100644
> --- a/net/ipv6/ip6_vti.c
> +++ b/net/ipv6/ip6_vti.c
> @@ -1044,6 +1044,9 @@ static int vti6_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct __ip6_tnl_parm p;
>         struct vti6_net *ip6n;
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         ip6n = net_generic(net, vti6_net_id);
>         if (dev == ip6n->fb_tnl_dev)
>                 return -EINVAL;
> diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
> index 330a05286a56..a1029a829406 100644
> --- a/net/xfrm/xfrm_interface_core.c
> +++ b/net/xfrm/xfrm_interface_core.c
> @@ -869,6 +869,9 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
>         struct net *net = xi->net;
>         struct xfrm_if_parms p = {};
>
> +       if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> +               return -EPERM;
> +
>         xfrmi_netlink_parms(data, &p);
>         if (!p.if_id) {
>                 NL_SET_ERR_MSG(extack, "if_id must be non zero");
> --
> 2.34.1
>
Re: [PATCH net v2] net: require CAP_NET_ADMIN in the device netns for tunnel changelink
Posted by Maoyi Xie 6 days ago
Hi Xiao,

On Tue, Jun 2, 2026 at 10:10 AM Xiao Liang <shaw.leon@gmail.com> wrote:
> Should modifying params that don't affect tunnel lookup (e.g. GRE_CSUM
> and GRE_SEQ) require CAP_NET_ADMIN in link netns?

ip_tunnel_update() unlinks and relinks the tunnel in the creation
netns hash unconditionally, even for a CSUM or SEQ only change, and
ip6gre_changelink() does the same. So every changelink writes that
hash, not only the ones that change the endpoints. That is why the
check sits at the entry, and it matches the ioctl side in 8b484efd5cb4.

You are right that a CSUM or SEQ only change relinks into the same
bucket and has no lookup effect. I can narrow the check to the params
that move the bucket if you prefer, though that means diffing old and
new params in each changelink. I lean towards the entry check for
consistency, but happy to go either way.

Thanks,
Maoyi