All of lore.kernel.org
 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
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ 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] 5+ 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
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ 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] 5+ 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-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
  3 siblings, 0 replies; 5+ 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] 5+ 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
  3 siblings, 0 replies; 5+ 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] 5+ 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
  3 siblings, 0 replies; 5+ 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] 5+ messages in thread

end of thread, other threads:[~2026-09-09 23:31 UTC | newest]

Thread overview: 5+ 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-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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.