[PATCH v1 net-next 0/7] nexthop: Convert RTM_{NEW,DEL}NEXTHOP to per-netns RTNL.

[PATCH v1 net-next 0/7] nexthop: Convert RTM_{NEW,DEL}NEXTHOP to per-netns RTNL.

[Kuniyuki Iwashima]

Patch 1 - 5 move some validation for RTM_NEWNEXTHOP so that it can be
done without RTNL.

Patch 6 & 7 converts RTM_NEWNEXTHOP and RTM_DELNEXTHOP to per-netns RTNL.

Note that RTM_GETNEXTHOP and RTM_GETNEXTHOPBUCKET are not touched in
this series.

rtm_get_nexthop() can be easily converted to RCU, but rtm_dump_nexthop()
needs more work due to the left-to-right rbtree walk, which looks prone
to node deletion and tree rotation without a retry mechanism.


Kuniyuki Iwashima (7):
  nexthop: Move nlmsg_parse() in rtm_to_nh_config() to
    rtm_new_nexthop().
  nexthop: Split nh_check_attr_group().
  nexthop: Move NHA_OIF validation to rtm_to_nh_config_rtnl().
  nexthop: Check NLM_F_REPLACE and NHA_ID in rtm_new_nexthop().
  nexthop: Remove redundant group len check in nexthop_create_group().
  nexthop: Convert RTM_NEWNEXTHOP to per-netns RTNL.
  nexthop: Convert RTM_DELNEXTHOP to per-netns RTNL.

 net/ipv4/nexthop.c | 183 +++++++++++++++++++++++++++------------------
 1 file changed, 112 insertions(+), 71 deletions(-)

-- 
2.48.1

[PATCH v1 net-next 1/7] nexthop: Move nlmsg_parse() in rtm_to_nh_config() to rtm_new_nexthop().

[Kuniyuki Iwashima]

We will split rtm_to_nh_config() into non-RTNL and RTNL parts,
and then the latter also needs tb.

As a prep, let's move nlmsg_parse() to rtm_new_nexthop().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/nexthop.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 01df7dd795f0..487933ecdb68 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -3016,19 +3016,13 @@ static int rtm_to_nh_config_grp_res(struct nlattr *res, struct nh_config *cfg,
 }
 
 static int rtm_to_nh_config(struct net *net, struct sk_buff *skb,
-			    struct nlmsghdr *nlh, struct nh_config *cfg,
+			    struct nlmsghdr *nlh, struct nlattr **tb,
+			    struct nh_config *cfg,
 			    struct netlink_ext_ack *extack)
 {
 	struct nhmsg *nhm = nlmsg_data(nlh);
-	struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_new)];
 	int err;
 
