Skip to content

Commit 6f2cd30

Browse files
Kalesh APgregkh
authored andcommitted
bnxt_en: Fix double invocation of bnxt_ulp_stop()/bnxt_ulp_start()
[ Upstream commit 1e9ac33 ] Before the commit under the Fixes tag below, bnxt_ulp_stop() and bnxt_ulp_start() were always invoked in pairs. After that commit, the new bnxt_ulp_restart() can be invoked after bnxt_ulp_stop() has been called. This may result in the RoCE driver's aux driver .suspend() method being invoked twice. The 2nd bnxt_re_suspend() call will crash when it dereferences a NULL pointer: (NULL ib_device): Handle device suspend call BUG: kernel NULL pointer dereference, address: 0000000000000b78 PGD 0 P4D 0 Oops: Oops: 0000 [#1] SMP PTI CPU: 20 UID: 0 PID: 181 Comm: kworker/u96:5 Tainted: G S 6.15.0-rc1 #4 PREEMPT(voluntary) Tainted: [S]=CPU_OUT_OF_SPEC Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.4.3 01/17/2017 Workqueue: bnxt_pf_wq bnxt_sp_task [bnxt_en] RIP: 0010:bnxt_re_suspend+0x45/0x1f0 [bnxt_re] Code: 8b 05 a7 3c 5b f5 48 89 44 24 18 31 c0 49 8b 5c 24 08 4d 8b 2c 24 e8 ea 06 0a f4 48 c7 c6 04 60 52 c0 48 89 df e8 1b ce f9 ff <48> 8b 83 78 0b 00 00 48 8b 80 38 03 00 00 a8 40 0f 85 b5 00 00 00 RSP: 0018:ffffa2e84084fd88 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000001 RDX: 0000000000000000 RSI: ffffffffb4b6b934 RDI: 00000000ffffffff RBP: ffffa1760954c9c0 R08: 0000000000000000 R09: c0000000ffffdfff R10: 0000000000000001 R11: ffffa2e84084fb50 R12: ffffa176031ef070 R13: ffffa17609775000 R14: ffffa17603adc180 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffffa17daa397000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000b78 CR3: 00000004aaa30003 CR4: 00000000003706f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> bnxt_ulp_stop+0x69/0x90 [bnxt_en] bnxt_sp_task+0x678/0x920 [bnxt_en] ? __schedule+0x514/0xf50 process_scheduled_works+0x9d/0x400 worker_thread+0x11c/0x260 ? __pfx_worker_thread+0x10/0x10 kthread+0xfe/0x1e0 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x2b/0x40 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 Check the BNXT_EN_FLAG_ULP_STOPPED flag and do not proceed if the flag is already set. This will preserve the original symmetrical bnxt_ulp_stop() and bnxt_ulp_start(). Also, inside bnxt_ulp_start(), clear the BNXT_EN_FLAG_ULP_STOPPED flag after taking the mutex to avoid any race condition. And for symmetry, only proceed in bnxt_ulp_start() if the BNXT_EN_FLAG_ULP_STOPPED is set. Fixes: 3c163f3 ("bnxt_en: Optimize recovery path ULP locking in the driver") Signed-off-by: Kalesh AP <[email protected]> Co-developed-by: Michael Chan <[email protected]> Signed-off-by: Michael Chan <[email protected]> Reviewed-by: Simon Horman <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent ac462a7 commit 6f2cd30

File tree

1 file changed

+10
-14
lines changed

1 file changed

+10
-14
lines changed

drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,9 @@ void bnxt_ulp_stop(struct bnxt *bp)
230230
return;
231231

232232
mutex_lock(&edev->en_dev_lock);
233-
if (!bnxt_ulp_registered(edev)) {
234-
mutex_unlock(&edev->en_dev_lock);
235-
return;
236-
}
233+
if (!bnxt_ulp_registered(edev) ||
234+
(edev->flags & BNXT_EN_FLAG_ULP_STOPPED))
235+
goto ulp_stop_exit;
237236

238237
edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
239238
if (aux_priv) {
@@ -249,6 +248,7 @@ void bnxt_ulp_stop(struct bnxt *bp)
249248
adrv->suspend(adev, pm);
250249
}
251250
}
251+
ulp_stop_exit:
252252
mutex_unlock(&edev->en_dev_lock);
253253
}
254254

@@ -257,19 +257,13 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
257257
struct bnxt_aux_priv *aux_priv = bp->aux_priv;
258258
struct bnxt_en_dev *edev = bp->edev;
259259

260-
if (!edev)
261-
return;
262-
263-
edev->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
264-
265-
if (err)
260+
if (!edev || err)
266261
return;
267262

268263
mutex_lock(&edev->en_dev_lock);
269-
if (!bnxt_ulp_registered(edev)) {
270-
mutex_unlock(&edev->en_dev_lock);
271-
return;
272-
}
264+
if (!bnxt_ulp_registered(edev) ||
265+
!(edev->flags & BNXT_EN_FLAG_ULP_STOPPED))
266+
goto ulp_start_exit;
273267

274268
if (edev->ulp_tbl->msix_requested)
275269
bnxt_fill_msix_vecs(bp, edev->msix_entries);
@@ -286,6 +280,8 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
286280
adrv->resume(adev);
287281
}
288282
}
283+
ulp_start_exit:
284+
edev->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
289285
mutex_unlock(&edev->en_dev_lock);
290286
}
291287

0 commit comments

Comments
 (0)