| 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
| ||
|
Date: Mon, 25 Mar 2013 13:17:57 +0000
From: "Luca Porzio (lporzio)" <lporzio@...ron.com>
To: "merez@...eaurora.org" <merez@...eaurora.org>
CC: Yaniv Gardi <ygardi@...eaurora.org>,
"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"vgoyal@...hat.com" <vgoyal@...hat.com>,
"tj@...nel.org" <tj@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>
Subject: RE: [PATCH v1] mmc: card: Adding support for sanitize in eMMC 4.5
Maya,
It sounds good for me.
What about the second comment about issuing the HPI on timeout?
If you think we can accept to issue an HPI on timeout, I think the first comment about the timeout duration can also be neglected.
Thanks,
Luca
> -----Original Message-----
> From: linux-mmc-owner@...r.kernel.org [mailto:linux-mmc-
> owner@...r.kernel.org] On Behalf Of merez@...eaurora.org
> Sent: Thursday, March 21, 2013 8:50 PM
> To: Luca Porzio (lporzio)
> Cc: Yaniv Gardi; linux-mmc@...r.kernel.org; vgoyal@...hat.com;
> tj@...nel.org; linux-kernel@...r.kernel.org; linux-arm-msm@...r.kernel.org
> Subject: RE: [PATCH v1] mmc: card: Adding support for sanitize in eMMC 4.5
>
> Hi Luca,
>
> Having a timeout that takes into consideration the card size would be
> artificial as we cannot have the ability to create a function for its
> calculation that will fit all the card vendors.
>
> I suggest keeping it as a constant value for simplicity, as 4 minutes
> cover all the card sizes.
>
> Thanks,
> Maya
> > Hi Yaniv,
> >
> >> -----Original Message-----
> >> From: linux-mmc-owner@...r.kernel.org [mailto:linux-mmc-
> >> owner@...r.kernel.org] On Behalf Of Yaniv Gardi
> >> Sent: Sunday, February 24, 2013 12:39 PM
> >> To: linux-mmc@...r.kernel.org; vgoyal@...hat.com; tj@...nel.org; linux-
> >> kernel@...r.kernel.org
> >> Cc: linux-arm-msm@...r.kernel.org; Yaniv Gardi
> >> Subject: [PATCH v1] mmc: card: Adding support for sanitize in eMMC 4.5
> >>
> >> The sanitize support is added as a user-app ioctl call, and
> >> was removed from the block-device request, since its purpose is
> >> to be invoked not via File-System but by a user.
> >> This feature deletes the unmap memory region of the eMMC card,
> >> by writing to a specific register in the EXT_CSD.
> >> unmap region is the memory region that was previously deleted
> >> (by erase, trim or discard operation).
> >> In order to avoid timeout when sanitizing large-scale cards,
> >> the timeout for sanitize operation is 240 seconds.
> >>
> >> Signed-off-by: Yaniv Gardi <ygardi@...eaurora.org>
> >>
> >> ---
> >> drivers/mmc/card/block.c | 68
> >> +++++++++++++++++++++++++++++++-----------
> >> ---
> >> drivers/mmc/card/queue.c | 2 +-
> >> include/linux/mmc/host.h | 1 +
> >> 3 files changed, 49 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> >> index 21056b9..21bb8b4 100644
> >> --- a/drivers/mmc/card/block.c
> >> +++ b/drivers/mmc/card/block.c
> >> @@ -58,6 +58,8 @@ MODULE_ALIAS("mmc:block");
> >> #define INAND_CMD38_ARG_SECTRIM1 0x81
> >> #define INAND_CMD38_ARG_SECTRIM2 0x88
> >> #define MMC_BLK_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute
> >> timeout
> >> */
> >> +#define MMC_SANITIZE_REQ_TIMEOUT 240000
> >
> > Though I would agree that 4 minutes is a reasonable sanitize time,
> > the sanitize command may also depend on card size.
> > As such I am not sure whether it can be regarded as a constant or
> > needs to be proportional to card size.
> >
> >> +#define MMC_EXTRACT_INDEX_FROM_ARG(x) ((x & 0x00FF0000) >> 16)
> >>
> >> static DEFINE_MUTEX(block_mutex);
> >>
> >> @@ -394,6 +396,35 @@ static int ioctl_rpmb_card_status_poll(struct
> >> mmc_card
> >> *card, u32 *status,
> >> return err;
> >> }
> >>
> >> +static int ioctl_do_sanitize(struct mmc_card *card)
> >> +{
> >> + int err;
> >> +
> >> + if (!(mmc_can_sanitize(card) &&
> >> + (card->host->caps2 & MMC_CAP2_SANITIZE))) {
> >> + pr_warn("%s: %s - SANITIZE is not supported\n",
> >> + mmc_hostname(card->host), __func__);
> >> + err = -EOPNOTSUPP;
> >> + goto out;
> >> + }
> >> +
> >> + pr_debug("%s: %s - SANITIZE IN PROGRESS...\n",
> >> + mmc_hostname(card->host), __func__);
> >> +
> >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >> + EXT_CSD_SANITIZE_START, 1,
> >> + MMC_SANITIZE_REQ_TIMEOUT);
> >> +
> >> + if (err)
> >> + pr_err("%s: %s - EXT_CSD_SANITIZE_START failed. err=%d\n",
> >> + mmc_hostname(card->host), __func__, err);
> >> +
> >
> > In case of Sanitize timeout, the eMMC might go in an unclear state.
> > May I suggest to:
> > - issue an HPI before leaving thus bring the eMMC back into safe status
> > - report the 'sanitize not complete error' and let the user decide on
> > Whether he wants to re-issue (i.e. continue) the sanitize or just let
> > it go.
> >
> > Thanks,
> > Luca
> >
> >> + pr_debug("%s: %s - SANITIZE COMPLETED\n", mmc_hostname(card->host),
> >> + __func__);
> >> +out:
> >> + return err;
> >> +}
> >> +
> >> static int mmc_blk_ioctl_cmd(struct block_device *bdev,
> >> struct mmc_ioc_cmd __user *ic_ptr)
> >> {
> >> @@ -496,6 +527,16 @@ static int mmc_blk_ioctl_cmd(struct block_device
> >> *bdev,
> >> goto cmd_rel_host;
> >> }
> >>
> >> + if (MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) {
> >> + err = ioctl_do_sanitize(card);
> >> +
> >> + if (err)
> >> + pr_err("%s: ioctl_do_sanitize() failed. err = %d",
> >> + __func__, err);
> >> +
> >> + goto cmd_rel_host;
> >> + }
> >> +
> >> mmc_wait_for_req(card->host, &mrq);
> >>
> >> if (cmd.error) {
> >> @@ -925,10 +966,10 @@ static int mmc_blk_issue_secdiscard_rq(struct
> >> mmc_queue *mq,
> >> {
> >> struct mmc_blk_data *md = mq->data;
> >> struct mmc_card *card = md->queue.card;
> >> - unsigned int from, nr, arg, trim_arg, erase_arg;
> >> + unsigned int from, nr, arg;
> >> int err = 0, type = MMC_BLK_SECDISCARD;
> >>
> >> - if (!(mmc_can_secure_erase_trim(card) || mmc_can_sanitize(card))) {
> >> + if (!(mmc_can_secure_erase_trim(card))) {
> >> err = -EOPNOTSUPP;
> >> goto out;
> >> }
> >> @@ -936,23 +977,11 @@ static int mmc_blk_issue_secdiscard_rq(struct
> >> mmc_queue *mq,
> >> from = blk_rq_pos(req);
> >> nr = blk_rq_sectors(req);
> >>
> >> - /* The sanitize operation is supported at v4.5 only */
> >> - if (mmc_can_sanitize(card)) {
> >> - erase_arg = MMC_ERASE_ARG;
> >> - trim_arg = MMC_TRIM_ARG;
> >> - } else {
> >> - erase_arg = MMC_SECURE_ERASE_ARG;
> >> - trim_arg = MMC_SECURE_TRIM1_ARG;
> >> - }
> >> + if (mmc_can_trim(card) && !mmc_erase_group_aligned(card, from, nr))
> >> + arg = MMC_SECURE_TRIM1_ARG;
> >> + else
> >> + arg = MMC_SECURE_ERASE_ARG;
> >>
> >> - if (mmc_erase_group_aligned(card, from, nr))
> >> - arg = erase_arg;
> >> - else if (mmc_can_trim(card))
> >> - arg = trim_arg;
> >> - else {
> >> - err = -EINVAL;
> >> - goto out;
> >> - }
> >> retry:
> >> if (card->quirks & MMC_QUIRK_INAND_CMD38) {
> >> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >> @@ -988,9 +1017,6 @@ retry:
> >> goto out;
> >> }
> >>
> >> - if (mmc_can_sanitize(card))
> >> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >> - EXT_CSD_SANITIZE_START, 1, 0);
> >> out_retry:
> >> if (err && !mmc_blk_reset(md, card->host, type))
> >> goto retry;
> >> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> >> index fadf52e..483f0e8 100644
> >> --- a/drivers/mmc/card/queue.c
> >> +++ b/drivers/mmc/card/queue.c
> >> @@ -148,7 +148,7 @@ static void mmc_queue_setup_discard(struct
> >> request_queue *q,
> >> /* granularity must not be greater than max. discard */
> >> if (card->pref_erase > max_discard)
> >> q->limits.discard_granularity = 0;
> >> - if (mmc_can_secure_erase_trim(card) || mmc_can_sanitize(card))
> >> + if (mmc_can_secure_erase_trim(card))
> >> queue_flag_set_unlocked(QUEUE_FLAG_SECDISCARD, q);
> >> }
> >>
> >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> >> index 61a10c1..045e9f7 100644
> >> --- a/include/linux/mmc/host.h
> >> +++ b/include/linux/mmc/host.h
> >> @@ -258,6 +258,7 @@ struct mmc_host {
> >> #define MMC_CAP2_HC_ERASE_SZ (1 << 9) /* High-capacity erase size */
> >> #define MMC_CAP2_CD_ACTIVE_HIGH (1 << 10) /* Card-detect signal
> active
> >> high */
> >> #define MMC_CAP2_RO_ACTIVE_HIGH (1 << 11) /* Write-protect signal
> >> active
> >> high */
> >> +#define MMC_CAP2_SANITIZE (1 << 12) /* Support Sanitize */
> >>
> >> mmc_pm_flag_t pm_caps; /* supported pm features */
> >>
> >> --
> >> 1.7.6
> >> --
> >> Sent by a consultant of the Qualcomm Innovation Center, Inc.
> >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> >> Forum
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> >> the body of a message to majordomo@...r.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>
> --
> Maya Erez
> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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/
Powered by blists - more mailing lists