linux-kernel - [PATCH] mmc: core: Fix error propagation for some ioctl commands

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]

Message-Id: <20230816114712.25093-1-ulf.hansson@linaro.org>
Date:   Wed, 16 Aug 2023 13:47:12 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     linux-mmc@...r.kernel.org, Ulf Hansson <ulf.hansson@...aro.org>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        Avri Altman <avri.altman@....com>,
        Christian Lohle <cloehle@...erstone.com>,
        linux-kernel@...r.kernel.org,
        Christian Loehle <CLoehle@...erstone.com>
Subject: [PATCH] mmc: core: Fix error propagation for some ioctl commands

Userspace has currently has no way of checking the internal R1 response
error bits for some commands. This is a problem for some commands, like
RPMB for example. Typically, we may detect that the busy completion
successfully has ended, while in fact the card did not complete the
requested operation.

To fix the problem, let's always poll with CDM13 for these commands and
during the polling aggregate the R1 response bits. Before completing the
ioctl request, let's propagate the R1 response bits too.

Cc: Avri Altman <avri.altman@....com>
Co-developed-by: Christian Loehle <CLoehle@...erstone.com>
Signed-off-by: Christian Loehle <CLoehle@...erstone.com>
Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
---

Christian, I took the liberty of re-working your previous patch [1]. But rather
than keeping your authorship I added you as a co-developer. Please tell me if
you prefer differently.

Kind regards
Uffe

[1]
https://lore.kernel.org/all/26d178dcfc2f4b7d9010145d0c051394@hyperstone.com/ 

---
 drivers/mmc/core/block.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index b6f4be25b31b..62a8aacc996c 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -179,6 +179,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 			       struct mmc_queue *mq);
 static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
 static int mmc_spi_err_check(struct mmc_card *card);
+static int mmc_blk_busy_cb(void *cb_data, bool *busy);
 
 static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
 {
@@ -470,7 +471,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 	struct mmc_data data = {};
 	struct mmc_request mrq = {};
 	struct scatterlist sg;
-	bool r1b_resp, use_r1b_resp = false;
+	bool r1b_resp;
 	unsigned int busy_timeout_ms;
 	int err;
 	unsigned int target_part;
@@ -551,8 +552,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 	busy_timeout_ms = idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS;
 	r1b_resp = (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B;
 	if (r1b_resp)
-		use_r1b_resp = mmc_prepare_busy_cmd(card->host, &cmd,
-						    busy_timeout_ms);
+		mmc_prepare_busy_cmd(card->host, &cmd, busy_timeout_ms);
 
 	mmc_wait_for_req(card->host, &mrq);
 	memcpy(&idata->ic.response, cmd.resp, sizeof(cmd.resp));
@@ -605,19 +605,28 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 	if (idata->ic.postsleep_min_us)
 		usleep_range(idata->ic.postsleep_min_us, idata->ic.postsleep_max_us);
 
-	/* No need to poll when using HW busy detection. */
-	if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
-		return 0;
-
 	if (mmc_host_is_spi(card->host)) {
 		if (idata->ic.write_flag || r1b_resp || cmd.flags & MMC_RSP_SPI_BUSY)
 			return mmc_spi_err_check(card);
 		return err;
 	}
-	/* Ensure RPMB/R1B command has completed by polling with CMD13. */
-	if (idata->rpmb || r1b_resp)
-		err = mmc_poll_for_busy(card, busy_timeout_ms, false,
-					MMC_BUSY_IO);
+
+	/*
+	 * Ensure RPMB, writes and R1B responses are completed by polling with
+	 * CMD13. Note that, usually we don't need to poll when using HW busy
+	 * detection, but here it's needed since some commands may indicate the
+	 * error through the R1 status bits.
+	 */
+	if (idata->rpmb || idata->ic.write_flag || r1b_resp) {
+		struct mmc_blk_busy_data cb_data;
+
+		cb_data.card = card;
+		cb_data.status = 0;
+		err = __mmc_poll_for_busy(card->host, 0, busy_timeout_ms,
+					  &mmc_blk_busy_cb, &cb_data);
+
+		idata->ic.response[0] = cb_data.status;
+	}
 
 	return err;
 }
-- 
2.34.1