linux-kernel - [PATCH 2/4] mmc: sunxi: Lock fix

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]

Date:	Tue, 16 Dec 2014 15:11:04 +0100
From:	David Lanzendörfer <david.lanzendoerfer@....ch>
To:	Ulf Hansson <ulf.hansson@...aro.org>,
	Tomeu Vizoso <tomeu.vizoso@...labora.com>,
	Arnd Bergmann <arnd@...db.de>, linux-mmc@...r.kernel.org,
	Chris Ball <chris@...ntf.net>, linux-kernel@...r.kernel.org,
	Peter Griffin <peter.griffin@...aro.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Chen-Yu Tsai <wens@...e.org>,
	David Lanzendörfer <david.lanzendoerfer@....ch>,
	李想 <lixiang@...winnertech.com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	linux-arm-kernel@...ts.infradead.org
Subject: [PATCH 2/4] mmc: sunxi: Lock fix

1)	Adding a comment in order to clarify the choice of the locks within
	sunxi_mmc_handle_manual_stop

2)	As 李想 has pointed out the wait_dma variable was not accessed within the spin lock
	block in sunxi_mmc_request and so (even if it should never happend) it would have
	theoretically been possible that some other function would access the variable
	at the same time as the function.
	This has been changed now and the function is using local variables outside the lock
	and copys the value over during the lock phase.

Signed-off-by: David Lanzendörfer <david.lanzendoerfer@....ch>
Reported-by: 李想 <lixiang@...winnertech.com>
---
 drivers/mmc/host/sunxi-mmc.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 1fe54a8..67e680c 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -572,6 +572,15 @@ static irqreturn_t sunxi_mmc_handle_manual_stop(int irq, void *dev_id)
 	}
 
 	dev_err(mmc_dev(host->mmc), "data error, sending stop command\n");
+
+	/*
+	 * We will never have more than one outstanding request,
+	 * and we do not complete the request until after
+	 * we've cleared host->manual_stop_mrq so we do not need to
+	 * spin lock this function.
+	 * Additionally we have wait states within this function
+	 * so having it in a lock is a very bad idea.
+	 */
 	sunxi_mmc_send_manual_stop(host, mrq);
 
 	spin_lock_irqsave(&host->lock, iflags);
@@ -768,6 +777,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	unsigned long iflags;
 	u32 imask = SDXC_INTERRUPT_ERROR_BIT;
 	u32 cmd_val = SDXC_START | (cmd->opcode & 0x3f);
+	bool wait_dma = host->wait_dma;
 	int ret;
 
 	/* Check for set_ios errors (should never happen) */
@@ -818,7 +828,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 			if (cmd->data->flags & MMC_DATA_WRITE)
 				cmd_val |= SDXC_WRITE;
 			else
-				host->wait_dma = true;
+				wait_dma = true;
 		} else {
 			imask |= SDXC_COMMAND_DONE;
 		}
@@ -852,6 +862,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	}
 
 	host->mrq = mrq;
+	host->wait_dma = wait_dma;
 	mmc_writel(host, REG_IMASK, host->sdio_imask | imask);
 	mmc_writel(host, REG_CARG, cmd->arg);
 	mmc_writel(host, REG_CMDR, cmd_val);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/