Re: [PATCH rdma-next] IB/core: Fix zero dmac race in neighbor resolution

Jason Gunthorpe
2026-04-09T15:28:50Z

On Sun, Apr 05, 2026 at 06:44:55PM +0300, Leon Romanovsky wrote:
> From: Chen Zhao <chezhao@nvidia.com>
>
> dst_fetch_ha() checks nud_state without holding the neighbor lock, then
> copies ha under the seqlock. A race in __neigh_update() where nud_state
> is set to NUD_REACHABLE before ha is written allows dst_fetch_ha() to
> read a zero MAC address while the seqlock reports no concurrent writer.
>
> netevent_callback amplifies this by waking ALL pending addr_req workers
> when ANY neighbor becomes NUD_VALID. At scale (N peers resolving ARP
> concurrently), the hit probability scales as N^2, making it near-certain
> for large RDMA workloads.
>
> N(A): neigh_update(A) W(A): addr_resolve(A)
> | [sleep]
> | write_lock_bh(&A->lock) |
> | A->nud_state = NUD_REACHABLE |
> | // A->ha is still 0 |
> | [woken by netevent_cb() of
> | another neighbour]
> | | dst_fetch_ha(A)
> | | A->nud_state & NUD_VALID
> | | read_seqbegin(&A->ha_lock)
> | | snapshot = A->ha /* 0 */
> | | read_seqretry(&A->ha_lock)
> | | return snapshot
> | seqlock(&A->ha_lock)
> | A->ha = mac_A /* too late */
> | sequnlock(&A->ha_lock)
> | write_unlock_bh(&A->lock)
>
> The incorrect/zero mac is read and programmed in the device QP while it
> was not yet updated. This causes silent packet loss and eventual
> RETRY_EXC_ERR.
>
> Fix by holding the neighbor read lock across the nud_state check and
> ha copy in dst_fetch_ha(), ensuring it synchronizes with
> __neigh_update() which is updating while holding the write lock.
>
> Fixes: 92ebb6a0a13a ("IB/cm: Remove now useless rcu_lock in dst_fetch_ha")
> Signed-off-by: Chen Zhao <chezhao@nvidia.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
> Strictly speaking the commit in Fixes doesn't look like the one which
> caused the race, but it is most relevant one to put.
> ---
> drivers/infiniband/core/addr.c | 3 +++
> 1 file changed, 3 insertions(+)

Applied to for-next

Thanks,
Jason