[PATCH net v2] net: neighbour: Serialize proxy timer teardown

[Runyu Xiao]

__neigh_ifdown() purges device-specific entries and then checks whether
the shared proxy queue is empty before deleting proxy_timer. A concurrent
ARP or NDISC request can enqueue a delayed proxy response between the check
and timer_delete_sync(). The timer deletion can then leave the new skb
queued without a timer to process it.

Serialize proxy queue insertion and timer rearming with the purge, empty
check, and timer deletion. The timer callback does not take the new lock,
so timer_delete_sync() can still wait for an in-flight callback. Keep
timer_delete_sync() rather than timer_shutdown_sync(), since the timer is
reused after per-device and carrier teardown.

A check of netif_running() in pneigh_enqueue() alone would not serialize
the insertion with the purge, empty check, and timer deletion. An enqueue
can pass that check while the device is still up and then be delayed until
after __neigh_ifdown() has observed an empty queue. Also, __neigh_ifdown()
is used by neigh_carrier_down(), where netif_running() can remain true.

Tested in an x86_64 QEMU guest with proxy ARP, three network namespaces,
and veth pairs. An AF_PACKET helper sent valid broadcast ARP requests while
the proxy-ARP ingress interface was repeatedly brought down and up. A
temporary test-only build flag added a 5 ms delay before pneigh_enqueue()
and after the empty-queue check before timer_delete_sync() to amplify the
narrow scheduling window.

The unfixed kernel reached both paths and observed a non-empty proxy_queue
with proxy_timer not pending 772 times. The fixed kernel reached both paths
under the same workload without observing this state. Natural-load runs
reached both paths but did not hit the narrow race without the temporary
delay.

Fixes: 66ba215cb513 ("neigh: fix possible DoS due to net iface start/stop loop")

Assisted-by: LLM Codex
Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
---
v2:
- Add the QEMU reproduction and buggy/fixed results.
- Explain why a device-state check does not serialize carrier teardown.

 include/net/neighbour.h | 2 ++
 net/core/neighbour.c    | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 8860cc2175fc1c9eb6b67f2c5ef149188c9feb16..15107c00b4df473e53e6cf2e757fb2ef3c4fd3f7 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -239,6 +239,8 @@ struct neigh_table {
 	struct list_head	gc_list;
 	struct list_head	managed_list;
 	spinlock_t		lock;
+	/* Serializes proxy timer rearming with teardown. */
+	spinlock_t		proxy_timer_lock;
 	unsigned long		last_rand;
 	struct neigh_statistics	__percpu *stats;
 	struct neigh_hash_table __rcu *nht;
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 1349c0eedb642539b28391390fd82f543e004353..1ea794a541efb6852700062da2e25efac6c060e8 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -471,10 +471,13 @@ static int __neigh_ifdown(struct neigh_table *tbl, struct net_device *dev,
 	spin_unlock_bh(&tbl->lock);
 
 	pneigh_ifdown(tbl, dev, skip_perm);
+	/* The callback does not take this lock, so sync deletion can wait for it. */
+	spin_lock_bh(&tbl->proxy_timer_lock);
 	pneigh_queue_purge(&tbl->proxy_queue, dev ? dev_net(dev) : NULL,
 			   tbl->family);
 	if (skb_queue_empty_lockless(&tbl->proxy_queue))
 		timer_delete_sync(&tbl->proxy_timer);
+	spin_unlock_bh(&tbl->proxy_timer_lock);
 	return 0;
 }
 
@@ -1729,6 +1732,7 @@ void pneigh_enqueue(struct neigh_table *tbl, struct neigh_parms *p,
 	NEIGH_CB(skb)->sched_next = sched_next;
 	NEIGH_CB(skb)->flags |= LOCALLY_ENQUEUED;
 
+	spin_lock_bh(&tbl->proxy_timer_lock);
 	spin_lock(&tbl->proxy_queue.lock);
 	if (timer_delete(&tbl->proxy_timer)) {
 		if (time_before(tbl->proxy_timer.expires, sched_next))
@@ -1740,6 +1744,7 @@ void pneigh_enqueue(struct neigh_table *tbl, struct neigh_parms *p,
 	p->qlen++;
 	mod_timer(&tbl->proxy_timer, sched_next);
 	spin_unlock(&tbl->proxy_queue.lock);
+	spin_unlock_bh(&tbl->proxy_timer_lock);
 }
 EXPORT_SYMBOL(pneigh_enqueue);
 
@@ -1858,6 +1863,7 @@ void neigh_table_init(int index, struct neigh_table *tbl)
 		WARN_ON(tbl->entry_size % NEIGH_PRIV_ALIGN);
 
 	spin_lock_init(&tbl->lock);
+	spin_lock_init(&tbl->proxy_timer_lock);
 	mutex_init(&tbl->phash_lock);
 
 	INIT_DEFERRABLE_WORK(&tbl->gc_work, neigh_periodic_work);
-- 
2.34.1