linux-kernel - [PATCH 2/6] mmc: dw_mmc: fix race with request and removal

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: <4E04892E.406@imgtec.com>
Date:	Fri, 24 Jun 2011 13:55:10 +0100
From:	James Hogan <james.hogan@...tec.com>
To:	James Hogan <james.hogan@...tec.com>
CC:	Chris Ball <cjb@...top.org>, Will Newton <will.newton@...tec.com>,
	Jaehoon Chung <jh80.chung@...sung.com>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Matt Fleming <matt@...sole-pimps.org>,
	linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
	james@...anarts.com
Subject: [PATCH 2/6] mmc: dw_mmc: fix race with request and removal

When a request is made, the card presence is checked and the request is
queued. These two parts must be atomic with respect to card removal, or
a card removal could be handled in between, and the new request wouldn't
get cancelled until another card was inserted. Therefore move the
spinlock protection from dw_mci_queue_request() up into dw_mci_request()
to cover the presence check.

Note that the test_bit() used for the presence check isn't atomic
itself, so should have been protected by a spinlock anyway.

Signed-off-by: James Hogan <james.hogan@...tec.com>
---
 drivers/mmc/host/dw_mmc.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 08c0592..c4bddf6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -625,13 +625,13 @@ static void dw_mci_start_request(struct dw_mci *host,
 		host->stop_cmdr = dw_mci_prepare_command(slot->mmc, mrq->stop);
 }
 
+/* must be called with host->lock held */
 static void dw_mci_queue_request(struct dw_mci *host, struct dw_mci_slot *slot,
 				 struct mmc_request *mrq)
 {
 	dev_vdbg(&slot->mmc->class_dev, "queue request: state=%d\n",
 		 host->state);
 
-	spin_lock_bh(&host->lock);
 	slot->mrq = mrq;
 
 	if (host->state == STATE_IDLE) {
@@ -640,8 +640,6 @@ static void dw_mci_queue_request(struct dw_mci *host, struct dw_mci_slot *slot,
 	} else {
 		list_add_tail(&slot->queue_node, &host->queue);
 	}
-
-	spin_unlock_bh(&host->lock);
 }
 
 static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
@@ -651,14 +649,23 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	WARN_ON(slot->mrq);
 
+	/*
+	 * The check for card presence and queueing of the request must be
+	 * atomic, otherwise the card could be removed in between and the
+	 * request wouldn't fail until another card was inserted.
+	 */
+	spin_lock_bh(&host->lock);
+
 	if (!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
+		spin_unlock_bh(&host->lock);
 		mrq->cmd->error = -ENOMEDIUM;
 		mmc_request_done(mmc, mrq);
 		return;
 	}
 
-	/* We don't support multiple blocks of weird lengths. */
 	dw_mci_queue_request(host, slot, mrq);
+
+	spin_unlock_bh(&host->lock);
 }
 
 static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
-- 
1.7.2.3

--
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/