-	err = nlmsg_parse(nlh, sizeof(*nhm), tb,
-			  ARRAY_SIZE(rtm_nh_policy_new) - 1,
-			  rtm_nh_policy_new, extack);
-	if (err < 0)
-		return err;
-
 	err = -EINVAL;
 	if (nhm->resvd || nhm->nh_scope) {
 		NL_SET_ERR_MSG(extack, "Invalid values in ancillary header");
@@ -3093,7 +3087,8 @@ static int rtm_to_nh_config(struct net *net, struct sk_buff *skb,
 			NL_SET_ERR_MSG(extack, "Invalid group type");
 			goto out;
 		}
-		err = nh_check_attr_group(net, tb, ARRAY_SIZE(tb),
+
+		err = nh_check_attr_group(net, tb, ARRAY_SIZE(rtm_nh_policy_new),
 					  cfg->nh_grp_type, extack);
 		if (err)
 			goto out;
@@ -3211,18 +3206,26 @@ static int rtm_to_nh_config(struct net *net, struct sk_buff *skb,
 static int rtm_new_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 			   struct netlink_ext_ack *extack)
 {
+	struct nlattr *tb[ARRAY_SIZE(rtm_nh_policy_new)];
 	struct net *net = sock_net(skb->sk);
 	struct nh_config cfg;
 	struct nexthop *nh;
 	int err;
 
-	err = rtm_to_nh_config(net, skb, nlh, &cfg, extack);
-	if (!err) {
-		nh = nexthop_add(net, &cfg, extack);
-		if (IS_ERR(nh))
-			err = PTR_ERR(nh);
-	}
+	err = nlmsg_parse(nlh, sizeof(struct nhmsg), tb,
+			  ARRAY_SIZE(rtm_nh_policy_new) - 1,
+			  rtm_nh_policy_new, extack);
+	if (err < 0)
+		goto out;
 
+	err = rtm_to_nh_config(net, skb, nlh, tb, &cfg, extack);
+	if (err)
+		goto out;
+
+	nh = nexthop_add(net, &cfg, extack);
+	if (IS_ERR(nh))
+		err = PTR_ERR(nh);
+out:
 	return err;
 }
 
-- 
2.48.1

[PATCH v1 net-next 2/7] nexthop: Split nh_check_attr_group().

[Kuniyuki Iwashima]

We will push RTNL down to rtm_new_nexthop(), and then we want
to move non-RTNL operations out of the scope.

nh_check_attr_group() validates NHA_GROUP attributes, and some
validation requires RTNL.

Let's factorise such parts as nh_check_attr_group_rtnl() and
call it from rtm_to_nh_config_rtnl().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/nexthop.c | 68 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 47 insertions(+), 21 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 487933ecdb68..98d5bf6e40f9 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -1272,10 +1272,8 @@ static int nh_check_attr_group(struct net *net,
 			       u16 nh_grp_type, struct netlink_ext_ack *extack)
 {
 	unsigned int len = nla_len(tb[NHA_GROUP]);
-	u8 nh_family = AF_UNSPEC;
 	struct nexthop_grp *nhg;
 	unsigned int i, j;
-	u8 nhg_fdb = 0;
 
 	if (!len || len & (sizeof(struct nexthop_grp) - 1)) {
 		NL_SET_ERR_MSG(extack,
@@ -1307,10 +1305,41 @@ static int nh_check_attr_group(struct net *net,
 		}
 	}
 
-	if (tb[NHA_FDB])
-		nhg_fdb = 1;
 	nhg = nla_data(tb[NHA_GROUP]);
-	for (i = 0; i < len; ++i) {
+	for (i = NHA_GROUP_TYPE + 1; i < tb_size; ++i) {
+		if (!tb[i])
+			continue;
+		switch (i) {
+		case NHA_HW_STATS_ENABLE:
+		case NHA_FDB:
+			continue;
+		case NHA_RES_GROUP:
+			if (nh_grp_type == NEXTHOP_GRP_TYPE_RES)
+				continue;
+			break;
+		}
+		NL_SET_ERR_MSG(extack,
+			       "No other attributes can be set in nexthop groups");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int nh_check_attr_group_rtnl(struct net *net, struct nlattr *tb[],
+				    struct netlink_ext_ack *extack)
+{
+	u8 nh_family = AF_UNSPEC;
+	struct nexthop_grp *nhg;
+	unsigned int len;
+	unsigned int i;
+	u8 nhg_fdb;
+
+	len = nla_len(tb[NHA_GROUP]) / sizeof(*nhg);
+	nhg = nla_data(tb[NHA_GROUP]);
+	nhg_fdb = !!tb[NHA_FDB];
+
+	for (i = 0; i < len; i++) {
 		struct nexthop *nh;
 		bool is_fdb_nh;
 
@@ -1330,22 +1359,6 @@ static int nh_check_attr_group(struct net *net,
 			return -EINVAL;
 		}
 	}
-	for (i = NHA_GROUP_TYPE + 1; i < tb_size; ++i) {
-		if (!tb[i])
-			continue;
-		switch (i) {
-		case NHA_HW_STATS_ENABLE:
-		case NHA_FDB:
-			continue;
-		case NHA_RES_GROUP:
-			if (nh_grp_type == NEXTHOP_GRP_TYPE_RES)
-				continue;
-			break;
-		}
-		NL_SET_ERR_MSG(extack,
-			       "No other attributes can be set in nexthop groups");
-		return -EINVAL;
-	}
 
 	return 0;
 }
@@ -3202,6 +3215,15 @@ static int rtm_to_nh_config(struct net *net, struct sk_buff *skb,
 	return err;
 }
 
+static int rtm_to_nh_config_rtnl(struct net *net, struct nlattr **tb,
+				 struct netlink_ext_ack *extack)
+{
+	if (tb[NHA_GROUP])
+		return nh_check_attr_group_rtnl(net, tb, extack);
+
+	return 0;
+}
+
 /* rtnl */
 static int rtm_new_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 			   struct netlink_ext_ack *extack)
@@ -3222,6 +3244,10 @@ static int rtm_new_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err)
 		goto out;
 
+	err = rtm_to_nh_config_rtnl(net, tb, extack);
+	if (!err)
+		goto out;
+
 	nh = nexthop_add(net, &cfg, extack);
 	if (IS_ERR(nh))
 		err = PTR_ERR(nh);
-- 
2.48.1

[PATCH v1 net-next 3/7] nexthop: Move NHA_OIF validation to rtm_to_nh_config_rtnl().

[Kuniyuki Iwashima]

NHA_OIF needs to look up a device by __dev_get_by_index(),
which requires RTNL.

Let's move NHA_OIF validation to rtm_to_nh_config_rtnl().

Note that the proceeding checks made the original !cfg->nh_fdb
check redundant.

  NHA_FDB is set           -> NHA_OIF cannot be set
  NHA_FDB is set but false -> NHA_OIF must be set
  NHA_FDB is not set       -> NHA_OIF must be set

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/nexthop.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 98d5bf6e40f9..f21ea1ddd68f 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -3134,25 +3134,6 @@ static int rtm_to_nh_config(struct net *net, struct sk_buff *skb,
 		goto out;
 	}
 
-	if (!cfg->nh_fdb && tb[NHA_OIF]) {
-		cfg->nh_ifindex = nla_get_u32(tb[NHA_OIF]);
-		if (cfg->nh_ifindex)
-			cfg->dev = __dev_get_by_index(net, cfg->nh_ifindex);
-
-		if (!cfg->dev) {
-			NL_SET_ERR_MSG(extack, "Invalid device index");
-			goto out;
-		} else if (!(cfg->dev->flags & IFF_UP)) {
-			NL_SET_ERR_MSG(extack, "Nexthop device is not up");
-			err = -ENETDOWN;
-			goto out;
-		} else if (!netif_carrier_ok(cfg->dev)) {
-			NL_SET_ERR_MSG(extack, "Carrier for nexthop device is down");
-			err = -ENETDOWN;
-			goto out;
-		}
-	}
-
 	err = -EINVAL;
 	if (tb[NHA_GATEWAY]) {
 		struct nlattr *gwa = tb[NHA_GATEWAY];
@@ -3216,11 +3197,33 @@ static int rtm_to_nh_config(struct net *net, struct sk_buff *skb,
 }
 
 static int rtm_to_nh_config_rtnl(struct net *net, struct nlattr **tb,
+				 struct nh_config *cfg,
 				 struct netlink_ext_ack *extack)
 {
 	if (tb[NHA_GROUP])
 		return nh_check_attr_group_rtnl(net, tb, extack);
 
+	if (tb[NHA_OIF]) {
+		cfg->nh_ifindex = nla_get_u32(tb[NHA_OIF]);
+		if (cfg->nh_ifindex)
+			cfg->dev = __dev_get_by_index(net, cfg->nh_ifindex);
+
+		if (!cfg->dev) {
+			NL_SET_ERR_MSG(extack, "Invalid device index");
+			return -EINVAL;
+		}
+
+		if (!(cfg->dev->flags & IFF_UP)) {
+			NL_SET_ERR_MSG(extack, "Nexthop device is not up");
+			return -ENETDOWN;
+		}
+
+		if (!netif_carrier_ok(cfg->dev)) {
+			NL_SET_ERR_MSG(extack, "Carrier for nexthop device is down");
+			return -ENETDOWN;
+		}
+	}
+
 	return 0;
 }
 
@@ -3244,7 +3247,7 @@ static int rtm_new_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err)
 		goto out;
 
-	err = rtm_to_nh_config_rtnl(net, tb, extack);
+	err = rtm_to_nh_config_rtnl(net, tb, &cfg, extack);
 	if (!err)
 		goto out;
 
-- 
2.48.1

[PATCH v1 net-next 4/7] nexthop: Check NLM_F_REPLACE and NHA_ID in rtm_new_nexthop().

[Kuniyuki Iwashima]

nexthop_add() checks if NLM_F_REPLACE is specified without
non-zero NHA_ID, which does not require RTNL.

Let's move the check to rtm_new_nexthop().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/nexthop.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index f21ea1ddd68f..09f5f31f34a0 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -2928,11 +2928,6 @@ static struct nexthop *nexthop_add(struct net *net, struct nh_config *cfg,
 	struct nexthop *nh;
 	int err;
 
-	if (cfg->nlflags & NLM_F_REPLACE && !cfg->nh_id) {
-		NL_SET_ERR_MSG(extack, "Replace requires nexthop id");
-		return ERR_PTR(-EINVAL);
-	}
-
 	if (!cfg->nh_id) {
 		cfg->nh_id = nh_find_unused_id(net);
 		if (!cfg->nh_id) {
@@ -3247,6 +3242,12 @@ static int rtm_new_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err)
 		goto out;
 
+	if (cfg.nlflags & NLM_F_REPLACE && !cfg.nh_id) {
+		NL_SET_ERR_MSG(extack, "Replace requires nexthop id");
+		err = -EINVAL;
+		goto out;
+	}
+
 	err = rtm_to_nh_config_rtnl(net, tb, &cfg, extack);
 	if (!err)
 		goto out;
-- 
2.48.1

[PATCH v1 net-next 5/7] nexthop: Remove redundant group len check in nexthop_create_group().

[Kuniyuki Iwashima]

The number of NHA_GROUP entries is guaranteed to be non-zero in
nh_check_attr_group().

Let's remove the redundant check in nexthop_create_group().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/nexthop.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 09f5f31f34a0..409f13d64ed4 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -2692,9 +2692,6 @@ static struct nexthop *nexthop_create_group(struct net *net,
 	int err;
 	int i;
 
-	if (WARN_ON(!num_nh))
-		return ERR_PTR(-EINVAL);
-
 	nh = nexthop_alloc();
 	if (!nh)
 		return ERR_PTR(-ENOMEM);
-- 
2.48.1

[PATCH v1 net-next 6/7] nexthop: Convert RTM_NEWNEXTHOP to per-netns RTNL.

[Kuniyuki Iwashima]

If we pass false to the rtnl_held param of lwtunnel_valid_encap_type(),
we can move RTNL down before rtm_to_nh_config_rtnl().

Let's use rtnl_net_lock() in rtm_new_nexthop().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/nexthop.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 409f13d64ed4..ea62454e0a0c 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -3169,7 +3169,7 @@ static int rtm_to_nh_config(struct net *net, struct sk_buff *skb,
 
 		cfg->nh_encap_type = nla_get_u16(tb[NHA_ENCAP_TYPE]);
 		err = lwtunnel_valid_encap_type(cfg->nh_encap_type,
-						extack, true);
+						extack, false);
 		if (err < 0)
 			goto out;
 
@@ -3245,13 +3245,18 @@ static int rtm_new_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 	}
 
+	rtnl_net_lock(net);
+
 	err = rtm_to_nh_config_rtnl(net, tb, &cfg, extack);
 	if (!err)
-		goto out;
+		goto unlock;
 
 	nh = nexthop_add(net, &cfg, extack);
 	if (IS_ERR(nh))
 		err = PTR_ERR(nh);
+
+unlock:
+	rtnl_net_unlock(net);
 out:
 	return err;
 }
@@ -4067,18 +4072,19 @@ static struct pernet_operations nexthop_net_ops = {
 };
 
 static const struct rtnl_msg_handler nexthop_rtnl_msg_handlers[] __initconst = {
-	{.msgtype = RTM_NEWNEXTHOP, .doit = rtm_new_nexthop},
+	{.msgtype = RTM_NEWNEXTHOP, .doit = rtm_new_nexthop,
+	 .flags = RTNL_FLAG_DOIT_PERNET},
 	{.msgtype = RTM_DELNEXTHOP, .doit = rtm_del_nexthop},
 	{.msgtype = RTM_GETNEXTHOP, .doit = rtm_get_nexthop,
 	 .dumpit = rtm_dump_nexthop},
 	{.msgtype = RTM_GETNEXTHOPBUCKET, .doit = rtm_get_nexthop_bucket,
 	 .dumpit = rtm_dump_nexthop_bucket},
 	{.protocol = PF_INET, .msgtype = RTM_NEWNEXTHOP,
-	 .doit = rtm_new_nexthop},
+	 .doit = rtm_new_nexthop, .flags = RTNL_FLAG_DOIT_PERNET},
 	{.protocol = PF_INET, .msgtype = RTM_GETNEXTHOP,
 	 .dumpit = rtm_dump_nexthop},
 	{.protocol = PF_INET6, .msgtype = RTM_NEWNEXTHOP,
-	 .doit = rtm_new_nexthop},
+	 .doit = rtm_new_nexthop, .flags = RTNL_FLAG_DOIT_PERNET},
 	{.protocol = PF_INET6, .msgtype = RTM_GETNEXTHOP,
 	 .dumpit = rtm_dump_nexthop},
 };
