neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL.

While working on the follow-up suggested here,

https://lore.kernel.org/netdev/20260902143023.GA3966681@shredder/

I found a few bugs in RTM_GETNEIGHTBL and RTM_SETNEIGHTBL, which this series fixes.

Patch 1, 3, 4 will conflict with net-next in neightbl_dump_info() due to removal of net_eq() below:

	p = list_next_entry(&tbl->parms, list);
	list_for_each_entry_from_rcu(p, &tbl->parms_list, list) {
		if (!net_eq(neigh_parms_net(p), net))
			continue;

Note also that currently neigh_proc_dointvec_ms_jiffies_positive() is buggy and does not enforce min/max, and it needs this fix:

https://lore.kernel.org/linux-fsdevel/[email protected]/

Changes:
v2:
* Patch 1
* Use list_for_each_entry_rcu() and skip the default parms
* Patch 2
* Clarify the attribute is for NTF_MANAGED
* Update docs, rt-neigh.yaml and ip-sysctl.rst

v1: https://lore.kernel.org/netdev/[email protected]/

Kuniyuki Iwashima (4):
neighbour: Add missing RCU annotation for neightbl_dump_info().
neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.
neighbour: Don't render blackhole_netdev via RTM_GETNEIGHTBL.
neighbour: Skip default parms when resumed in neightbl_dump_info().

 Documentation/netlink/specs/rt-neigh.yaml |  2 ++
 Documentation/networking/ip-sysctl.rst    |  2 +-
 net/core/neighbour.c                      | 35 +++++++++++++++++------
 3 files changed, 29 insertions(+), 10 deletions(-)

2.55.0.979.g7e5102b832-goog

neightbl_dump_info() fetches the first non-default neigh_parms with list_next_entry(&tbl->parms, ...) and iterates through the list with list_for_each_entry_from_rcu().

However, list_next_entry() does not use RCU helper.

Let's use list_for_each_entry_rcu() and skip the default parms.

---
v2: Use list_for_each_entry_rcu() and skip the default parms

---
 net/core/neighbour.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

index 1349c0eedb64..49dd7df149ef 100644
@@ -2611,11 +2611,14 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 			break;
 
 		nidx = 0;
-		p = list_next_entry(&tbl->parms, list);
-		list_for_each_entry_from_rcu(p, &tbl->parms_list, list) {
+
+		list_for_each_entry_rcu(p, &tbl->parms_list, list) {
 			if (!net_eq(neigh_parms_net(p), net))
 				continue;
 
+			if (!p->dev)
+				continue;
+
 			if (nidx < neigh_skip)
 				goto next;
 

2.55.0.979.g7e5102b832-goog

NDTPA_INTERVAL_PROBE_TIME_MS sets .type and .min but misses .validation_type, so no validation is applied:

  # ynl --family rt-neigh --do setneightbl \
  --json '{"name": "arp_cache", "parms": {"interval-probe-time-ms": 0}}'

  # ynl --family rt-neigh --dump getneightbl --output-json | \
  jq '.[] | select(.name == "arp_cache" and has("config"))
          | .parms["interval-probe-time-ms"]'
  0

Moreover, nla_get_msecs() uses msecs_to_jiffies(), and u64 is silently cast to u32, so a larger value can bypass the min check:

e.g. 4294967296 == 0x100000000

  # ynl --family rt-neigh --do setneightbl \
  --json '{"name": "arp_cache", "parms": {"interval-probe-time-ms": 4294967296}}'

  # ynl --family rt-neigh --dump getneightbl --output-json | \
  jq '.[] | select(.name == "arp_cache" and has("config"))
          | .parms["interval-probe-time-ms"]'
  0

msecs_to_jiffies() returns MAX_JIFFY_OFFSET if the value is larger than INT_MAX. Also, INT_MAX ms overflows int NEIGH_VAR() when HZ > 1000 (Alpha, MIPS), and passing a negative integer to queue_delayed_work(unsigned long delay) causes sign extension, which wraps around the expiry time to the past, resulting in it being handled as 0 delay in the timer wheel.

Let's use NLA_POLICY_FULL_RANGE() and limit the max to 1 day.

The same max check is applied to sysctl as well.

Note that this controls the probe interval for NTF_MANAGED entries, and therefore the max of 1 day is unlikely to break any deployments.

---
Currently, the sysctl range check is not applied to interval_probe_time_ms, which needs this fix:
https://lore.kernel.org/linux-fsdevel/[email protected]/

v2:
* Clarify the attribute is for NTF_MANAGED
* Update docs, rt-neigh.yaml and ip-sysctl.rst

---
 Documentation/netlink/specs/rt-neigh.yaml |  2 ++
 Documentation/networking/ip-sysctl.rst    |  2 +-
 net/core/neighbour.c                      | 17 +++++++++++++----
 3 files changed, 16 insertions(+), 5 deletions(-)

index 0f46ef313590..5ec57d95d7f6 100644
@@ -341,6 +341,8 @@ attribute-sets:
       -
         name: interval-probe-time-ms
         type: u64
+        min: 1
+        max: 86400000
 
 operations:
   enum-model: directional

index 208f46967ee5..b05829e44d8f 100644
@@ -248,7 +248,7 @@ neigh/default/unres_qlen - INTEGER
 
 neigh/default/interval_probe_time_ms - INTEGER
 	The probe interval for neighbor entries with NTF_MANAGED flag,
-	the min value is 1.
+	the min value is 1, and the max value is 86400000 (1 day).
 
 	Default: 5000
 

index 49dd7df149ef..0db78a0dfb51 100644
@@ -2359,6 +2359,13 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
 	[NDTA_PARMS]		= { .type = NLA_NESTED },
 };
 
+#define NTBL_PARM_MS_MAX	(24 * 60 * 60 * MSEC_PER_SEC)
+
+static const struct netlink_range_validation nl_ntbl_parm_ms_range = {
+	.min = 1,
+	.max = NTBL_PARM_MS_MAX,
+};
+
 static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
 	[NDTPA_IFINDEX]			= { .type = NLA_U32 },
 	[NDTPA_QUEUE_LEN]		= { .type = NLA_U32 },

@@ -2375,7 +2382,8 @@ static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
 	[NDTPA_ANYCAST_DELAY]		= { .type = NLA_U64 },
 	[NDTPA_PROXY_DELAY]		= { .type = NLA_U64 },
 	[NDTPA_LOCKTIME]		= { .type = NLA_U64 },
-	[NDTPA_INTERVAL_PROBE_TIME_MS]	= { .type = NLA_U64, .min = 1 },
+	[NDTPA_INTERVAL_PROBE_TIME_MS]	= NLA_POLICY_FULL_RANGE(NLA_U64,
+								&nl_ntbl_parm_ms_range),
 };
 
 static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh,

@@ -3672,12 +3680,13 @@ static int neigh_proc_dointvec_ms_jiffies_positive(const struct ctl_table *ctl,
 						   void *buffer, size_t *lenp, loff_t *ppos)
 {
 	struct ctl_table tmp = *ctl;
-	int ret;
+	int ret, min, max;
 
-	int min = msecs_to_jiffies(1);
+	min = msecs_to_jiffies(1);
+	max = msecs_to_jiffies(NTBL_PARM_MS_MAX);
 
 	tmp.extra1 = &min;
-	tmp.extra2 = NULL;
+	tmp.extra2 = &max;
 
 	ret = proc_dointvec_ms_jiffies_minmax(&tmp, write, buffer, lenp, ppos);
 	neigh_proc_update(ctl, write);

2.55.0.979.g7e5102b832-goog

BTW, patch LGTM otherwise, so you can add my tag to v3. Thanks!

Oh sorry, I completely overlooked "check:" !

TIL : --validate

I was wondering how it could be used after seeing strace showed recvmsg() with -ERANGE from kernel.

Will fix "check:" in v3 and add your tag.

Thank you !

The cited commits started to initialise blackhole_netdev with neigh_parms_alloc().

This is visible in init_net as the ifindex==0 entries via RTM_GETNEIGHTBL:

  # ynl --family rt-neigh --dump getneightbl --output-json \
    | jq '.[] | select(.parms.ifindex == 0)
              | {name: .name, ifindex: .parms.ifindex}'
  {
    "name": "arp_cache",
    "ifindex": 0
  }
  {
    "name": "ndisc_cache",
    "ifindex": 0
  }

For RTM_SETNEIGHTBL, ifindex being 0 means wildcard.

Let's skip blackhole_netdev's parms in neightbl_dump_info().

Note that lookup_neigh_parms() does not need the same change because the default parms is always the first entry and matches with ifindex == 0.

---
Cc: Xin Long <[email protected]>

---
 net/core/neighbour.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 0db78a0dfb51..02bf940ce00a 100644
@@ -2624,7 +2624,7 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 			if (!net_eq(neigh_parms_net(p), net))
 				continue;
 
-			if (!p->dev)
+			if (!p->dev || p->dev == blackhole_netdev)
 				continue;
 
 			if (nidx < neigh_skip)

2.55.0.979.g7e5102b832-goog

neightbl_dump_info() calls neightbl_fill_info() in each loop to render the default parms.

If there are many devices and neightbl_fill_param_info() failed, neightbl_fill_info() is called again when the dump resumes:

  # ynl --family rt-neigh --dump getneightbl --output-json |
    jq '.[] | {name: .name, ifindex: .parms.ifindex}'
  ...
  {
    "name": "ndisc_cache",
    "ifindex": null
  }
  ...
  {
    "name": "ndisc_cache",
    "ifindex": 6
  }
  {
    "name": "ndisc_cache",
    "ifindex": null
  }
  {
    "name": "ndisc_cache",
    "ifindex": 5
  }

Let's skip neightbl_fill_info() if it is already called in neightbl_dump_info().

Note that we cannot use !neigh_skip instead of !default_skip because default_skip == 1 && neigh_skip == 0 could be true if the first neightbl_fill_param_info() fails.

---
Cc: Thomas Graf <[email protected]>

---
 net/core/neighbour.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

index 02bf940ce00a..7448320f7ad5 100644
@@ -2587,9 +2587,10 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
+	int default_skip = cb->args[2];
+	int neigh_skip = cb->args[1];
 	int family, tidx, nidx = 0;
 	int tbl_skip = cb->args[0];
-	int neigh_skip = cb->args[1];
 	struct neigh_table *tbl;
 
 	if (cb->strict_check) {

@@ -2613,12 +2614,13 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 		if (tidx < tbl_skip || (family && tbl->family != family))
 			continue;
 
-		if (neightbl_fill_info(skb, tbl, NETLINK_CB(cb->skb).portid,
+		if (!default_skip &&
+		    neightbl_fill_info(skb, tbl, NETLINK_CB(cb->skb).portid,
 				       nlh->nlmsg_seq, RTM_NEWNEIGHTBL,
 				       NLM_F_MULTI) < 0)
 			break;
 
-		nidx = 0;
+		default_skip = 1;
 
 		list_for_each_entry_rcu(p, &tbl->parms_list, list) {
 			if (!net_eq(neigh_parms_net(p), net))

@@ -2641,12 +2643,15 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 		}
 
 		neigh_skip = 0;
+		nidx = 0;
+		default_skip = 0;
 	}
 out:
 	rcu_read_unlock();
 
 	cb->args[0] = tidx;
 	cb->args[1] = nidx;
+	cb->args[2] = default_skip;
 
 	return skb->len;
 }

2.55.0.979.g7e5102b832-goog