Re: [PATCH net-next] net: neighbour: Avoid sending RTM_NEWNEIGH for a dead neighbor

[Kuniyuki Iwashima <kuniyu@google.com>]

On Wed, 04 Nov 2026 14:56:56 +0100, Petr Machata wrote:
> During the processing of neighbor timer, in neigh_timer_handler(),
> when the number of probes for an unresolved neighbor is exceeded, a
> non-ext_validated neighbor is marked as failed. To send the ICMP /
> ICMPv6 destination unreachable diagnostic, neigh_invalidate() unlocks,
> and gives opportunity for a concurrent update to mark the neighbor as
> dead and send an RTM_DELNEIGH notification. After the lock is
> regained, neigh_timer_handler() proceeds to send its own RTM_NEWNEIGH
> notification, this time on a dead neighbor.
> 
> Sending an RTM_NEWNEIGH after the corresponding RTM_DELNEIGH has the
> obvious semantics of recreating the neighbor, which is not what we want to
> convey here.
> 
> Userspace can in theory recognize these faulty RTM_NEWNEIGH reports by
> the state, which will be NUD_NONE, but there is no reason why it
> should.
> 
> The following diagram, which illustrates the issue, comes from Andy
> Roulin:
> 
> timer handler (softirq)                neigh_flush_dev() (linkwatch)
> write_lock(n->lock)
> nud_state = NUD_FAILED
> neigh_invalidate():
>   write_unlock(n->lock)
>   error_report()
>                                        write_lock(n->lock)
>                                        unlink, n->dead = 1
>                                        refcnt != 1 -> nud_state = NUD_NONE
>                                        write_unlock(n->lock)
>                                        RTM_DELNEIGH
>   write_lock(n->lock)
> write_unlock(n->lock)
> RTM_NEWNEIGH: state 0, no lladdr, ndm_refcnt 0   <- stale
> neigh_release() -> neigh_destroy()
> 
> Fix the problem by refusing to send any notification other than
> RTM_DELNEIGH when the neighbor is dead.
> 
> The issue comes from a user report and was obseved in the wild. It was
> reproduced on a vanilla kernel by extending the race window by inserting an
> mdelay for neighbors on a device with a hardcoded name.
> 
> It is tricky to track down where the issue was actually introduced, and
> practically speaking, it may have been present "always". At least as far
> back as 5.10, which is currently the oldest LTS-supported kernel release,
> the code still looks substantially the same: neighbor timer causes neighbor
> invalidation, which unlocks to send ICMP, which a flush can then race. Even
> 2.6 has elements that look like they could combine into the same race. I am
> therefore not citing any particular Fixes tag.
> 
> Assisted-by: LLM
> Co-developed-by: Andy Roulin <aroulin@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Andy Roulin <aroulin@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> ---
>  net/core/neighbour.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index d6e9c7c59a89..6fa490f7652a 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -3593,6 +3593,13 @@ static void __neigh_notify(struct neighbour *n, int type, int flags,
>  	int err = -ENOBUFS;
>  	struct net *net;
>  
> +	/* An attempt to send anything but RTM_DELNEIGH on a dead neighbor
> +	 * indicates that the neighbor was marked dead while another update
> +	 * gave the lock up for a bit. Do not send such notifications.
> +	 */

I'd remove the comment, it's apparent from ->dead that there
was a race and the rest is just repeating the condition bleow.

Otherwise looks good.

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>



> +	if (n->dead && type != RTM_DELNEIGH)
> +		return;
> +
>  	rcu_read_lock();
>  	net = dev_net_rcu(n->dev);
>  	skb = nlmsg_new(neigh_nlmsg_size(), GFP_ATOMIC);
> -- 
> 2.56.0
>