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

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

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

- [High] The sysctl half of this patch is inert in this tree:
neigh_proc_dointvec_ms_jiffies_positive()…

--

neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.

The patch replaces the ineffective .min-only policy entry for NDTPA_INTERVAL_PROBE_TIME_MS with NLA_POLICY_FULL_RANGE(), bounding it to 1..86400000 ms, and adds the same max to the sysctl handler plus the netlink spec and ip-sysctl documentation.

One question about the sysctl half of the change below.

Quoted 42 lines

Documentation/netlink/specs/rt-neigh.yaml +3 -0

index 0f46ef3135905..c8e55c98d5649 100644
@@ -341,6 +341,9 @@ attribute-sets:
       -
         name: interval-probe-time-ms
         type: u64
+        checks:
+          min: 1
+          max: 86400000
 
 operations:
   enum-model: directional

Documentation/networking/ip-sysctl.rst +1 -1

index 208f46967ee59..b05829e44d8fc 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
 

net/core/neighbour.c +7 -0

index 49dd7df149ef3..0db78a0dfb516 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,
+};
+

[ ... ]

Quoted 6 lines

net/core/neighbour.c +1 -1

@@ -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;

net/core/neighbour.c static int neigh_proc_dointvec_ms_jiffies_positive(const struct ctl_table *ctl, · line 3691

Quoted 10 lines

net/core/neighbour.c +3 -2

 
-     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 proc_dointvec_ms_jiffies_minmax() actually look at extra1/extra2 in this tree? The call chain from neigh_proc_dointvec_ms_jiffies_positive() seems to drop the range check:

kernel/time/jiffies.c:

static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
int *k_ptr, int dir, const struct ctl_table *tbl)
{
return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, false,
sysctl_u2k_int_conv_ms, sysctl_k2u_int_conv_ms);
}

kernel/sysctl.c:proc_int_conv() only consults the bounds when k_ptr_range_check is true:

        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);

Since the converter passes false, aren't both tmp.extra1 = &min and the new tmp.extra2 = &max dead stores, so a write of 0, of a value well above 86400000, or of a negative value to /proc/sys/net/ipv{4,6}/neigh/*/interval_probe_time_ms still succeeds?

If so, the failure mode described in the commit message stays reachable via the sysctl entry point, where neigh_managed_work() requeues itself with the unvalidated value:

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

with 0 meaning no delay and a negative int sign-extending into the unsigned long delay argument.

That would also make this sentence in the commit message inaccurate:

The same max check is applied to sysctl as well.

and likewise the new documentation text:

the min value is 1, and the max value is 86400000 (1 day).

The netlink half looks fine, so the sysctl path and the range now declared in Documentation/netlink/specs/rt-neigh.yaml would disagree: an out-of-range sysctl write can still be reported back by RTM_GETNEIGHTBL as a value outside the declared 1..86400000 range.

The cover note below the --- line points at a separate, not-yet-merged sysctl-core fix
(https://lore.kernel.org/linux-fsdevel/[email protected]/)
as a prerequisite. Since that note does not end up in git history,

would it
make sense to order this patch behind that fix, or to mention the dependency in the commit message itself? Note this commit carries a Fixes: tag, so a stable backport would carry the documented bound without the enforcement.

I checked the end of this series and do_proc_int_conv_ms_jiffies_minmax() still passes false, so nothing later in the series appears to restore the check.

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

While working on the follow-up suggested here,

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

Quoted 3 lines · On Wed, 9 Sep 2026 23:31:22 +0000 Kuniyuki Iwashima wrote:

While working on the follow-up suggested here,

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

Conflict resolution with net-next:

net/core/neighbour.c +6 -3

diff --cc net/core/neighbour.c
index 27602bcbeb69,7448320f7ad5..000000000000
@@@ -2659,9 -2620,15 +2669,12 @@@ static int neightbl_dump_info(struct sk
                                       NLM_F_MULTI) < 0)
                        break;
 
-               nidx = 0;
-               p = list_next_entry(&tbl->parms, list);
-               list_for_each_entry_from_rcu(p, &tbl->parms_list, list) {
+               default_skip = 1;
+
+               list_for_each_entry_rcu(p, &tbl->parms_list, list) {
 -                      if (!net_eq(neigh_parms_net(p), net))
 -                              continue;
 -
+                       if (!p->dev || p->dev == blackhole_netdev)
+                               continue;
+
                        if (nidx < neigh_skip)
                                goto next;

?