Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL.
@ 2026-09-09 23:31 Kuniyuki Iwashima
  2026-09-09 23:31 ` [PATCH v3 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info() Kuniyuki Iwashima
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-09 23:31 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Ido Schimmel, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

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/


Changes:
  v3:
    * Patch 2
      * Nest min/max inside checks: in rt-neigh.yaml

  v2: https://lore.kernel.org/netdev/20260908211419.882954-1-kuniyu@google.com/
    * 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/20260907215853.3709987-1-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().

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

-- 
2.55.0.1003.g10538fe699-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info().
  2026-09-09 23:31 [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL Kuniyuki Iwashima
@ 2026-09-09 23:31 ` Kuniyuki Iwashima
  2026-09-09 23:31 ` [PATCH v3 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS Kuniyuki Iwashima
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-09 23:31 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Ido Schimmel, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev

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.

Fixes: 4ae34be50064 ("neighbour: Convert RTM_GETNEIGHTBL to RCU.")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
v2: Use list_for_each_entry_rcu() and skip the default parms
---
 net/core/neighbour.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 1349c0eedb64..49dd7df149ef 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -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.1003.g10538fe699-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.
  2026-09-09 23:31 [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL Kuniyuki Iwashima
  2026-09-09 23:31 ` [PATCH v3 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info() Kuniyuki Iwashima
@ 2026-09-09 23:31 ` Kuniyuki Iwashima
  2026-09-11  1:15   ` netdev-bot+sashiko
  2026-09-09 23:31 ` [PATCH v3 net 3/4] neighbour: Don't render blackhole_netdev via RTM_GETNEIGHTBL Kuniyuki Iwashima
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-09 23:31 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Ido Schimmel, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev, Yuwei Wang

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, so the max of 1 day is unlikely to break any
deployments.

Fixes: 211da42eaa45 ("net, neigh: introduce interval_probe_time_ms for periodic probe")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.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>

v3:
  * Nest min/max inside checks: in rt-neigh.yaml
v2:
  * Clarify the attribute is for NTF_MANAGED
  * Update docs, rt-neigh.yaml and ip-sysctl.rst
---
 Documentation/netlink/specs/rt-neigh.yaml |  3 +++
 Documentation/networking/ip-sysctl.rst    |  2 +-
 net/core/neighbour.c                      | 17 +++++++++++++----
 3 files changed, 17 insertions(+), 5 deletions(-)

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
diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 208f46967ee5..b05829e44d8f 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -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
 
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 49dd7df149ef..0db78a0dfb51 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,
@@ -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.1003.g10538fe699-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 net 3/4] neighbour: Don't render blackhole_netdev via RTM_GETNEIGHTBL.
  2026-09-09 23:31 [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL Kuniyuki Iwashima
  2026-09-09 23:31 ` [PATCH v3 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info() Kuniyuki Iwashima
  2026-09-09 23:31 ` [PATCH v3 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS Kuniyuki Iwashima
@ 2026-09-09 23:31 ` Kuniyuki Iwashima
  2026-09-09 23:31 ` [PATCH v3 net 4/4] neighbour: Skip default parms when resumed in neightbl_dump_info() Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-09 23:31 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Ido Schimmel, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev, Xin Long

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>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
Cc: Xin Long <lucien.xin@gmail.com>
---
 net/core/neighbour.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 0db78a0dfb51..02bf940ce00a 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -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.1003.g10538fe699-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 net 4/4] neighbour: Skip default parms when resumed in neightbl_dump_info().
  2026-09-09 23:31 [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2026-09-09 23:31 ` [PATCH v3 net 3/4] neighbour: Don't render blackhole_netdev via RTM_GETNEIGHTBL Kuniyuki Iwashima
@ 2026-09-09 23:31 ` Kuniyuki Iwashima
  2026-09-12  0:30 ` [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL patchwork-bot+netdevbpf
  2026-09-12  0:30 ` Jakub Kicinski
  5 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-09 23:31 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Ido Schimmel, Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev, Thomas Graf

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.

Also, nidx must be cleared at the end of each table loop;
otherwise, if neightbl_fill_info() for a subsequent table
fails, the leftover nidx from the previous table would be
saved in cb->args[1], resulting in erroneously skipping parms
of the subsequent table in the next dump.

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>
---
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 02bf940ce00a..7448320f7ad5 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;
 
 		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.1003.g10538fe699-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.
  2026-09-09 23:31 ` [PATCH v3 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS Kuniyuki Iwashima
@ 2026-09-11  1:15   ` netdev-bot+sashiko
  2026-09-11 17:40     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-09-11  1:15 UTC (permalink / raw)
  To: kuniyu
  Cc: davem, edumazet, kuba, pabeni, idosch, horms, kuni1840, netdev,
	tgraf

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.

> diff --git a/Documentation/netlink/specs/rt-neigh.yaml b/Documentation/netlink/specs/rt-neigh.yaml
> index 0f46ef3135905..c8e55c98d5649 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
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index 208f46967ee59..b05829e44d8fc 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -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
>  
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 49dd7df149ef3..0db78a0dfb516 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,
> +};
> +

[ ... ]

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.
  2026-09-11  1:15   ` netdev-bot+sashiko
@ 2026-09-11 17:40     ` Kuniyuki Iwashima
  0 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-11 17:40 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, idosch, horms, kuni1840, netdev,
	tgraf

On Thu, Sep 10, 2026 at 6:15 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> 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.
>
> > diff --git a/Documentation/netlink/specs/rt-neigh.yaml b/Documentation/netlink/specs/rt-neigh.yaml
> > index 0f46ef3135905..c8e55c98d5649 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
> > diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> > index 208f46967ee59..b05829e44d8fc 100644
> > --- a/Documentation/networking/ip-sysctl.rst
> > +++ b/Documentation/networking/ip-sysctl.rst
> > @@ -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
> >
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index 49dd7df149ef3..0db78a0dfb516 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,
> > +};
> > +
>
> [ ... ]
>
> > @@ -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);
>
> [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/20260905233819.1064529-2-kuniyu@google.com/)
> as a prerequisite.  Since that note does not end up in git history,

Good to know Sashiko reads comments under --- :)

The fix was already queued for sysctl-fixes, no need to wait.
https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/commit/?h=jag/sysctl-fixes&id=1bb4e0f7a019b6b34228efbcc9c7399e3dbf72a8


> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL.
  2026-09-09 23:31 [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL Kuniyuki Iwashima
                   ` (3 preceding siblings ...)
  2026-09-09 23:31 ` [PATCH v3 net 4/4] neighbour: Skip default parms when resumed in neightbl_dump_info() Kuniyuki Iwashima
@ 2026-09-12  0:30 ` patchwork-bot+netdevbpf
  2026-09-12  0:30 ` Jakub Kicinski
  5 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-12  0:30 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: davem, edumazet, kuba, pabeni, idosch, horms, kuni1840, netdev

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  9 Sep 2026 23:31:22 +0000 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [v3,net,1/4] neighbour: Add missing RCU annotation for neightbl_dump_info().
    https://git.kernel.org/netdev/net/c/764dcebb0337
  - [v3,net,2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS.
    https://git.kernel.org/netdev/net/c/6d79b223ec44
  - [v3,net,3/4] neighbour: Don't render blackhole_netdev via RTM_GETNEIGHTBL.
    https://git.kernel.org/netdev/net/c/7b430fcfc972
  - [v3,net,4/4] neighbour: Skip default parms when resumed in neightbl_dump_info().
    https://git.kernel.org/netdev/net/c/979aabdad8dd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL.
  2026-09-09 23:31 [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL Kuniyuki Iwashima
                   ` (4 preceding siblings ...)
  2026-09-12  0:30 ` [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL patchwork-bot+netdevbpf
@ 2026-09-12  0:30 ` Jakub Kicinski
  2026-09-12  0:32   ` Kuniyuki Iwashima
  5 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-09-12  0:30 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: David S . Miller, Eric Dumazet, Paolo Abeni, Ido Schimmel,
	Simon Horman, Kuniyuki Iwashima, netdev

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:

diff --cc net/core/neighbour.c
index 27602bcbeb69,7448320f7ad5..000000000000
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@@ -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;
  
?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL.
  2026-09-12  0:30 ` Jakub Kicinski
@ 2026-09-12  0:32   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-12  0:32 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S . Miller, Eric Dumazet, Paolo Abeni, Ido Schimmel,
	Simon Horman, Kuniyuki Iwashima, netdev

On Fri, Sep 11, 2026 at 5:30 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> 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:
>
> diff --cc net/core/neighbour.c
> index 27602bcbeb69,7448320f7ad5..000000000000
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@@ -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;
>
> ?

Looks good, thank you !

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-09-12  0:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 23:31 [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL Kuniyuki Iwashima
2026-09-09 23:31 ` [PATCH v3 net 1/4] neighbour: Add missing RCU annotation for neightbl_dump_info() Kuniyuki Iwashima
2026-09-09 23:31 ` [PATCH v3 net 2/4] neighbour: Enforce min/max to NDTPA_INTERVAL_PROBE_TIME_MS Kuniyuki Iwashima
2026-09-11  1:15   ` netdev-bot+sashiko
2026-09-11 17:40     ` Kuniyuki Iwashima
2026-09-09 23:31 ` [PATCH v3 net 3/4] neighbour: Don't render blackhole_netdev via RTM_GETNEIGHTBL Kuniyuki Iwashima
2026-09-09 23:31 ` [PATCH v3 net 4/4] neighbour: Skip default parms when resumed in neightbl_dump_info() Kuniyuki Iwashima
2026-09-12  0:30 ` [PATCH v3 net 0/4] neighbour: Small fixes for RTM_{GET,SET}NEIGHTBL patchwork-bot+netdevbpf
2026-09-12  0:30 ` Jakub Kicinski
2026-09-12  0:32   ` Kuniyuki Iwashima

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox