2006-11-17 13:18:20

by Anderson Briglia

[permalink] [raw]
Subject: [patch 1/6] [RFC] Add MMC Password Protection (lock/unlock) support V6

When a card is locked, only commands from the "basic" and "lock card" classes
are accepted. To be able to use the other commands, the card must be unlocked
first.

This patch prevents the device drivers from trying to run privileged class
commands on locked MMC cards, which will fail anyway.

Signed-off-by: Carlos Eduardo Aguiar <carlos.aguiar <at> indt.org.br>
Signed-off-by: Anderson Lizardo <anderson.lizardo <at> indt.org.br>
Signed-off-by: Anderson Briglia <anderson.briglia <at> indt.org.br>
Signed-off-by: David Brownell <david-b <at> pacbell.net>

Index: linux-omap-2.6.git/drivers/mmc/mmc_sysfs.c
===================================================================
--- linux-omap-2.6.git.orig/drivers/mmc/mmc_sysfs.c 2006-11-14 08:46:54.000000000 -0400
+++ linux-omap-2.6.git/drivers/mmc/mmc_sysfs.c 2006-11-14 09:06:39.000000000 -0400
@@ -17,6 +17,7 @@

#include <linux/mmc/card.h>
#include <linux/mmc/host.h>
+#include <linux/mmc/protocol.h>

#include "mmc.h"

@@ -73,10 +74,19 @@ static void mmc_release_card(struct devi
* This currently matches any MMC driver to any MMC card - drivers
* themselves make the decision whether to drive this card in their
* probe method. However, we force "bad" cards to fail.
+ *
+ * We also fail for all locked cards; drivers expect to be able to do block
+ * I/O still on probe(), which is not possible while the card is locked.
+ * Device probing must be triggered sometime later to make the card available
+ * to the block driver.
*/
static int mmc_bus_match(struct device *dev, struct device_driver *drv)
{
struct mmc_card *card = dev_to_mmc_card(dev);
+ if (mmc_card_lockable(card) && mmc_card_locked(card)) {
+ dev_dbg(&card->dev, "card is locked; binding is deferred\n");
+ return 0;
+ }
return !mmc_card_bad(card);
}

Index: linux-omap-2.6.git/include/linux/mmc/card.h
===================================================================
--- linux-omap-2.6.git.orig/include/linux/mmc/card.h 2006-11-14 08:46:54.000000000 -0400
+++ linux-omap-2.6.git/include/linux/mmc/card.h 2006-11-14 09:06:39.000000000 -0400
@@ -62,6 +62,7 @@ struct mmc_card {
#define MMC_STATE_BAD (1<<2) /* unrecognised device */
#define MMC_STATE_SDCARD (1<<3) /* is an SD card */
#define MMC_STATE_READONLY (1<<4) /* card is read-only */
+#define MMC_STATE_LOCKED (1<<5) /* card is currently locked */
u32 raw_cid[4]; /* raw card CID */
u32 raw_csd[4]; /* raw card CSD */
u32 raw_scr[2]; /* raw card SCR */
@@ -75,12 +76,19 @@ struct mmc_card {
#define mmc_card_bad(c) ((c)->state & MMC_STATE_BAD)
#define mmc_card_sd(c) ((c)->state & MMC_STATE_SDCARD)
#define mmc_card_readonly(c) ((c)->state & MMC_STATE_READONLY)
+#define mmc_card_locked(c) ((c)->state & MMC_STATE_LOCKED)
+#define mmc_card_clear_locked(c) ((c)->state &= ~MMC_STATE_LOCKED)
+
+#define mmc_card_lockable(c) (((c)->csd.cmdclass & CCC_LOCK_CARD) && \
+ ((c)->host->caps & MMC_CAP_LOCK_UNLOCK) && \
+ ((c)->host->caps & MMC_CAP_BYTEBLOCK))

#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT)
#define mmc_card_set_dead(c) ((c)->state |= MMC_STATE_DEAD)
#define mmc_card_set_bad(c) ((c)->state |= MMC_STATE_BAD)
#define mmc_card_set_sd(c) ((c)->state |= MMC_STATE_SDCARD)
#define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
+#define mmc_card_set_locked(c) ((c)->state |= MMC_STATE_LOCKED)

#define mmc_card_name(c) ((c)->cid.prod_name)
#define mmc_card_id(c) ((c)->dev.bus_id)
Index: linux-omap-2.6.git/include/linux/mmc/host.h
===================================================================
--- linux-omap-2.6.git.orig/include/linux/mmc/host.h 2006-11-14 08:46:54.000000000 -0400
+++ linux-omap-2.6.git/include/linux/mmc/host.h 2006-11-14 09:06:39.000000000 -0400
@@ -87,6 +87,7 @@ struct mmc_host {
#define MMC_CAP_4_BIT_DATA (1 << 0) /* Can the host do 4 bit transfers */
#define MMC_CAP_MULTIWRITE (1 << 1) /* Can accurately report bytes sent to card on error */
#define MMC_CAP_BYTEBLOCK (1 << 2) /* Can do non-log2 block sizes */
+#define MMC_CAP_LOCK_UNLOCK (1 << 3) /* Host password support capability */

/* host specific block data */
unsigned int max_seg_size; /* see blk_queue_max_segment_size */
Index: linux-omap-2.6.git/drivers/mmc/mmc.c
===================================================================
--- linux-omap-2.6.git.orig/drivers/mmc/mmc.c 2006-11-14 08:46:54.000000000 -0400
+++ linux-omap-2.6.git/drivers/mmc/mmc.c 2006-11-14 09:06:39.000000000 -0400
@@ -922,6 +922,11 @@ static void mmc_discover_cards(struct mm
if (err != MMC_ERR_NONE)
mmc_card_set_dead(card);
}
+
+ if (cmd.resp[0] & R1_CARD_IS_LOCKED)
+ mmc_card_set_locked(card);
+ else
+ mmc_card_clear_locked(card);
}
}


Attachments:
mmc_ignore_locked.diff (4.62 kB)

2006-11-17 16:04:28

by Russell King

[permalink] [raw]
Subject: Re: [patch 1/6] [RFC] Add MMC Password Protection (lock/unlock) support V6

On Fri, Nov 17, 2006 at 09:01:11AM -0400, Anderson Briglia wrote:
> Index: linux-omap-2.6.git/drivers/mmc/mmc_sysfs.c
> ===================================================================
> --- linux-omap-2.6.git.orig/drivers/mmc/mmc_sysfs.c 2006-11-14
> 08:46:54.000000000 -0400
> +++ linux-omap-2.6.git/drivers/mmc/mmc_sysfs.c 2006-11-14
> 09:06:39.000000000 -0400
> @@ -17,6 +17,7 @@
>
> #include <linux/mmc/card.h>
> #include <linux/mmc/host.h>
> +#include <linux/mmc/protocol.h>

Why does this file need mmc/protocol.h? I don't see anything added
which would require this include.

> @@ -73,10 +74,19 @@ static void mmc_release_card(struct devi
> * This currently matches any MMC driver to any MMC card - drivers
> * themselves make the decision whether to drive this card in their
> * probe method. However, we force "bad" cards to fail.
> + *
> + * We also fail for all locked cards; drivers expect to be able to do block
> + * I/O still on probe(), which is not possible while the card is locked.
> + * Device probing must be triggered sometime later to make the card
> available

Your mailer wrapped the patch... Also it would be nice to have this manually
wrapped so it's viewable sanely on a standard 80 column display.

> @@ -75,12 +76,19 @@ struct mmc_card {
> #define mmc_card_bad(c) ((c)->state & MMC_STATE_BAD)
> #define mmc_card_sd(c) ((c)->state & MMC_STATE_SDCARD)
> #define mmc_card_readonly(c) ((c)->state & MMC_STATE_READONLY)
> +#define mmc_card_locked(c) ((c)->state & MMC_STATE_LOCKED)
> +#define mmc_card_clear_locked(c) ((c)->state &= ~MMC_STATE_LOCKED)

Since we separate out the "mmc_card_set_xxx" macros, please do the same
with the "mmc_card_clear_xxx" for consistency sake.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2006-11-17 16:09:59

by Russell King

[permalink] [raw]
Subject: Re: [patch 1/6] [RFC] Add MMC Password Protection (lock/unlock) support V6

On Fri, Nov 17, 2006 at 09:01:11AM -0400, Anderson Briglia wrote:
> #define MMC_CAP_BYTEBLOCK (1 << 2) /* Can do non-log2 block
> sizes */
> +#define MMC_CAP_LOCK_UNLOCK (1 << 3) /* Host password support
> capability */

What's the point of this capability. If the host can do BYTEBLOCK transfers
it can send the password commands.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2006-11-17 16:41:22

by Pierre Ossman

[permalink] [raw]
Subject: Re: [patch 1/6] [RFC] Add MMC Password Protection (lock/unlock) support V6

Russell King wrote:
> On Fri, Nov 17, 2006 at 09:01:11AM -0400, Anderson Briglia wrote:
>
>> #define MMC_CAP_BYTEBLOCK (1 << 2) /* Can do non-log2 block
>> sizes */
>> +#define MMC_CAP_LOCK_UNLOCK (1 << 3) /* Host password support
>> capability */
>>
>
> What's the point of this capability. If the host can do BYTEBLOCK transfers
> it can send the password commands.
>
>

This has been pointed out in the past. Anderson, please make sure you
address all the previous concerns before sending out a new patch set.
This just creates extra work for us.

Rgds

--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org

2006-11-17 23:01:43

by Anderson Briglia

[permalink] [raw]
Subject: Re: [patch 1/6] [RFC] Add MMC Password Protection (lock/unlock) support V6

Hi Russel and Pierre,

Thanks for all comments, I'm fixing the series and will send another version asap.

ext Russell King wrote:
> On Fri, Nov 17, 2006 at 09:01:11AM -0400, Anderson Briglia wrote:
>> Index: linux-omap-2.6.git/drivers/mmc/mmc_sysfs.c
>> ===================================================================
>> --- linux-omap-2.6.git.orig/drivers/mmc/mmc_sysfs.c 2006-11-14
>> 08:46:54.000000000 -0400
>> +++ linux-omap-2.6.git/drivers/mmc/mmc_sysfs.c 2006-11-14
>> 09:06:39.000000000 -0400
>> @@ -17,6 +17,7 @@
>>
>> #include <linux/mmc/card.h>
>> #include <linux/mmc/host.h>
>> +#include <linux/mmc/protocol.h>
>
> Why does this file need mmc/protocol.h? I don't see anything added
> which would require this include.

We need this here because mmc_card_lockable function uses "CCC_LOCK_CARD" macro defined on
linux/mmc/protocol.h

>
>> @@ -73,10 +74,19 @@ static void mmc_release_card(struct devi
>> * This currently matches any MMC driver to any MMC card - drivers
>> * themselves make the decision whether to drive this card in their
>> * probe method. However, we force "bad" cards to fail.
>> + *
>> + * We also fail for all locked cards; drivers expect to be able to do block
>> + * I/O still on probe(), which is not possible while the card is locked.
>> + * Device probing must be triggered sometime later to make the card
>> available
>
> Your mailer wrapped the patch... Also it would be nice to have this manually
> wrapped so it's viewable sanely on a standard 80 column display.
>
>> @@ -75,12 +76,19 @@ struct mmc_card {
>> #define mmc_card_bad(c) ((c)->state & MMC_STATE_BAD)
>> #define mmc_card_sd(c) ((c)->state & MMC_STATE_SDCARD)
>> #define mmc_card_readonly(c) ((c)->state & MMC_STATE_READONLY)
>> +#define mmc_card_locked(c) ((c)->state & MMC_STATE_LOCKED)
>> +#define mmc_card_clear_locked(c) ((c)->state &= ~MMC_STATE_LOCKED)
>
> Since we separate out the "mmc_card_set_xxx" macros, please do the same
> with the "mmc_card_clear_xxx" for consistency sake.
>