public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
@ 2026-03-28 21:33 Nikolaos Gkarlis
  2026-03-31 14:43 ` Nikolaos Gkarlis
  2026-04-02  2:45 ` Jakub Kicinski
  0 siblings, 2 replies; 6+ messages in thread
From: Nikolaos Gkarlis @ 2026-03-28 21:33 UTC (permalink / raw)
  To: netdev; +Cc: kuba, nickgarlis, kuniyu

rtnl_newlink() lacks a CAP_NET_ADMIN capability check on the peer
network namespace when creating paired devices (veth, vxcan,
netkit). This allows an unprivileged user with a user namespace
to create interfaces in arbitrary network namespaces, including
init_net.

Add a netlink_ns_capable() check for CAP_NET_ADMIN in the peer
namespace before allowing device creation to proceed.

Fixes: 81adee47dfb6 ("net: Support specifying the network namespace upon device creation.")
Signed-off-by: Nikolaos Gkarlis <nickgarlis@gmail.com>
---
v4:
  - Use IS_ERR_OR_NULL instead of IS_ERR + null check.
v3:
  - Move netlink_ns_capable() check from rtnl_newlink() into
    rtnl_get_peer_net(), after the last rtnl_link_get_net_ifla(tb)
    call. The tbp path is already covered by rtnl_link_get_net_capable()
    in the caller. (suggested by Kuniyuki)
  - Pass skb to rtnl_get_peer_net() for the capability check.
  - Add IS_ERR() check on rtnl_link_get_net_ifla(tb) return value.
v2:
  - Removed "Reported-by" tag
  - Fixed "Fixes" tag with the help of Kuniyuki Iwashima (thanks !)

 net/core/rtnetlink.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index fae8034efbf..a4d8fd8232e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3894,12 +3894,14 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
 	goto out;
 }
 
-static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
+static struct net *rtnl_get_peer_net(struct sk_buff *skb,
+				     const struct rtnl_link_ops *ops,
 				     struct nlattr *tbp[],
 				     struct nlattr *data[],
 				     struct netlink_ext_ack *extack)
 {
 	struct nlattr *tb[IFLA_MAX + 1];
+	struct net *net;
 	int err;
 
 	if (!data || !data[ops->peer_type])
@@ -3915,7 +3917,16 @@ static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
 			return ERR_PTR(err);
 	}
 
