Skip to content

[PWCI] "net/bnxt: fix a segmentation fault" #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ovsrobot wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[PWCI] "net/bnxt: fix a segmentation fault" #526

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions drivers/net/bnxt/bnxt_hwrm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1198,15 +1198,6 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
if (BNXT_PF(bp))
bp->pf->total_vnics = rte_le_to_cpu_16(resp->max_vnics);

if (flags & HWRM_FUNC_QCAPS_OUTPUT_FLAGS_PTP_SUPPORTED) {
if (BNXT_CHIP_P5(bp) || BNXT_PF(bp)) {
bp->flags |= BNXT_FLAG_PTP_SUPPORTED;
PMD_DRV_LOG_LINE(DEBUG, "PTP SUPPORTED");
HWRM_UNLOCK();
bnxt_hwrm_ptp_qcfg(bp);
}
}

if (flags & HWRM_FUNC_QCAPS_OUTPUT_FLAGS_EXT_STATS_SUPPORTED)
bp->flags |= BNXT_FLAG_EXT_STATS_SUPPORTED;

Expand Down Expand Up @@ -1263,6 +1254,18 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
if (flags_ext3 & HWRM_FUNC_QCAPS_OUTPUT_FLAGS_EXT3_RX_RATE_PROFILE_SEL_SUPPORTED)
bp->fw_cap |= BNXT_FW_CAP_RX_RATE_PROFILE;

/* This block should be kept at the end of this function because it
* sends another hwrm msg.
*/
if (flags & HWRM_FUNC_QCAPS_OUTPUT_FLAGS_PTP_SUPPORTED) {
if (BNXT_CHIP_P5(bp) || BNXT_PF(bp)) {
bp->flags |= BNXT_FLAG_PTP_SUPPORTED;
PMD_DRV_LOG_LINE(DEBUG, "PTP SUPPORTED");
HWRM_UNLOCK();
bnxt_hwrm_ptp_qcfg(bp);
}
}

unlock:
HWRM_UNLOCK();
Comment on lines +1257 to 1270
Copy link

@coderabbitai coderabbitai bot Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Double-unlock of bp->hwrm_lock spinlock causes undefined behavior.

When PTP is supported and the chip is P5 or PF, the code:

  1. Unlocks at line 1264
  2. Calls bnxt_hwrm_ptp_qcfg(bp) at line 1265
  3. Falls through to line 1270 which unlocks again

This double-unlock can cause segmentation faults, lock corruption, or data races.

Add a return statement after calling bnxt_hwrm_ptp_qcfg(bp) to prevent falling through to the duplicate unlock:

 	if (flags & HWRM_FUNC_QCAPS_OUTPUT_FLAGS_PTP_SUPPORTED) {
 		if (BNXT_CHIP_P5(bp) || BNXT_PF(bp)) {
 			bp->flags |= BNXT_FLAG_PTP_SUPPORTED;
 			PMD_DRV_LOG_LINE(DEBUG, "PTP SUPPORTED");
 			HWRM_UNLOCK();
 			bnxt_hwrm_ptp_qcfg(bp);
+			return rc;
 		}
 	}
 
 unlock:
 	HWRM_UNLOCK();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/* This block should be kept at the end of this function because it
* sends another hwrm msg.
*/
if (flags & HWRM_FUNC_QCAPS_OUTPUT_FLAGS_PTP_SUPPORTED) {
if (BNXT_CHIP_P5(bp) || BNXT_PF(bp)) {
bp->flags |= BNXT_FLAG_PTP_SUPPORTED;
PMD_DRV_LOG_LINE(DEBUG, "PTP SUPPORTED");
HWRM_UNLOCK();
bnxt_hwrm_ptp_qcfg(bp);
}
}
unlock:
HWRM_UNLOCK();
/* This block should be kept at the end of this function because it
* sends another hwrm msg.
*/
if (flags & HWRM_FUNC_QCAPS_OUTPUT_FLAGS_PTP_SUPPORTED) {
if (BNXT_CHIP_P5(bp) || BNXT_PF(bp)) {
bp->flags |= BNXT_FLAG_PTP_SUPPORTED;
PMD_DRV_LOG_LINE(DEBUG, "PTP SUPPORTED");
HWRM_UNLOCK();
bnxt_hwrm_ptp_qcfg(bp);
return rc;
}
}
unlock:
HWRM_UNLOCK();
🤖 Prompt for AI Agents 
In drivers/net/bnxt/bnxt_hwrm.c around lines 1257 to 1270, the spinlock is
unlocked twice when PTP is supported because HWRM_UNLOCK() is called before
bnxt_hwrm_ptp_qcfg(bp) and the function then falls through to the unlock at the
end; to fix this, after calling bnxt_hwrm_ptp_qcfg(bp) add a return to exit the
function immediately (keeping the early HWRM_UNLOCK() and the PTP code block
order intact) so the final unlock at the function end is not executed a second
time.


Expand Down