linux-kernel - [RFC PATCH] mmc: dw_mmc: avoid race condition of cpu and IDMAC

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 16 Aug 2016 11:23:47 +0800
From:	Shawn Lin <shawn.lin@...k-chips.com>
To:	Jaehoon Chung <jh80.chung@...sung.com>
Cc:	Ulf Hansson <ulf.hansson@...aro.org>, linux-mmc@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Doug Anderson <dianders@...omium.org>,
	Brian Norris <briannorris@...omium.org>,
	Heiko Stuebner <heiko@...ech.de>,
	linux-rockchip@...ts.infradead.org,
	Shawn Lin <shawn.lin@...k-chips.com>
Subject: [RFC PATCH] mmc: dw_mmc: avoid race condition of cpu and IDMAC

We could see an obvious race condition by test that
the former write operation by IDMAC aiming to clear
OWN bit reach right after the later configuration of
the same desc, which makes the IDMAC be in SUSPEND
state as the OWN bit was cleared by the asynchronous
write operation of IDMAC. The bug can be very easy
reproduced on RK3288 or similar when lowering the
bandwidth of bus and aggravating the Qos to make the
large numbers of IP fight for the priority. One possible
replaceable solution may be alloc dual buff for the
desc to avoid it but could still race each other
theoretically.

Signed-off-by: Shawn Lin <shawn.lin@...k-chips.com>

---

 drivers/mmc/host/dw_mmc.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 32380d5..7b01fab 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -490,6 +490,23 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
 				length -= desc_len;
 
 				/*
+				 * OWN bit should be clear by IDMAC after
+				 * finishing transfer. Let's wait for the
+				 * asynchronous operation of IDMAC and cpu
+				 * to make sure that we do not rely on the
+				 * order of Qos of bus and architecture.
+				 * Otherwise we could see a race condition
+				 * here that the former write operation of
+				 * IDMAC(to clear the OWN bit) reach right
+				 * after the later new configuration of desc
+				 * which makes value of desc been covered
+				 * leading to DMA_SUSPEND state as IDMAC fecth
+				 * the wrong desc then.
+				 */
+				while ((readl(&desc->des0) & IDMAC_DES0_OWN))
+					;
+
+				/*
 				 * Set the OWN bit and disable interrupts
 				 * for this descriptor
 				 */
@@ -535,6 +552,23 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
 				length -= desc_len;
 
 				/*
+				 * OWN bit should be clear by IDMAC after
+				 * finishing transfer. Let's wait for the
+				 * asynchronous operation of IDMAC and cpu
+				 * to make sure that we do not rely on the
+				 * order of Qos of bus and architecture.
+				 * Otherwise we could see a race condition
+				 * here that the former write operation of
+				 * IDMAC(to clear the OWN bit) reach right
+				 * after the later new configuration of desc
+				 * which makes value of desc been covered
+				 * leading to DMA_SUSPEND state as IDMAC fecth
+				 * the wrong desc then.
+				 */
+				while ((readl(&desc->des0) & IDMAC_DES0_OWN))
+					;
+
+				/*
 				 * Set the OWN bit and disable interrupts
 				 * for this descriptor
 				 */
-- 
2.3.7