-	return rtnl_link_get_net_ifla(tb);
+	net = rtnl_link_get_net_ifla(tb);
+	if (IS_ERR_OR_NULL(net))
+		return net;
+
+	if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
+		put_net(net);
+		return ERR_PTR(-EPERM);
+	}
+
+	return net;
 }
 
 static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -4054,7 +4065,7 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		}
 
 		if (ops->peer_type) {
-			peer_net = rtnl_get_peer_net(ops, tb, data, extack);
+			peer_net = rtnl_get_peer_net(skb, ops, tb, data, extack);
 			if (IS_ERR(peer_net)) {
 				ret = PTR_ERR(peer_net);
 				goto put_ops;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
  2026-03-28 21:33 [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns Nikolaos Gkarlis
@ 2026-03-31 14:43 ` Nikolaos Gkarlis
  2026-04-02  2:45 ` Jakub Kicinski
  1 sibling, 0 replies; 6+ messages in thread
From: Nikolaos Gkarlis @ 2026-03-31 14:43 UTC (permalink / raw)
  To: netdev; +Cc: kuba, kuniyu

Just following up on this patch; is it okay as is, or does anything
need to be addressed?

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
  2026-03-28 21:33 [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns Nikolaos Gkarlis
  2026-03-31 14:43 ` Nikolaos Gkarlis
@ 2026-04-02  2:45 ` Jakub Kicinski
  2026-04-02 17:45   ` Nikolaos Gkarlis
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-04-02  2:45 UTC (permalink / raw)
  To: Nikolaos Gkarlis; +Cc: netdev, kuniyu

On Sat, 28 Mar 2026 22:33:38 +0100 Nikolaos Gkarlis wrote:
> -static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
> +static struct net *rtnl_get_peer_net(struct sk_buff *skb,
> +				     const struct rtnl_link_ops *ops,
>  				     struct nlattr *tbp[],
>  				     struct nlattr *data[],
>  				     struct netlink_ext_ack *extack)
>  {
>  	struct nlattr *tb[IFLA_MAX + 1];
> +	struct net *net;
>  	int err;
>  
>  	if (!data || !data[ops->peer_type])

There's an early return hiding outside of the context here.
the patch is technically correct, I think, because if we take this
shortcut we end up with the same netns as tgt_net so we'll validate
that it's capable later. But it's probably not obvious to a casual
reader of this code (or AI agents, sigh)

So let's rewrite this along the lines of:

        struct nlattr *tb[IFLA_MAX + 1], **attrs;
        struct net *net;
        int err;
 
        if (!data || !data[ops->peer_type]) {
               attrs = tbp;
	} else {
        	err = rtnl_nla_parse_ifinfomsg(tb, data[ops->peer_type], extack);
	        if (err < 0)
        	        return ERR_PTR(err);
 
	        if (ops->validate) {
        	        err = ops->validate(tb, NULL, extack);
                	if (err < 0)
	                        return ERR_PTR(err);
        	}
 
		attrs = tb;
	}

        net = rtnl_link_get_net_ifla(attrs);
        if (IS_ERR_OR_NULL(net))
                return net;
 
        if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
...

?

> @@ -3915,7 +3917,16 @@ static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
>  			return ERR_PTR(err);
>  	}
>  
> -	return rtnl_link_get_net_ifla(tb);
> +	net = rtnl_link_get_net_ifla(tb);
> +	if (IS_ERR_OR_NULL(net))
> +		return net;
> +
> +	if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
> +		put_net(net);
> +		return ERR_PTR(-EPERM);
> +	}
> +
> +	return net;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
  2026-04-02  2:45 ` Jakub Kicinski
@ 2026-04-02 17:45   ` Nikolaos Gkarlis
  2026-04-02 17:52     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 6+ messages in thread
From: Nikolaos Gkarlis @ 2026-04-02 17:45 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, kuniyu

On Thu, Apr 2, 2026 at 4:45 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 28 Mar 2026 22:33:38 +0100 Nikolaos Gkarlis wrote:
> > -static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
> > +static struct net *rtnl_get_peer_net(struct sk_buff *skb,
> > +                                  const struct rtnl_link_ops *ops,
> >                                    struct nlattr *tbp[],
> >                                    struct nlattr *data[],
> >                                    struct netlink_ext_ack *extack)
> >  {
> >       struct nlattr *tb[IFLA_MAX + 1];
> > +     struct net *net;
> >       int err;
> >
> >       if (!data || !data[ops->peer_type])
>
> There's an early return hiding outside of the context here.
> the patch is technically correct, I think, because if we take this
> shortcut we end up with the same netns as tgt_net so we'll validate
> that it's capable later. But it's probably not obvious to a casual
> reader of this code (or AI agents, sigh)
>
> So let's rewrite this along the lines of:
>
>         struct nlattr *tb[IFLA_MAX + 1], **attrs;
>         struct net *net;
>         int err;
>
>         if (!data || !data[ops->peer_type]) {
>                attrs = tbp;
>         } else {
>                 err = rtnl_nla_parse_ifinfomsg(tb, data[ops->peer_type], extack);
>                 if (err < 0)
>                         return ERR_PTR(err);
>
>                 if (ops->validate) {
>                         err = ops->validate(tb, NULL, extack);
>                         if (err < 0)
>                                 return ERR_PTR(err);
>                 }
>
>                 attrs = tb;
>         }
>
>         net = rtnl_link_get_net_ifla(attrs);
>         if (IS_ERR_OR_NULL(net))
>                 return net;
>
>         if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
> ...
>
> ?

I agree it’s a bit confusing with the early exit. I’ll apply the suggested
changes and send a v5.

That said, would it make sense to introduce a separate
rtnl_nets_add_capable() helper that wraps rtnl_nets_add() instead?

Something along these lines:

@@ -334,6 +334,19 @@ static void rtnl_nets_add(struct rtnl_nets
*rtnl_nets, struct net *net)
     rtnl_nets->len++;
 }

+static struct net *rtnl_nets_add_capable(struct sk_buff *skb,
+                                         struct rtnl_nets *rtnl_nets,
+                                         struct net *net)
+{
+    if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
+        put_net(net);
+        return ERR_PTR(-EPERM);
+    }
+
+    rtnl_nets_add(rtnl_nets, net);
+    return net;
+}
+
 static void rtnl_nets_lock(struct rtnl_nets *rtnl_nets)
 {
     int i;
@@ -4059,18 +4072,29 @@ static int rtnl_newlink(struct sk_buff *skb,
struct nlmsghdr *nlh,
         ret = PTR_ERR(peer_net);
         goto put_ops;
     }
-    if (peer_net)
-        rtnl_nets_add(&rtnl_nets, peer_net);
+    if (peer_net) {
+        peer_net = rtnl_nets_add_capable(skb,
+                                        &rtnl_nets,
+                                        peer_net);
+        if (IS_ERR(peer_net)) {
+            ret = PTR_ERR(peer_net);
+            goto put_ops;
+        }
+    }
     }
     }

-    tgt_net = rtnl_link_get_net_capable(skb, sock_net(skb->sk), tb,
CAP_NET_ADMIN);
+   tgt_net = rtnl_link_get_net_by_nlattr(sock_net(skb->sk), tb);
     if (IS_ERR(tgt_net)) {
         ret = PTR_ERR(tgt_net);
         goto put_net;
     }

-    rtnl_nets_add(&rtnl_nets, tgt_net);
+    tgt_net = rtnl_nets_add_capable(skb, &rtnl_nets, tgt_net);
+    if (IS_ERR(tgt_net)) {
+        ret = PTR_ERR(tgt_net);
+        goto put_net;
+    }

     if (tb[IFLA_LINK_NETNSID]) {
         int id = nla_get_s32(tb[IFLA_LINK_NETNSID]);
@@ -4082,10 +4106,10 @@ static int rtnl_newlink(struct sk_buff *skb,
struct nlmsghdr *nlh,
         goto put_net;
     }

-    rtnl_nets_add(&rtnl_nets, link_net);
-
-    if (!netlink_ns_capable(skb, link_net->user_ns, CAP_NET_ADMIN)) {
-        ret = -EPERM;
+    link_net = rtnl_nets_add_capable(skb, &rtnl_nets,
+                                    link_net);
+    if (IS_ERR(link_net)) {
+        ret = PTR_ERR(link_net);
         goto put_net;
     }
     }

Note that this changes the order in link_net, checking capabilities
before creating the object.

Disclaimer: I’m not very familiar with this code, so this may be a bad idea.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
  2026-04-02 17:45   ` Nikolaos Gkarlis
@ 2026-04-02 17:52     ` Kuniyuki Iwashima
  2026-04-02 18:17       ` Nikolaos Gkarlis
  0 siblings, 1 reply; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-04-02 17:52 UTC (permalink / raw)
  To: Nikolaos Gkarlis; +Cc: Jakub Kicinski, netdev

On Thu, Apr 2, 2026 at 10:46 AM Nikolaos Gkarlis <nickgarlis@gmail.com> wrote:
>
> On Thu, Apr 2, 2026 at 4:45 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Sat, 28 Mar 2026 22:33:38 +0100 Nikolaos Gkarlis wrote:
> > > -static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
> > > +static struct net *rtnl_get_peer_net(struct sk_buff *skb,
> > > +                                  const struct rtnl_link_ops *ops,
> > >                                    struct nlattr *tbp[],
> > >                                    struct nlattr *data[],
> > >                                    struct netlink_ext_ack *extack)
> > >  {
> > >       struct nlattr *tb[IFLA_MAX + 1];
> > > +     struct net *net;
> > >       int err;
> > >
> > >       if (!data || !data[ops->peer_type])
> >
> > There's an early return hiding outside of the context here.
> > the patch is technically correct, I think, because if we take this
> > shortcut we end up with the same netns as tgt_net so we'll validate
> > that it's capable later. But it's probably not obvious to a casual
> > reader of this code (or AI agents, sigh)
> >
> > So let's rewrite this along the lines of:
> >
> >         struct nlattr *tb[IFLA_MAX + 1], **attrs;
> >         struct net *net;
> >         int err;
> >
> >         if (!data || !data[ops->peer_type]) {
> >                attrs = tbp;
> >         } else {
> >                 err = rtnl_nla_parse_ifinfomsg(tb, data[ops->peer_type], extack);
> >                 if (err < 0)
> >                         return ERR_PTR(err);
> >
> >                 if (ops->validate) {
> >                         err = ops->validate(tb, NULL, extack);
> >                         if (err < 0)
> >                                 return ERR_PTR(err);
> >                 }
> >
> >                 attrs = tb;
> >         }
> >
> >         net = rtnl_link_get_net_ifla(attrs);
> >         if (IS_ERR_OR_NULL(net))
> >                 return net;
> >
> >         if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
> > ...
> >
> > ?
>
> I agree it’s a bit confusing with the early exit. I’ll apply the suggested
> changes and send a v5.
>
> That said, would it make sense to introduce a separate
> rtnl_nets_add_capable() helper that wraps rtnl_nets_add() instead?

Let's focus on the fix in net.git.

Also, I don't want to hide rtnl_nets_add() in a helper.


>
> Something along these lines:
>
> @@ -334,6 +334,19 @@ static void rtnl_nets_add(struct rtnl_nets
> *rtnl_nets, struct net *net)
>      rtnl_nets->len++;
>  }
>
> +static struct net *rtnl_nets_add_capable(struct sk_buff *skb,
> +                                         struct rtnl_nets *rtnl_nets,
> +                                         struct net *net)
> +{
> +    if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
> +        put_net(net);
> +        return ERR_PTR(-EPERM);
> +    }
> +
> +    rtnl_nets_add(rtnl_nets, net);
> +    return net;
> +}
> +
>  static void rtnl_nets_lock(struct rtnl_nets *rtnl_nets)
>  {
>      int i;
> @@ -4059,18 +4072,29 @@ static int rtnl_newlink(struct sk_buff *skb,
> struct nlmsghdr *nlh,
>          ret = PTR_ERR(peer_net);
>          goto put_ops;
>      }
> -    if (peer_net)
> -        rtnl_nets_add(&rtnl_nets, peer_net);
> +    if (peer_net) {
> +        peer_net = rtnl_nets_add_capable(skb,
> +                                        &rtnl_nets,
> +                                        peer_net);
> +        if (IS_ERR(peer_net)) {
> +            ret = PTR_ERR(peer_net);
> +            goto put_ops;
> +        }
> +    }
>      }
>      }
>
> -    tgt_net = rtnl_link_get_net_capable(skb, sock_net(skb->sk), tb,
> CAP_NET_ADMIN);
> +   tgt_net = rtnl_link_get_net_by_nlattr(sock_net(skb->sk), tb);
>      if (IS_ERR(tgt_net)) {
>          ret = PTR_ERR(tgt_net);
>          goto put_net;
>      }
>
> -    rtnl_nets_add(&rtnl_nets, tgt_net);
> +    tgt_net = rtnl_nets_add_capable(skb, &rtnl_nets, tgt_net);
> +    if (IS_ERR(tgt_net)) {
> +        ret = PTR_ERR(tgt_net);
> +        goto put_net;
> +    }
>
>      if (tb[IFLA_LINK_NETNSID]) {
>          int id = nla_get_s32(tb[IFLA_LINK_NETNSID]);
> @@ -4082,10 +4106,10 @@ static int rtnl_newlink(struct sk_buff *skb,
> struct nlmsghdr *nlh,
>          goto put_net;
>      }
>
> -    rtnl_nets_add(&rtnl_nets, link_net);
> -
> -    if (!netlink_ns_capable(skb, link_net->user_ns, CAP_NET_ADMIN)) {
> -        ret = -EPERM;
> +    link_net = rtnl_nets_add_capable(skb, &rtnl_nets,
> +                                    link_net);
> +    if (IS_ERR(link_net)) {
> +        ret = PTR_ERR(link_net);
>          goto put_net;
>      }
>      }
>
> Note that this changes the order in link_net, checking capabilities
> before creating the object.
>
> Disclaimer: I’m not very familiar with this code, so this may be a bad idea.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
  2026-04-02 17:52     ` Kuniyuki Iwashima
@ 2026-04-02 18:17       ` Nikolaos Gkarlis
  0 siblings, 0 replies; 6+ messages in thread
From: Nikolaos Gkarlis @ 2026-04-02 18:17 UTC (permalink / raw)
  To: Kuniyuki Iwashima; +Cc: Jakub Kicinski, netdev

> Let's focus on the fix in net.git.

> Also, I don't want to hide rtnl_nets_add() in a helper.

I've now submitted v5. Thanks for reviewing, and apologies
for the detour.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-02 18:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-28 21:33 [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns Nikolaos Gkarlis
2026-03-31 14:43 ` Nikolaos Gkarlis
2026-04-02  2:45 ` Jakub Kicinski
2026-04-02 17:45   ` Nikolaos Gkarlis
2026-04-02 17:52     ` Kuniyuki Iwashima
2026-04-02 18:17       ` Nikolaos Gkarlis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox