[PATCH net v2] pds_core: fix cmd_regs access racing BAR unmap on reset

"Nikhil P. Rao" <[email protected]>
Newsgroups org.kernel.vger.netdev
Message-ID <[email protected]>
pdsc_reset_prepare() and pdsc_reset_done()'s pdsc_map_bars() error path
clear/iounmap cmd_regs without devcmd_lock, and some devcmd callers
access cmd_regs under the lock without checking it. An FLR concurrent
with a devlink flash can unmap cmd_regs under an in-flight devcmd,
causing a NULL deref or a write to unmapped MMIO.

Take devcmd_lock across the BAR unmap/remap, and check cmd_regs at those
callers. Only the PF maps cmd_regs and runs devcmd, so skip the unmap on
a VF, as pdsc_remove() and pdsc_reset_done() already do.

pdsc_unmap_bars() also clears info_regs, intr_status, intr_ctrl and
db_pages. The interrupt and start/stop readers of those are quiesced
before the unmap by pdsc_fw_down(), which frees the interrupts and tears
down the queues. The debugfs readers are not, since those files outlive
a reset; that is pre-existing and out of scope here.

Fixes: e96094c1d11c ("pds_core: Clear BARs on reset")
Reported-by: sashiko-bot <[email protected]>
Closes: https://sashiko.dev/#/patchset/20260708212222.296202-1-nikhil.rao%40amd.com?part=3
Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Nikhil P. Rao <[email protected]>
---
v2: address sashiko findings on v1
  - guard cmd_regs under devcmd_lock in pdsc_identify() and
    pdsc_core_init(), which stage into cmd_regs->data before
    pdsc_devcmd_locked()'s check
  - skip the unmap for VFs instead of running it unlocked; it is a
    no-op there
  - the download loop is left as-is: an interrupted flash is not
    committed, as the device validates on install and reports
    PDS_RC_BAD_FW
  - say why the other BAR0-backed pointers (intr_status, intr_ctrl,
    db_pages) are not affected, and what remains out of scope
  - rebased on net
v1: https://lore.kernel.org/all/[email protected]/

 drivers/net/ethernet/amd/pds_core/core.c | 8 ++++++++
 drivers/net/ethernet/amd/pds_core/dev.c  | 5 +++++
 drivers/net/ethernet/amd/pds_core/fw.c   | 6 ++++++
 drivers/net/ethernet/amd/pds_core/main.c | 8 +++++++-
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/pds_core/core.c b/drivers/net/ethernet/amd/pds_core/core.c
index 04ec2569c61c..c3c27a7c63d0 100644
--- a/drivers/net/ethernet/amd/pds_core/core.c
+++ b/drivers/net/ethernet/amd/pds_core/core.c
@@ -364,6 +364,14 @@ static int pdsc_core_init(struct pdsc *pdsc)
 
 	mutex_lock(&pdsc->devcmd_lock);
 
+	if (!pdsc->cmd_regs) {
+		mutex_unlock(&pdsc->devcmd_lock);
+		err = -ENXIO;
+		dev_err(pdsc->dev, "Device init command failed: %pe\n",
+			ERR_PTR(err));
+		goto err_out_uninit;
+	}
+
 	sz = min_t(size_t, sizeof(cidi), sizeof(pdsc->cmd_regs->data));
 	memcpy_toio(&pdsc->cmd_regs->data, &cidi, sz);
 
diff --git a/drivers/net/ethernet/amd/pds_core/dev.c b/drivers/net/ethernet/amd/pds_core/dev.c
index bded6b33289c..ec96f67dc67b 100644
--- a/drivers/net/ethernet/amd/pds_core/dev.c
+++ b/drivers/net/ethernet/amd/pds_core/dev.c
@@ -294,6 +294,11 @@ static int pdsc_identify(struct pdsc *pdsc)
 	 */
 	mutex_lock(&pdsc->devcmd_lock);
 
+	if (!pdsc->cmd_regs) {
+		mutex_unlock(&pdsc->devcmd_lock);
+		return -ENXIO;
+	}
+
 	sz = min_t(size_t, sizeof(drv), sizeof(pdsc->cmd_regs->data));
 	memcpy_toio(&pdsc->cmd_regs->data, &drv, sz);
 
diff --git a/drivers/net/ethernet/amd/pds_core/fw.c b/drivers/net/ethernet/amd/pds_core/fw.c
index fa626719e68d..cd7616ed9ef3 100644
--- a/drivers/net/ethernet/amd/pds_core/fw.c
+++ b/drivers/net/ethernet/amd/pds_core/fw.c
@@ -134,6 +134,12 @@ int pdsc_firmware_update(struct pdsc *pdsc, const struct firmware *fw,
 
 		copy_sz = min_t(unsigned int, buf_sz, fw->size - offset);
 		mutex_lock(&pdsc->devcmd_lock);
+		if (!pdsc->cmd_regs) {
+			mutex_unlock(&pdsc->devcmd_lock);
+			err = -ENXIO;
+			NL_SET_ERR_MSG_MOD(extack, "Device reset during flash");
+			goto err_out;
+		}
 		memcpy_toio(&pdsc->cmd_regs->data, fw->data + offset, copy_sz);
 		err = pdsc_devcmd_fw_download_locked(pdsc, data_addr,
 						     offset, copy_sz);
diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
index 9a2c64198d03..1b960139de4e 100644
--- a/drivers/net/ethernet/amd/pds_core/main.c
+++ b/drivers/net/ethernet/amd/pds_core/main.c
@@ -507,7 +507,11 @@ static void pdsc_reset_prepare(struct pci_dev *pdev)
 		pdsc_auxbus_dev_del(pdsc, pdsc, &pdsc->padev);
 	}
 
-	pdsc_unmap_bars(pdsc);
+	if (!pdev->is_virtfn) {
+		mutex_lock(&pdsc->devcmd_lock);
+		pdsc_unmap_bars(pdsc);
+		mutex_unlock(&pdsc->devcmd_lock);
+	}
 	pci_release_regions(pdev);
 	if (pci_is_enabled(pdev))
 		pci_disable_device(pdev);
@@ -536,7 +540,9 @@ static void pdsc_reset_done(struct pci_dev *pdev)
 			return;
 		}
 
+		mutex_lock(&pdsc->devcmd_lock);
 		err = pdsc_map_bars(pdsc);
+		mutex_unlock(&pdsc->devcmd_lock);
 		if (err)
 			return;
 	}
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.