linux-kernel - [PATCH v1 1/2] dmaengine: idxd: Fix race condition between WQ enable and reset paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250522063329.51156-2-xueshuai@linux.alibaba.com>
Date: Thu, 22 May 2025 14:33:28 +0800
From: Shuai Xue <xueshuai@...ux.alibaba.com>
To: vinicius.gomes@...el.com,
	dave.jiang@...el.com,
	fenghuay@...dia.com,
	vkoul@...nel.org
Cc: xueshuai@...ux.alibaba.com,
	dmaengine@...r.kernel.org,
	colin.i.king@...il.com,
	linux-kernel@...r.kernel.org
Subject: [PATCH v1 1/2] dmaengine: idxd: Fix race condition between WQ enable and reset paths

A device reset command disables all WQs in hardware. If issued while a WQ
is being enabled, it can cause a mismatch between the software and hardware
states.

When a hardware error occurs, the IDXD driver calls idxd_device_reset() to
send a reset command and clear the state (wq->state) of all WQs. It then
uses wq_enable_map (a bitmask tracking enabled WQs) to re-enable them and
ensure consistency between the software and hardware states.

However, a race condition exists between the WQ enable path and the
reset/recovery path. For example:

A: WQ enable path                   B: Reset and recovery path
------------------                 ------------------------
a1. issue IDXD_CMD_ENABLE_WQ
                                   b1. issue IDXD_CMD_RESET_DEVICE
                                   b2. clear wq->state
                                   b3. check wq_enable_map bit, not set
a2. set wq->state = IDXD_WQ_ENABLED
a3. set wq_enable_map

In this case, b1 issues a reset command that disables all WQs in hardware.
Since b3 checks wq_enable_map before a2, it doesn't re-enable the WQ,
leading to an inconsistency between wq->state (software) and the actual
hardware state (IDXD_WQ_DISABLED).

To fix this, the WQ state should be set to IDXD_WQ_ENABLED before issuing
the IDXD_CMD_ENABLE_WQ command. This ensures the software state aligns with
the expected hardware behavior during resets:

A: WQ enable path                   B: Reset and recovery path
------------------                 ------------------------
                                   b1. issue IDXD_CMD_RESET_DEVICE
                                   b2. clear wq->state
a1. set wq->state = IDXD_WQ_ENABLED
a2. set wq_enable_map
                                   b3. check wq_enable_map bit, true
                                   b4. check wq state, return
a3. issue IDXD_CMD_ENABLE_WQ

By updating the state early, the reset path can safely re-enable the WQ
based on wq_enable_map.

A corner case arises when both paths attempt to enable the same WQ:

A: WQ enable path                   B: Reset and recovery path
------------------                 ------------------------
                                   b1. issue IDXD_CMD_RESET_DEVICE
                                   b2. clear wq->state
a1. set wq->state = IDXD_WQ_ENABLED
a2. set wq_enable_map
                                   b3. check wq_enable_map bit
                                   b4. check wq state, not reset
                                   b5. issue IDXD_CMD_ENABLE_WQ
a3. issue IDXD_CMD_ENABLE_WQ

Here, the command status (CMDSTS) returns IDXD_CMDSTS_ERR_WQ_ENABLED,
but the driver treats it as success (IDXD_CMDSTS_SUCCESS), ensuring the WQ
remains enabled.

Signed-off-by: Shuai Xue <xueshuai@...ux.alibaba.com>
---
 drivers/dma/idxd/device.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 5cf419fe6b46..b424c3c8f359 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -188,16 +188,32 @@ int idxd_wq_enable(struct idxd_wq *wq)
 		return 0;
 	}
 
+	/*
+	 * A device reset command disables all WQs in hardware. If issued
+	 * while a WQ is being enabled, it can cause a mismatch between the
+	 * software and hardware states.
+	 *
+	 * When a hardware error occurs, the IDXD driver calls
+	 * idxd_device_reset() to send a reset command and clear the state
+	 * (wq->state) of all WQs. It then uses wq_enable_map to re-enable
+	 * them and ensure consistency between the software and hardware states.
+	 *
+	 * To avoid inconsistency between software and hardware states,
+	 * issue the IDXD_CMD_ENABLE_WQ command as the final step.
+	 */
+	wq->state = IDXD_WQ_ENABLED;
+	set_bit(wq->id, idxd->wq_enable_map);
+
 	idxd_cmd_exec(idxd, IDXD_CMD_ENABLE_WQ, wq->id, &status);
 
 	if (status != IDXD_CMDSTS_SUCCESS &&
 	    status != IDXD_CMDSTS_ERR_WQ_ENABLED) {
+		clear_bit(wq->id, idxd->wq_enable_map);
+		wq->state = IDXD_WQ_DISABLED;
 		dev_dbg(dev, "WQ enable failed: %#x\n", status);
 		return -ENXIO;
 	}
 
-	wq->state = IDXD_WQ_ENABLED;
-	set_bit(wq->id, idxd->wq_enable_map);
 	dev_dbg(dev, "WQ %d enabled\n", wq->id);
 	return 0;
 }
-- 
2.43.5