-- 
2.48.1

[PATCH v1 net-next 7/7] nexthop: Convert RTM_DELNEXTHOP to per-netns RTNL.

[Kuniyuki Iwashima]

In rtm_del_nexthop(), only nexthop_find_by_id() and remove_nexthop()
require RTNL as they touch net->nexthop.rb_root.

Let's move RTNL down as rtnl_net_lock() before nexthop_find_by_id().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/ipv4/nexthop.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index ea62454e0a0c..66d7796ba174 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -3314,13 +3314,17 @@ static int rtm_del_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (err)
 		return err;
 
+	rtnl_net_lock(net);
+
 	nh = nexthop_find_by_id(net, id);
-	if (!nh)
-		return -ENOENT;
+	if (nh)
+		remove_nexthop(net, nh, &nlinfo);
+	else
+		err = -ENOENT;
 
-	remove_nexthop(net, nh, &nlinfo);
+	rtnl_net_unlock(net);
 
-	return 0;
+	return err;
 }
 
 /* rtnl */
@@ -4074,7 +4078,8 @@ static struct pernet_operations nexthop_net_ops = {
 static const struct rtnl_msg_handler nexthop_rtnl_msg_handlers[] __initconst = {
 	{.msgtype = RTM_NEWNEXTHOP, .doit = rtm_new_nexthop,
 	 .flags = RTNL_FLAG_DOIT_PERNET},
-	{.msgtype = RTM_DELNEXTHOP, .doit = rtm_del_nexthop},
+	{.msgtype = RTM_DELNEXTHOP, .doit = rtm_del_nexthop,
+	 .flags = RTNL_FLAG_DOIT_PERNET},
 	{.msgtype = RTM_GETNEXTHOP, .doit = rtm_get_nexthop,
 	 .dumpit = rtm_dump_nexthop},
 	{.msgtype = RTM_GETNEXTHOPBUCKET, .doit = rtm_get_nexthop_bucket,
-- 
2.48.1

Re: [PATCH v1 net-next 0/7] nexthop: Convert RTM_{NEW,DEL}NEXTHOP to per-netns RTNL.

[Paolo Abeni]

Hi,

On 3/19/25 12:31 AM, Kuniyuki Iwashima wrote:
> Patch 1 - 5 move some validation for RTM_NEWNEXTHOP so that it can be
> done without RTNL.
> 
> Patch 6 & 7 converts RTM_NEWNEXTHOP and RTM_DELNEXTHOP to per-netns RTNL.
> 
> Note that RTM_GETNEXTHOP and RTM_GETNEXTHOPBUCKET are not touched in
> this series.
> 
> rtm_get_nexthop() can be easily converted to RCU, but rtm_dump_nexthop()
> needs more work due to the left-to-right rbtree walk, which looks prone
> to node deletion and tree rotation without a retry mechanism.
> 
> 
> Kuniyuki Iwashima (7):
>   nexthop: Move nlmsg_parse() in rtm_to_nh_config() to
>     rtm_new_nexthop().
>   nexthop: Split nh_check_attr_group().
>   nexthop: Move NHA_OIF validation to rtm_to_nh_config_rtnl().
>   nexthop: Check NLM_F_REPLACE and NHA_ID in rtm_new_nexthop().
>   nexthop: Remove redundant group len check in nexthop_create_group().
>   nexthop: Convert RTM_NEWNEXTHOP to per-netns RTNL.
>   nexthop: Convert RTM_DELNEXTHOP to per-netns RTNL.
> 
>  net/ipv4/nexthop.c | 183 +++++++++++++++++++++++++++------------------
>  1 file changed, 112 insertions(+), 71 deletions(-)

This series is apparently causing NULL ptr deref in the nexthop.sh
netdevsim selftests. Unfortunately, due to a transient nipa infra
outage, a lot of stuff landed into the same batch, so I'm not 110% this
series is the real curprit but looks like a reasonable suspect.

Kuniyuki, could you please have a look?

---
[    1.653896] BUG: kernel NULL pointer dereference, address:
0000000000000068
[    1.653963] #PF: supervisor read access in kernel mode
[    1.654003] #PF: error_code(0x0000) - not-present page
[    1.654037] PGD 7828067 P4D 7828067 PUD 782a067 PMD 0
[    1.654077] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
[    1.654119] CPU: 0 UID: 0 PID: 303 Comm: ip Not tainted
6.14.0-rc6-virtme #1
[    1.654176] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[    1.654219] RIP: 0010:rtm_new_nexthop+0x645/0x2260
[    1.654263] Code: 70 02 00 00 48 85 db 75 0e eb 1f 48 83 c3 10 48 8b
1b 48 85 db 74 13 3b 43 60 72 ef 76 0c 48 83 c3 08 48 8b 1b 48 85 db 75
ed <8b> 53 68 4c 8d 63 68 85 d2 0f 84 f1 02 00 00 8d 4a 01 89 d0 f0 41
[    1.654390] RSP: 0018:ffffae348037b860 EFLAGS: 00010246
[    1.654430] RAX: 0000000000000001 RBX: 0000000000000000 RCX:
0000000000000000
[    1.654482] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
ffff992d066b8000
[    1.654534] RBP: ffffae348037bab0 R08: ffff992d012d2fa8 R09:
ffff992d055d1780
[    1.654587] R10: ffffae348037b860 R11: ffff992d055d17c8 R12:
ffffae348037bb60
[    1.654638] R13: 0000000000000000 R14: 0000000000000001 R15:
ffff992d055d17c8
[    1.654692] FS:  00007f8b6fb0c800(0000) GS:ffff992d3ec00000(0000)
knlGS:0000000000000000
[    1.654749] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.654791] CR2: 0000000000000068 CR3: 00000000067ae005 CR4:
0000000000772ef0
[    1.654844] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[    1.654900] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[    1.654957] PKRU: 55555554
[    1.654974] Call Trace:
[    1.654993]  <TASK>
[    1.655015]  ? __die+0x24/0x70
[    1.655049]  ? page_fault_oops+0x15a/0x450
[    1.655080]  ? mas_topiary_replace+0x9ba/0xca0
[    1.655121]  ? exc_page_fault+0x69/0x150
[    1.655162]  ? asm_exc_page_fault+0x26/0x30
[    1.655202]  ? rtm_new_nexthop+0x645/0x2260
[    1.655239]  ? virtqueue_notify+0x1c/0x40
[    1.655269]  ? virtio_fs_enqueue_req+0x50c/0x570
[    1.655311]  ? __pfx_rtm_new_nexthop+0x10/0x10
[    1.655351]  ? rtnetlink_rcv_msg+0x361/0x410
[    1.655391]  rtnetlink_rcv_msg+0x361/0x410
[    1.655417]  ? __remove_hrtimer+0x39/0x90
[    1.655448]  ? sysvec_apic_timer_interrupt+0xf/0x90
[    1.655494]  ? __pfx_rtnetlink_rcv_msg+0x10/0x10
[    1.655528]  netlink_rcv_skb+0x58/0x110
[    1.655560]  netlink_unicast+0x247/0x370
[    1.655592]  netlink_sendmsg+0x1bf/0x3e0
[    1.655624]  ____sys_sendmsg+0x2bc/0x320
[    1.655656]  ? copy_msghdr_from_user+0x6d/0xa0
[    1.655696]  ___sys_sendmsg+0x88/0xd0
[    1.655729]  __sys_sendmsg+0x6c/0xc0
[    1.655760]  do_syscall_64+0x9e/0x1a0
[    1.655793]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[    1.655834] RIP: 0033:0x7f8b6fd189a7
[    1.655864] Code: 0a 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f
1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f
05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
[    1.655986] RSP: 002b:00007ffebd919418 EFLAGS: 00000246 ORIG_RAX:
000000000000002e
[    1.656043] RAX: ffffffffffffffda RBX: 00007ffebd919f80 RCX:
00007f8b6fd189a7
[    1.656099] RDX: 0000000000000000 RSI: 00007ffebd919480 RDI:
0000000000000005
[    1.656151] RBP: 00007ffebd919940 R08: 0000000006ba3910 R09:
0000000000000000
[    1.656202] R10: 00007f8b6fbd1708 R11: 0000000000000246 R12:
0000000006ba3918
[    1.656254] R13: 0000000067da36b9 R14: 0000000000498600 R15:
0000000006ba3910
[    1.656307]  </TASK>
[    1.656324] Modules linked in: netdevsim
[    1.656356] CR2: 0000000000000068

Re: [PATCH v1 net-next 0/7] nexthop: Convert RTM_{NEW,DEL}NEXTHOP to per-netns RTNL.

[Kuniyuki Iwashima]

From: Paolo Abeni <pabeni@redhat.com>
Date: Wed, 19 Mar 2025 08:57:52 +0100
> Hi,
> 
> On 3/19/25 12:31 AM, Kuniyuki Iwashima wrote:
> > Patch 1 - 5 move some validation for RTM_NEWNEXTHOP so that it can be
> > done without RTNL.
> > 
> > Patch 6 & 7 converts RTM_NEWNEXTHOP and RTM_DELNEXTHOP to per-netns RTNL.
> > 
> > Note that RTM_GETNEXTHOP and RTM_GETNEXTHOPBUCKET are not touched in
> > this series.
> > 
> > rtm_get_nexthop() can be easily converted to RCU, but rtm_dump_nexthop()
> > needs more work due to the left-to-right rbtree walk, which looks prone
> > to node deletion and tree rotation without a retry mechanism.
> > 
> > 
> > Kuniyuki Iwashima (7):
> >   nexthop: Move nlmsg_parse() in rtm_to_nh_config() to
> >     rtm_new_nexthop().
> >   nexthop: Split nh_check_attr_group().
> >   nexthop: Move NHA_OIF validation to rtm_to_nh_config_rtnl().
> >   nexthop: Check NLM_F_REPLACE and NHA_ID in rtm_new_nexthop().
> >   nexthop: Remove redundant group len check in nexthop_create_group().
> >   nexthop: Convert RTM_NEWNEXTHOP to per-netns RTNL.
> >   nexthop: Convert RTM_DELNEXTHOP to per-netns RTNL.
> > 
> >  net/ipv4/nexthop.c | 183 +++++++++++++++++++++++++++------------------
> >  1 file changed, 112 insertions(+), 71 deletions(-)
> 
> This series is apparently causing NULL ptr deref in the nexthop.sh
> netdevsim selftests. Unfortunately, due to a transient nipa infra
> outage, a lot of stuff landed into the same batch, so I'm not 110% this
> series is the real curprit but looks like a reasonable suspect.
> 
> Kuniyuki, could you please have a look?
> 
> ---
> [    1.653896] BUG: kernel NULL pointer dereference, address:
> 0000000000000068
> [    1.653963] #PF: supervisor read access in kernel mode
> [    1.654003] #PF: error_code(0x0000) - not-present page
> [    1.654037] PGD 7828067 P4D 7828067 PUD 782a067 PMD 0
> [    1.654077] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
> [    1.654119] CPU: 0 UID: 0 PID: 303 Comm: ip Not tainted
> 6.14.0-rc6-virtme #1
> [    1.654176] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [    1.654219] RIP: 0010:rtm_new_nexthop+0x645/0x2260

Sorry, I failed to resolve conflict during the last minute rebase,
and the normal test bailed out here...

---8<---
@@ -3245,7 +3248,7 @@ static int rtm_new_nexthop(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 
 	err = rtm_to_nh_config_rtnl(net, tb, extack);
-	if (!err)
+	if (err)
 		goto out;
 
 	nh = nexthop_add(net, &cfg, extack);
---8<---

The failed test case created a nexthop group with an invalid ID,
and nexthop_get() for nh by nexthop_find_by_id() assumes nh is not
NULL because it's checked in advance.

Will squash the diff above in v2.

Thanks!