public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH rdma-next] IB/core: Fix zero dmac race in neighbor resolution
@ 2026-04-05 15:44 Leon Romanovsky
  2026-04-09 15:28 ` Jason Gunthorpe
  0 siblings, 1 reply; 2+ messages in thread
From: Leon Romanovsky @ 2026-04-05 15:44 UTC (permalink / raw)
  To: Jason Gunthorpe, Leon Romanovsky
  Cc: linux-rdma, linux-kernel, Chen Zhao, Parav Pandit

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(+)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 866746695712a..6526fda8f9c0b 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -321,11 +321,14 @@ static int dst_fetch_ha(const struct dst_entry *dst,
 	if (!n)
 		return -ENODATA;
 
+	read_lock_bh(&n->lock);
 	if (!(n->nud_state & NUD_VALID)) {
+		read_unlock_bh(&n->lock);
 		neigh_event_send(n, NULL);
 		ret = -ENODATA;
 	} else {
 		neigh_ha_snapshot(dev_addr->dst_dev_addr, n, dst->dev);
+		read_unlock_bh(&n->lock);
 	}
 
 	neigh_release(n);

---
base-commit: 69db255d5fafb5651013c79e54c1d535fc5015fb
change-id: 20260405-fix-dmac-race-1c57aa966d4d

Best regards,
--  
Leon Romanovsky <leonro@nvidia.com>


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

* Re: [PATCH rdma-next] IB/core: Fix zero dmac race in neighbor resolution
  2026-04-05 15:44 [PATCH rdma-next] IB/core: Fix zero dmac race in neighbor resolution Leon Romanovsky
@ 2026-04-09 15:28 ` Jason Gunthorpe
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Gunthorpe @ 2026-04-09 15:28 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: linux-rdma, linux-kernel, Chen Zhao, Parav Pandit

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

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

end of thread, other threads:[~2026-04-09 15:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-05 15:44 [PATCH rdma-next] IB/core: Fix zero dmac race in neighbor resolution Leon Romanovsky
2026-04-09 15:28 ` Jason Gunthorpe

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