>> >> Userspace currently has no way of checking for error bits of detection >> mode X. These are error bits that are only detected by the card when >> executing the command. For e.g. a sanitize operation this may be >> minutes after the RSP was seen by the host. >> >> Currently userspace programs cannot see these error bits reliably. >> They could issue a multi ioctl cmd with a CMD13 immediately following >> it, but since errors of detection mode X are automatically cleared >> (they are all clear condition B). >> mmc_poll_for_busy of the first ioctl may have already hidden such an >> error flag. >> >> In case of the security operations: sanitize, secure erases and RPMB >> writes, this could lead to the operation not being performed >> successfully by the card with the user not knowing. >> If the user trusts that this operation is completed (e.g. their data >> is sanitized), this could be a security issue. >> An attacker could e.g. provoke a eMMC (VCC) flash fail, where a >> successful sanitize of a card is not possible. A card may move out of >> PROG state but issue a bit 19 R1 error. >> >> This patch therefore will also have the consequence of a mmc-utils >> patch, which enables the bit for the security-sensitive operations. >> >> Signed-off-by: Christian Loehle >> --- >> drivers/mmc/core/block.c | 17 ++++++----------- >> drivers/mmc/core/mmc_ops.c | 25 ++++++++++++++++++++++++- >> drivers/mmc/core/mmc_ops.h | 3 +++ >> 3 files changed, 33 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index >> e46330815484..44c1b2825032 100644 >> --- a/drivers/mmc/core/block.c >> +++ b/drivers/mmc/core/block.c >> @@ -470,7 +470,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 +551,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 +604,15 @@ 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); >> + /* Poll for write/R1B execution errors */ >> + if (idata->ic.write_flag || r1b_resp) > > Earlier we polled for requests that were targeted to rpmb, no matter if they were write or reads. Are you intentionally changing this? If so, can you explain why? > Will re-introduce. I cant really think of a reason right now to do this after rpmb reads, but thats a different story. >> + err = mmc_poll_for_busy_err_flags(card, busy_timeout_ms, false, >> + MMC_BUSY_IO, >> + &idata->ic.response[0]); > > I think it's better to extend the mmc_blk_busy_cb, rather than introducing an entirely new polling function. > > Then you can call __mmc_poll_for_busy() here instead. Not sure if I understood you right, but I will send a new version with __mmc_poll_for_busy call directly. It does feel a bit more awkward, at least to me, because both mmc_blk_busy_cb nor mmc_busy_data are currently only in mmc_ops.c Anyway, both versions "extend the mmc_blk_busy_cb", so I'm not sure if I understood you correctly, we will see. I may also just send both and you pick whichever you prefer. Regards, Christian