[PATCH v1 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL.

[PATCH v1 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL.

[Kuniyuki Iwashima]

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/20260905233819.1064529-2-kuniyu@google.com/


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().

 net/core/neighbour.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info().

[Kuniyuki Iwashima]

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 fetch the default parms with list_first_entry() and
use list_for_each_entry_continue_rcu() for iteration.

Note that the first entry is always tbl->parms, which never goes
away, so list_first_entry(&tbl->parms_list, ...) is safe.

Fixes: 4ae34be50064 ("neighbour: Convert RTM_GETNEIGHTBL to RCU.")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/neighbour.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 1349c0eedb64..4b17c2a15594 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2611,8 +2611,9 @@ 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) {
+
+		p = list_first_entry(&tbl->parms_list, struct neigh_parms, list);
+		list_for_each_entry_continue_rcu(p, &tbl->parms_list, list) {
 			if (!net_eq(neigh_parms_net(p), net))
 				continue;
 
-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.

[Kuniyuki Iwashima]

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.

Fixes: 211da42eaa45 ("net, neigh: introduce interval_probe_time_ms for periodic probe")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Currently, the sysctl range check is not applied to
interval_probe_time_ms, which needs this fix:
https://lore.kernel.org/linux-fsdevel/20260905233819.1064529-2-kuniyu@google.com/

Cc: Yuwei Wang <wangyuweihx@gmail.com>
---
 net/core/neighbour.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 4b17c2a15594..454a8b8f3aa0 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -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,
@@ -3670,12 +3678,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.1003.g10538fe699-goog

[PATCH v1 net 3/4] neighbour: Don't render blackhole_netdev via RTM_GETNEIGHTBL.

[Kuniyuki Iwashima]

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.

Fixes: e5f80fcf869a ("ipv6: give an IPv6 dev to blackhole_netdev")
Fixes: 22600596b675 ("ipv4: give an IPv4 dev to blackhole_netdev")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: Xin Long <lucien.xin@gmail.com>
---
 net/core/neighbour.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 454a8b8f3aa0..b90957b630ce 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2625,6 +2625,9 @@ 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 == blackhole_netdev)
+				continue;
+
 			if (nidx < neigh_skip)
 				goto next;
 
-- 
2.55.0.1003.g10538fe699-goog

[PATCH v1 net 4/4] neighbour: Skip default parms when resumed in neightbl_dump_info().

[Kuniyuki Iwashima]

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.

Fixes: c7fb64db001f ("[NETLINK]: Neighbour table configuration and statistics via rtnetlink")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: Thomas Graf <tgraf@suug.ch>
---
 net/core/neighbour.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index b90957b630ce..15eedaef6578 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -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;
 
 		p = list_first_entry(&tbl->parms_list, struct neigh_parms, list);
 		list_for_each_entry_continue_rcu(p, &tbl->parms_list, list) {
@@ -2642,12 +2644,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.1003.g10538fe699-goog

Re: [PATCH v1 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info().

[Ido Schimmel]

On Mon, Sep 07, 2026 at 09:57:52PM +0000, Kuniyuki Iwashima wrote:
> 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 fetch the default parms with list_first_entry() and
> use list_for_each_entry_continue_rcu() for iteration.
> 
> Note that the first entry is always tbl->parms, which never goes
> away, so list_first_entry(&tbl->parms_list, ...) is safe.
> 
> Fixes: 4ae34be50064 ("neighbour: Convert RTM_GETNEIGHTBL to RCU.")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  net/core/neighbour.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 1349c0eedb64..4b17c2a15594 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2611,8 +2611,9 @@ 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) {
> +
> +		p = list_first_entry(&tbl->parms_list, struct neigh_parms, list);
> +		list_for_each_entry_continue_rcu(p, &tbl->parms_list, list) {
>  			if (!net_eq(neigh_parms_net(p), net))
>  				continue;

I find this more readable:

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 1349c0eedb64..3af539f5fd4b 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2611,11 +2611,13 @@ 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;
 
Then patch #3 becomes:

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 3af539f5fd4b..b4d8e5b6832c 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2615,7 +2615,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)

Re: [PATCH v1 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.

[Ido Schimmel]

On Mon, Sep 07, 2026 at 09:57:53PM +0000, Kuniyuki Iwashima wrote:
> 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.

Please add a note that this controls the probe interval for "managed"
entries and therefore a max of 1 day is unlikely to break any
deployments.

> 
> The same max check is applied to sysctl as well.
> 
> Fixes: 211da42eaa45 ("net, neigh: introduce interval_probe_time_ms for periodic probe")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
> Currently, the sysctl range check is not applied to
> interval_probe_time_ms, which needs this fix:
> https://lore.kernel.org/linux-fsdevel/20260905233819.1064529-2-kuniyu@google.com/
> 
> Cc: Yuwei Wang <wangyuweihx@gmail.com>
> ---
>  net/core/neighbour.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)

The maximum value should be documented in Documentation/networking/ip-sysctl.rst

Also in Documentation/netlink/specs/rt-neigh.yaml:

diff --git a/Documentation/netlink/specs/rt-neigh.yaml b/Documentation/netlink/specs/rt-neigh.yaml
index 0f46ef313590..c8e55c98d564 100644
--- a/Documentation/netlink/specs/rt-neigh.yaml
+++ b/Documentation/netlink/specs/rt-neigh.yaml
@@ -341,6 +341,9 @@ attribute-sets:
       -
         name: interval-probe-time-ms
         type: u64
+        checks:
+          min: 1
+          max: 86400000
 
 operations:
   enum-model: directional

Re: [PATCH v1 net 4/4] neighbour: Skip default parms when resumed in neightbl_dump_info().

[Ido Schimmel]

On Mon, Sep 07, 2026 at 09:57:55PM +0000, Kuniyuki Iwashima wrote:
> 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.
> 
> Fixes: c7fb64db001f ("[NETLINK]: Neighbour table configuration and statistics via rtnetlink")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH v1 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info().

[Kuniyuki Iwashima]

On Tue, Sep 8, 2026 at 3:05 AM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Mon, Sep 07, 2026 at 09:57:52PM +0000, Kuniyuki Iwashima wrote:
> > 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 fetch the default parms with list_first_entry() and
> > use list_for_each_entry_continue_rcu() for iteration.
> >
> > Note that the first entry is always tbl->parms, which never goes
> > away, so list_first_entry(&tbl->parms_list, ...) is safe.
> >
> > Fixes: 4ae34be50064 ("neighbour: Convert RTM_GETNEIGHTBL to RCU.")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  net/core/neighbour.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index 1349c0eedb64..4b17c2a15594 100644
> > --- a/net/core/neighbour.c
> > +++ b/net/core/neighbour.c
> > @@ -2611,8 +2611,9 @@ 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) {
> > +
> > +             p = list_first_entry(&tbl->parms_list, struct neigh_parms, list);
> > +             list_for_each_entry_continue_rcu(p, &tbl->parms_list, list) {
> >                       if (!net_eq(neigh_parms_net(p), net))
> >                               continue;
>
> I find this more readable:

It's cleaner indeed.  Will change in v2.

Thanks !

>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 1349c0eedb64..3af539f5fd4b 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2611,11 +2611,13 @@ 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;
>
> Then patch #3 becomes:
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 3af539f5fd4b..b4d8e5b6832c 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2615,7 +2615,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)

Re: [PATCH v1 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.

[Kuniyuki Iwashima]

On Tue, Sep 8, 2026 at 3:06 AM Ido Schimmel <idosch@nvidia.com> wrote:
>
> On Mon, Sep 07, 2026 at 09:57:53PM +0000, Kuniyuki Iwashima wrote:
> > 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.
>
> Please add a note that this controls the probe interval for "managed"
> entries and therefore a max of 1 day is unlikely to break any
> deployments.

Acked.

>
> >
> > The same max check is applied to sysctl as well.
> >
> > Fixes: 211da42eaa45 ("net, neigh: introduce interval_probe_time_ms for periodic probe")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> > Currently, the sysctl range check is not applied to
> > interval_probe_time_ms, which needs this fix:
> > https://lore.kernel.org/linux-fsdevel/20260905233819.1064529-2-kuniyu@google.com/
> >
> > Cc: Yuwei Wang <wangyuweihx@gmail.com>
> > ---
> >  net/core/neighbour.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
>
> The maximum value should be documented in Documentation/networking/ip-sysctl.rst
>
> Also in Documentation/netlink/specs/rt-neigh.yaml:

Will add the changes.

Thanks !


>
> diff --git a/Documentation/netlink/specs/rt-neigh.yaml b/Documentation/netlink/specs/rt-neigh.yaml
> index 0f46ef313590..c8e55c98d564 100644
> --- a/Documentation/netlink/specs/rt-neigh.yaml
> +++ b/Documentation/netlink/specs/rt-neigh.yaml
> @@ -341,6 +341,9 @@ attribute-sets:
>        -
>          name: interval-probe-time-ms
>          type: u64
> +        checks:
> +          min: 1
> +          max: 86400000
>
>  operations:
>    enum-model: directional

Re: [PATCH v1 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 2

- [High] The sysctl half of this fix is dead code: in
  neigh_proc_dointvec_ms_jiffies_positive()…
- [Low] Documentation/netlink/specs/rt-neigh.yaml declares interval-
  probe-time-ms as a bare `type: u64` with no `checks:` block (lines…
- [Low] Documentation/networking/ip-sysctl.rst:249-253 documents
  neigh/default/interval_probe_time_ms as having only a lower bound…

Pre-existing issues:
- [Medium] Only NDTPA_INTERVAL_PROBE_TIME_MS gains range validation.

--

neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.

This adds NLA_POLICY_FULL_RANGE() validation to
NDTPA_INTERVAL_PROBE_TIME_MS so values outside [1, one day in ms] are
rejected on the RTM_SETNEIGHTBL path, and installs the same maximum on the
interval_probe_time_ms sysctl handler.  A few questions below.

> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 4b17c2a155947..454a8b8f3aa01 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -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,
> +};
> +

[Severity: Low]
Should Documentation/netlink/specs/rt-neigh.yaml be updated in the same
patch?  In the neighbour-table-parms attribute set the attribute is still
declared with no checks block:

      -
        name: interval-probe-time-ms
        type: u64

With nl_ntbl_parm_ms_range now enforcing [1, 86400000], ynl-generated
bindings and the generated documentation describe an unconstrained u64 for
an attribute the kernel rejects with -EINVAL outside a one-day window.
Would adding "checks: min: 1 / max: 86400000" here keep the spec and the
policy in agreement?

>  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 },

[Severity: Medium]
This isn't a bug introduced by this patch, but the sibling millisecond
attributes above have the same shape of problem and keep plain
{ .type = NLA_U64 }: NDTPA_BASE_REACHABLE_TIME, NDTPA_GC_STALETIME,
NDTPA_DELAY_PROBE_TIME, NDTPA_RETRANS_TIME, NDTPA_ANYCAST_DELAY,
NDTPA_PROXY_DELAY and NDTPA_LOCKTIME.

neightbl_set() feeds each of them through nla_get_msecs() into the int
NEIGH_VAR array:

			case NDTPA_BASE_REACHABLE_TIME:
				NEIGH_VAR_SET(p, BASE_REACHABLE_TIME,
					      nla_get_msecs(tbp[i]));

For a u64 above jiffies_to_msecs(MAX_JIFFY_OFFSET), msecs_to_jiffies()
clamps to MAX_JIFFY_OFFSET (0x3FFFFFFFFFFFFFFE on 64-bit), which narrows to
-2 in the int p->data[] slot.

That negative value then reaches the same sinks the commit message
describes, in neigh_periodic_work():

	queue_delayed_work(system_power_efficient_wq, &tbl->gc_work,
			      NEIGH_VAR(&tbl->parms, BASE_REACHABLE_TIME) >> 1);

and for the proxy delay:

	unsigned long proxy_delay = NEIGH_VAR(p, PROXY_DELAY);

	return proxy_delay ?
	       jiffies + get_random_u32_below(proxy_delay) : jiffies;

Since NTBL_PARM_MS_MAX and nl_ntbl_parm_ms_range are named generically
already, could they be reused for these attributes too, or is that
intentionally left for a follow-up?

> -	[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,

[ ... ]

> @@ -3670,12 +3678,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);

[Severity: High]
Does this half of the fix actually take effect at this commit?  As far as I
can tell proc_dointvec_ms_jiffies_minmax() never looks at extra1/extra2, so
both the new max and the pre-existing min are inert here.

kernel/time/jiffies.c:do_proc_int_conv_ms_jiffies_minmax() passes
k_ptr_range_check = false:

	return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, false,
			     sysctl_u2k_int_conv_ms, sysctl_k2u_int_conv_ms);

and kernel/sysctl.c:proc_int_conv() only performs the comparison when that
flag is set:

	if (k_ptr_range_check) {
		...
		if ((tbl->extra1 && *(int *)tbl->extra1 > tmp_k) ||
		    (tbl->extra2 && *(int *)tbl->extra2 < tmp_k))
			return -EINVAL;
		WRITE_ONCE(*k_ptr, tmp_k);
	} else
		return user_to_kern(negp, u_ptr, k_ptr);

The only remaining filter is proc_int_u2k_conv_uop(), which rejects
magnitudes above INT_MAX but lets 0 and negative values through:

	if (*negp) {
		if (u > (ulong) INT_MAX + 1)
			return -EINVAL;
		WRITE_ONCE(*k_ptr, -u);

So does

  echo 0 > /proc/sys/net/ipv4/neigh/default/interval_probe_time_ms

still store 0 jiffies, leaving the failure mode described in the commit
message reachable through sysctl in neigh_managed_work()?

	queue_delayed_work(system_power_efficient_wq, &tbl->managed_work,
			   NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME_MS));

If that is right, the commit message statement

    The same max check is applied to sysctl as well.

does not hold at this SHA, nor at the end of the series.  The prerequisite
sysctl range-check change is mentioned only below the scissors line:

    Currently, the sysctl range check is not applied to
    interval_probe_time_ms, which needs this fix:
    https://lore.kernel.org/linux-fsdevel/20260905233819.1064529-2-kuniyu@google.com/

That text is stripped when the patch is applied, so the permanent git log
keeps the unqualified claim.  Could the dependency be spelled out above the
scissors line, or the two patches ordered so this one lands after the
sysctl fix?

[Severity: Low]
Also, Documentation/networking/ip-sysctl.rst still documents only a lower
bound for this knob:

  neigh/default/interval_probe_time_ms - INTEGER
	The probe interval for neighbor entries with NTF_MANAGED flag,
	the min value is 1.

	Default: 5000

Should that be updated to mention the new 24 hour maximum?  The bound is
already observable on the RTM_SETNEIGHTBL path, and would become observable
on the sysctl file once the prerequisite range-check fix lands.

>  	neigh_proc_update(ctl, write);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907215853.3709987-1-kuniyu%40google.com

Re: [PATCH v1 net 4/4] neighbour: Skip default parms when resumed in neightbl_dump_info().

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 (1) · Low: 1

- [Low] net/core/neighbour.c: neightbl_dump_info() — the patch does two
  independent things but documents only one.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but it is the
  same user-visible symptom class the changelog addresses and remains…

--

neighbour: Skip default parms when resumed in neightbl_dump_info().

This adds a new cb->args[2] flag (default_skip) so that
neightbl_dump_info() does not re-emit the default parms message for a
table when a multi-part dump resumes in the middle of that table's
per-device parms walk.

> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index b90957b630cee..15eedaef65781 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -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;

[Severity: Low]
Is the removal of nidx = 0 from this spot, and its reappearance at the
tail of the per-table loop, a second and separate behaviour fix?

Before the patch, nidx was only reset right after the successful
neightbl_fill_info() call, never at the end of a table walk. So when the
skb fills up and neightbl_fill_info() fails for table N, the break path
stores the previous table's parm count:

	/* table 0 walked to completion, nidx == N */
	/* table 1: neightbl_fill_info() fails, break */
	cb->args[0] = 1;
	cb->args[1] = N;	/* table 0's count */

On resume neigh_skip is N while nidx restarts at 0 for table 1, so the
first N per-device parms of table 1 take:

			if (nidx < neigh_skip)
				goto next;

and are never emitted, which is the opposite symptom from the duplicate
described in the changelog.

After the patch nidx = 0 runs at the loop tail, so the break path stores
cb->args[1] = 0 and nothing is skipped. The changelog does not mention
this hunk at all. Could the changelog say that moving nidx = 0 fixes the
stale-nidx-on-break case, or could that hunk be split into its own patch?
As written it reads like cosmetic reordering that could be dropped during
review or a stable backport.

>  
>  		p = list_first_entry(&tbl->parms_list, struct neigh_parms, list);
>  		list_for_each_entry_continue_rcu(p, &tbl->parms_list, list) {
> @@ -2642,12 +2644,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;

[Severity: Medium]
This is a pre-existing issue and not introduced by this patch, but it is
the same class of user-visible symptom the changelog is fixing, so it may
be worth mentioning.

The parms walk in neightbl_dump_info() resumes by position index only.
rcu_read_lock() is dropped between dump invocations and no reference to
the last dumped struct neigh_parms is kept, so cb->args[1] can refer to a
different object next time around.

Writers mutate the same list under tbl->lock. neigh_parms_alloc() inserts
at the front of the region the dump iterates:

net/core/neighbour.c:neigh_parms_alloc() {
	...
	spin_lock_bh(&tbl->lock);
	list_add_rcu(&p->list, &tbl->parms.list);
	spin_unlock_bh(&tbl->lock);
	...
}

and neigh_parms_release() removes from it:

net/core/neighbour.c:neigh_parms_release() {
	...
	spin_lock_bh(&tbl->lock);
	list_del_rcu(&parms->list);
	parms->dead = 1;
	spin_unlock_bh(&tbl->lock);
	...
}

So if one invocation emits parms 0..4 and stores cb->args[1] = 5, and a
netdev is registered before the dump resumes, every previously dumped
entry shifts to index+1 and the entry formerly at index 4 is emitted a
second time. A netdev unregister shifts indices down instead and an entry
is silently dropped.

Neither cb->seq nor NLM_F_DUMP_INTR is used anywhere in
net/core/neighbour.c, so user space cannot detect this. Should a
generation counter plus NLM_F_DUMP_INTR be added on top of this patch for
the RTM_GETNEIGHTBL dump?

>  
>  	return skb->len;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907215853.3709987-1-kuniyu%40google.com