diff mbox series

[v1,net-next,12/13] rtnetlink: Call rtnl_link_get_net_capable() in do_setlink().

Message ID 20241009231656.57830-13-kuniyu@amazon.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series rtnetlink: Refactor rtnl_{new,del,set}link() for per-netns RTNL. | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 6 this patch: 6
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang fail Errors and warnings before: 6 this patch: 10
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success Found: 'put_net(' was: 1 now: 1
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 7 this patch: 7
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 6 this patch: 6
netdev/source_inline success Was 0 now: 0

Commit Message

Kuniyuki Iwashima Oct. 9, 2024, 11:16 p.m. UTC 
We will push RTNL down to rtnl_setlink().

RTM_SETLINK could call rtnl_link_get_net_capable() in do_setlink()
to move a dev to a new netns, but the netns needs to be fetched before
holding rtnl_net_lock().

Let's move it to rtnl_setlink() and pass the netns to do_setlink().

Now, RTM_NEWLINK paths (rtnl_changelink() and rtnl_group_changelink())
can pass the prefetched netns to do_setlink().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/core/rtnetlink.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

Comments

Eric Dumazet Oct. 10, 2024, 1:15 p.m. UTC | #1 
On Thu, Oct 10, 2024 at 1:21 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> We will push RTNL down to rtnl_setlink().
>
> RTM_SETLINK could call rtnl_link_get_net_capable() in do_setlink()
> to move a dev to a new netns, but the netns needs to be fetched before
> holding rtnl_net_lock().
>
> Let's move it to rtnl_setlink() and pass the netns to do_setlink().
>
> Now, RTM_NEWLINK paths (rtnl_changelink() and rtnl_group_changelink())
> can pass the prefetched netns to do_setlink().
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>
kernel test robot Oct. 11, 2024, 7:36 a.m. UTC | #2 
Hi Kuniyuki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/rtnetlink-Allocate-linkinfo-as-struct-rtnl_newlink_tbs/20241010-072158
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241009231656.57830-13-kuniyu%40amazon.com
patch subject: [PATCH v1 net-next 12/13] rtnetlink: Call rtnl_link_get_net_capable() in do_setlink().
config: x86_64-buildonly-randconfig-003-20241011 (https://download.01.org/0day-ci/archive/20241011/202410111515.TbOH4hSS-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241011/202410111515.TbOH4hSS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410111515.TbOH4hSS-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/core/rtnetlink.c:3281:6: warning: variable 'tgt_net' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    3281 |         if (err < 0)
         |             ^~~~~~~
   net/core/rtnetlink.c:3301:10: note: uninitialized use occurs here
    3301 |         put_net(tgt_net);
         |                 ^~~~~~~
   net/core/rtnetlink.c:3281:2: note: remove the 'if' if its condition is always false
    3281 |         if (err < 0)
         |         ^~~~~~~~~~~~
    3282 |                 goto errout;
         |                 ~~~~~~~~~~~
   net/core/rtnetlink.c:3277:6: warning: variable 'tgt_net' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    3277 |         if (err < 0)
         |             ^~~~~~~
   net/core/rtnetlink.c:3301:10: note: uninitialized use occurs here
    3301 |         put_net(tgt_net);
         |                 ^~~~~~~
   net/core/rtnetlink.c:3277:2: note: remove the 'if' if its condition is always false
    3277 |         if (err < 0)
         |         ^~~~~~~~~~~~
    3278 |                 goto errout;
         |                 ~~~~~~~~~~~
   net/core/rtnetlink.c:3272:21: note: initialize the variable 'tgt_net' to silence this warning
    3272 |         struct net *tgt_net;
         |                            ^
         |                             = NULL
   2 warnings generated.


vim +3281 net/core/rtnetlink.c

cc6090e985d7d6 Jiri Pirko        2019-09-30  3264  
c21ef3e343ae91 David Ahern       2017-04-16  3265  static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
c21ef3e343ae91 David Ahern       2017-04-16  3266  			struct netlink_ext_ack *extack)
0157f60c0caea2 Patrick McHardy   2007-06-13  3267  {
3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3268  	struct ifinfomsg *ifm = nlmsg_data(nlh);
3b1e0a655f8eba YOSHIFUJI Hideaki 2008-03-26  3269  	struct net *net = sock_net(skb->sk);
0157f60c0caea2 Patrick McHardy   2007-06-13  3270  	struct nlattr *tb[IFLA_MAX+1];
3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3271  	struct net_device *dev = NULL;
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3272  	struct net *tgt_net;
3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3273  	int err;
0157f60c0caea2 Patrick McHardy   2007-06-13  3274  
8cb081746c031f Johannes Berg     2019-04-26  3275  	err = nlmsg_parse_deprecated(nlh, sizeof(*ifm), tb, IFLA_MAX,
8cb081746c031f Johannes Berg     2019-04-26  3276  				     ifla_policy, extack);
0157f60c0caea2 Patrick McHardy   2007-06-13  3277  	if (err < 0)
0157f60c0caea2 Patrick McHardy   2007-06-13  3278  		goto errout;
0157f60c0caea2 Patrick McHardy   2007-06-13  3279  
4ff66cae7f10b6 Christian Brauner 2018-02-07  3280  	err = rtnl_ensure_unique_netns(tb, extack, false);
4ff66cae7f10b6 Christian Brauner 2018-02-07 @3281  	if (err < 0)
4ff66cae7f10b6 Christian Brauner 2018-02-07  3282  		goto errout;
4ff66cae7f10b6 Christian Brauner 2018-02-07  3283  
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3284  	tgt_net = rtnl_link_get_net_capable(skb, net, tb, CAP_NET_ADMIN);
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3285  	if (IS_ERR(tgt_net))
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3286  		return PTR_ERR(tgt_net);
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3287  
0157f60c0caea2 Patrick McHardy   2007-06-13  3288  	if (ifm->ifi_index > 0)
a3d1289126e7b1 Eric Dumazet      2009-10-21  3289  		dev = __dev_get_by_index(net, ifm->ifi_index);
76c9ac0ee878f6 Jiri Pirko        2019-09-30  3290  	else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
5ea08b5286f66e Florent Fourcot   2022-04-15  3291  		dev = rtnl_dev_get(net, tb);
0157f60c0caea2 Patrick McHardy   2007-06-13  3292  	else
3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3293  		err = -EINVAL;
0157f60c0caea2 Patrick McHardy   2007-06-13  3294  
3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3295  	if (dev)
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3296  		err = do_setlink(skb, dev, tgt_net, ifm, extack, tb, 0);
3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3297  	else if (!err)
0157f60c0caea2 Patrick McHardy   2007-06-13  3298  		err = -ENODEV;
0157f60c0caea2 Patrick McHardy   2007-06-13  3299  
da5e0494c542dd Thomas Graf       2006-08-10  3300  errout:
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3301  	put_net(tgt_net);
b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3302  
^1da177e4c3f41 Linus Torvalds    2005-04-16  3303  	return err;
^1da177e4c3f41 Linus Torvalds    2005-04-16  3304  }
^1da177e4c3f41 Linus Torvalds    2005-04-16  3305
Kuniyuki Iwashima Oct. 11, 2024, 4:17 p.m. UTC | #3 
From: kernel test robot <lkp@intel.com>
Date: Fri, 11 Oct 2024 15:36:55 +0800
> cc6090e985d7d6 Jiri Pirko        2019-09-30  3264  
> c21ef3e343ae91 David Ahern       2017-04-16  3265  static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> c21ef3e343ae91 David Ahern       2017-04-16  3266  			struct netlink_ext_ack *extack)
> 0157f60c0caea2 Patrick McHardy   2007-06-13  3267  {
> 3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3268  	struct ifinfomsg *ifm = nlmsg_data(nlh);
> 3b1e0a655f8eba YOSHIFUJI Hideaki 2008-03-26  3269  	struct net *net = sock_net(skb->sk);
> 0157f60c0caea2 Patrick McHardy   2007-06-13  3270  	struct nlattr *tb[IFLA_MAX+1];
> 3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3271  	struct net_device *dev = NULL;
> b27f78e2575aa2 Kuniyuki Iwashima 2024-10-09  3272  	struct net *tgt_net;
> 3a6cb17da69fbf Kuniyuki Iwashima 2024-10-09  3273  	int err;
> 0157f60c0caea2 Patrick McHardy   2007-06-13  3274  
> 8cb081746c031f Johannes Berg     2019-04-26  3275  	err = nlmsg_parse_deprecated(nlh, sizeof(*ifm), tb, IFLA_MAX,
> 8cb081746c031f Johannes Berg     2019-04-26  3276  				     ifla_policy, extack);
> 0157f60c0caea2 Patrick McHardy   2007-06-13  3277  	if (err < 0)
> 0157f60c0caea2 Patrick McHardy   2007-06-13  3278  		goto errout;
> 0157f60c0caea2 Patrick McHardy   2007-06-13  3279  
> 4ff66cae7f10b6 Christian Brauner 2018-02-07  3280  	err = rtnl_ensure_unique_netns(tb, extack, false);
> 4ff66cae7f10b6 Christian Brauner 2018-02-07 @3281  	if (err < 0)
> 4ff66cae7f10b6 Christian Brauner 2018-02-07  3282  		goto errout;

Oops, I'll simply remove the errout label in v2.
diff mbox series

Patch

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index de693a88986e..a0702e531331 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2875,8 +2875,8 @@  static int do_set_proto_down(struct net_device *dev,
 #define DO_SETLINK_MODIFIED	0x01
 /* notify flag means notify + modified. */
 #define DO_SETLINK_NOTIFY	0x03
-static int do_setlink(const struct sk_buff *skb,
-		      struct net_device *dev, struct ifinfomsg *ifm,
+static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
+		      struct net *tgt_net, struct ifinfomsg *ifm,
 		      struct netlink_ext_ack *extack,
 		      struct nlattr **tb, int status)
 {
@@ -2893,27 +2893,19 @@  static int do_setlink(const struct sk_buff *skb,
 	else
 		ifname[0] = '\0';
 
-	if (tb[IFLA_NET_NS_PID] || tb[IFLA_NET_NS_FD] || tb[IFLA_TARGET_NETNSID]) {
+	if (!net_eq(tgt_net, dev_net(dev))) {
 		const char *pat = ifname[0] ? ifname : NULL;
-		struct net *net;
 		int new_ifindex;
 
-		net = rtnl_link_get_net_capable(skb, dev_net(dev),
-						tb, CAP_NET_ADMIN);
-		if (IS_ERR(net)) {
-			err = PTR_ERR(net);
-			goto errout;
-		}
-
 		if (tb[IFLA_NEW_IFINDEX])
 			new_ifindex = nla_get_s32(tb[IFLA_NEW_IFINDEX]);
 		else
 			new_ifindex = 0;
 
-		err = __dev_change_net_namespace(dev, net, pat, new_ifindex);
-		put_net(net);
+		err = __dev_change_net_namespace(dev, tgt_net, pat, new_ifindex);
 		if (err)
 			goto errout;
+
 		status |= DO_SETLINK_MODIFIED;
 	}
 
@@ -3277,6 +3269,7 @@  static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tb[IFLA_MAX+1];
 	struct net_device *dev = NULL;
+	struct net *tgt_net;
 	int err;
 
 	err = nlmsg_parse_deprecated(nlh, sizeof(*ifm), tb, IFLA_MAX,
@@ -3288,6 +3281,10 @@  static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err < 0)
 		goto errout;
 
+	tgt_net = rtnl_link_get_net_capable(skb, net, tb, CAP_NET_ADMIN);
+	if (IS_ERR(tgt_net))
+		return PTR_ERR(tgt_net);
+
 	if (ifm->ifi_index > 0)
 		dev = __dev_get_by_index(net, ifm->ifi_index);
 	else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
@@ -3296,11 +3293,13 @@  static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		err = -EINVAL;
 
 	if (dev)
-		err = do_setlink(skb, dev, ifm, extack, tb, 0);
+		err = do_setlink(skb, dev, tgt_net, ifm, extack, tb, 0);
 	else if (!err)
 		err = -ENODEV;
 
 errout:
+	put_net(tgt_net);
+
 	return err;
 }
 
@@ -3593,7 +3592,7 @@  static int rtnl_changelink(const struct sk_buff *skb, struct nlmsghdr *nlh,
 		status |= DO_SETLINK_NOTIFY;
 	}
 
-	return do_setlink(skb, dev, nlmsg_data(nlh), extack, tb, status);
+	return do_setlink(skb, dev, tgt_net, nlmsg_data(nlh), extack, tb, status);
 }
 
 static int rtnl_group_changelink(const struct sk_buff *skb,
@@ -3607,7 +3606,7 @@  static int rtnl_group_changelink(const struct sk_buff *skb,
 
 	for_each_netdev_safe(net, dev, aux) {
 		if (dev->group == group) {
-			err = do_setlink(skb, dev, ifm, extack, tb, 0);
+			err = do_setlink(skb, dev, tgt_net, ifm, extack, tb, 0);
 			if (err < 0)
 				return err;
 		}