2006-12-04 20:10:00

by Anderson Briglia

[permalink] [raw]
Subject: [PATCH 4/4] Add MMC Password Protection (lock/unlock) support V8: mmc_sysfs.diff

Implement MMC password force erase, remove password, change password,
unlock card and assign password operations. It uses the sysfs mechanism
to send commands to the MMC subsystem.

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>

Index: linux-linus-2.6/drivers/mmc/mmc_sysfs.c
===================================================================
--- linux-linus-2.6.orig/drivers/mmc/mmc_sysfs.c 2006-12-04 15:26:05.000000000 -0400
+++ linux-linus-2.6/drivers/mmc/mmc_sysfs.c 2006-12-04 15:33:55.000000000 -0400
@@ -17,6 +17,7 @@
#include <linux/idr.h>
#include <linux/workqueue.h>
#include <linux/key.h>
+#include <linux/err.h>

#include <linux/mmc/card.h>
#include <linux/mmc/host.h>
@@ -65,6 +66,100 @@ static struct device_attribute mmc_dev_a

static struct device_attribute mmc_dev_attr_scr = MMC_ATTR_RO(scr);

+#ifdef CONFIG_MMC_PASSWORDS
+
+static ssize_t
+mmc_lockable_show(struct device *dev, struct device_attribute *att, char *buf)
+{
+ struct mmc_card *card = dev_to_mmc_card(dev);
+
+ if (!mmc_card_lockable(card))
+ return sprintf(buf, "unsupported\n");
+ else
+ return sprintf(buf, "%slocked\n", mmc_card_locked(card) ?
+ "" : "un");
+}
+
+/*
+ * implement MMC password functions: force erase, remove password, change
+ * password, unlock card and assign password.
+ */
+static ssize_t
+mmc_lockable_store(struct device *dev, struct device_attribute *att,
+ const char *data, size_t len)
+{
+ struct mmc_card *card = dev_to_mmc_card(dev);
+ int err = 0;
+
+ err = mmc_card_claim_host(card);
+ if (err != MMC_ERR_NONE)
+ return -EINVAL;
+
+ if (!mmc_card_lockable(card))
+ return -EINVAL;
+
+ if (mmc_card_locked(card) && !strncmp(data, "erase", 5)) {
+ /* forced erase only works while card is locked */
+ mmc_lock_unlock(card, NULL, MMC_LOCK_MODE_ERASE);
+ goto out;
+ } else if (!mmc_card_locked(card) && !strncmp(data, "remove", 6)) {
+ /* remove password only works while card is unlocked */
+ struct key *mmc_key = request_key(&mmc_key_type, "mmc:key", "remove");
+
+ if (!IS_ERR(mmc_key)) {
+ int err = mmc_lock_unlock(card, mmc_key, MMC_LOCK_MODE_CLR_PWD);
+ if (!err)
+ goto out;
+ } else
+ dev_dbg(&card->dev, "request_key returned error %ld\n", PTR_ERR(mmc_key));
+ } else if (!mmc_card_locked(card) && ((!strncmp(data, "assign", 6)) ||
+ (!strncmp(data, "change", 6)))) {
+
+ /* assign or change */
+ struct key *mmc_key;
+
+ if(!strncmp(data, "assign", 6))
+ mmc_key = request_key(&mmc_key_type, "mmc:key", "assign");
+ else
+ mmc_key = request_key(&mmc_key_type, "mmc:key", "change");
+
+ if (!IS_ERR(mmc_key)) {
+ int err = mmc_lock_unlock(card, mmc_key, MMC_LOCK_MODE_SET_PWD);
+ if (!err)
+ goto out;
+ } else
+ dev_dbg(&card->dev, "request_key returned error %ld\n", PTR_ERR(mmc_key));
+ } else if (mmc_card_locked(card) && !strncmp(data, "unlock", 6)) {
+ /* unlock */
+ struct key *mmc_key = request_key(&mmc_key_type, "mmc:key", "unlock");
+ if (!IS_ERR(mmc_key)) {
+ int err = mmc_lock_unlock(card, mmc_key, MMC_LOCK_MODE_UNLOCK);
+ if (err) {
+ dev_dbg(&card->dev, "Wrong password\n");
+ }
+ else {
+ mmc_card_release_host(card);
+ device_release_driver(dev);
+ device_attach(dev);
+ return len;
+ }
+ } else
+ dev_dbg(&card->dev, "request_key returned error %ld\n", PTR_ERR(mmc_key));
+ }
+
+ mmc_card_release_host(card);
+ return -EINVAL;
+out:
+ mmc_card_release_host(card);
+ return len;
+}
+
+static struct device_attribute mmc_dev_attr_lockable =
+ __ATTR(lockable, S_IWUSR | S_IRUGO,
+ mmc_lockable_show, mmc_lockable_store);
+
+#endif
+

static void mmc_release_card(struct device *dev)
{
@@ -234,6 +329,11 @@ int mmc_register_card(struct mmc_card *c
if (ret)
device_del(&card->dev);
}
+#ifdef CONFIG_MMC_PASSWORDS
+ ret = device_create_file(&card->dev, &mmc_dev_attr_lockable);
+ if (ret)
+ device_del(&card->dev);
+#endif
}
return ret;
}
@@ -248,6 +348,9 @@ void mmc_remove_card(struct mmc_card *ca
if (mmc_card_sd(card))
device_remove_file(&card->dev, &mmc_dev_attr_scr);

+#ifdef CONFIG_MMC_PASSWORDS
+ device_remove_file(&card->dev, &mmc_dev_attr_lockable);
+#endif
device_del(&card->dev);
}


Attachments:
mmc_sysfs.diff (4.26 kB)

2006-12-15 19:37:30

by Russell King

[permalink] [raw]
Subject: Re: [PATCH 4/4] Add MMC Password Protection (lock/unlock) support V8: mmc_sysfs.diff

On Mon, Dec 04, 2006 at 04:13:39PM -0400, Anderson Briglia wrote:
> Implement MMC password force erase, remove password, change password,
> unlock card and assign password operations. It uses the sysfs mechanism
> to send commands to the MMC subsystem.

Sorry, this is still unsuitable for mainline.

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

Please use the standard format, do not obfuscate these. The kernel
community utterly abhors this.

> +/*
> + * implement MMC password functions: force erase, remove password, change
> + * password, unlock card and assign password.
> + */
> +static ssize_t
> +mmc_lockable_store(struct device *dev, struct device_attribute *att,
> + const char *data, size_t len)
> +{
> + struct mmc_card *card = dev_to_mmc_card(dev);
> + int err = 0;

Where is the check that the host can do byte-wise data transfers?

> +
> + err = mmc_card_claim_host(card);
> + if (err != MMC_ERR_NONE)
> + return -EINVAL;
> +
> + if (!mmc_card_lockable(card))
> + return -EINVAL;

So writing to this file with a card which is not lockable results in
deadlocking the host. Suggest you do as other subsystems do and have
one exit path, and use gotos, setting the appropriate return code in a
variable. If everything goes via that it forces you to think about
where you want to jump to in each case.

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

2006-12-18 12:37:42

by Anderson Briglia

[permalink] [raw]
Subject: Re: [PATCH 4/4] Add MMC Password Protection (lock/unlock) support V8: mmc_sysfs.diff

ext Russell King wrote:
> On Mon, Dec 04, 2006 at 04:13:39PM -0400, Anderson Briglia wrote:
>> Implement MMC password force erase, remove password, change password,
>> unlock card and assign password operations. It uses the sysfs mechanism
>> to send commands to the MMC subsystem.
>
> Sorry, this is still unsuitable for mainline.

Ok. I will fix the code and send another version of this patch on the V9 series e-mail thread.

>
>> 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>
>
> Please use the standard format, do not obfuscate these. The kernel
> community utterly abhors this.

Ok.
>
>> +/*
>> + * implement MMC password functions: force erase, remove password, change
>> + * password, unlock card and assign password.
>> + */
>> +static ssize_t
>> +mmc_lockable_store(struct device *dev, struct device_attribute *att,
>> + const char *data, size_t len)
>> +{
>> + struct mmc_card *card = dev_to_mmc_card(dev);
>> + int err = 0;
>
> Where is the check that the host can do byte-wise data transfers?

It's checked on the macro "mmc_card_lockable".

>
>> +
>> + err = mmc_card_claim_host(card);
>> + if (err != MMC_ERR_NONE)
>> + return -EINVAL;
>> +
>> + if (!mmc_card_lockable(card))
>> + return -EINVAL;
>
> So writing to this file with a card which is not lockable results in
> deadlocking the host. Suggest you do as other subsystems do and have
> one exit path, and use gotos, setting the appropriate return code in a
> variable. If everything goes via that it forces you to think about
> where you want to jump to in each case.
>

Thanks, as said before, I'll update the code and send it again.

Best Regards,

Anderson Briglia

2006-12-23 14:14:03

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH 4/4] Add MMC Password Protection (lock/unlock) support V8: mmc_sysfs.diff

Anderson Briglia wrote:
> Ok. I will fix the code and send another version of this patch on the V9 series e-mail thread.
>

Have you found the time to fix this?

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-12-27 20:37:23

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH 4/4] Add MMC Password Protection (lock/unlock) support V8: mmc_sysfs.diff

[email protected] wrote:
> Did you see this patch at V9 series? This bug is fixed.
> I also fixed this code according the latest Russel's comments and will send again at V9, just this patch.
>
>

The V9 you sent me on the 15th was before Russell pointed out the
dangling lock, and doesn't contain a fix for it.

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-12-28 20:03:24

by Anderson Briglia

[permalink] [raw]
Subject: Re: [PATCH 4/4] Add MMC Password Protection (lock/unlock) support V8: mmc_sysfs.diff

ext Pierre Ossman wrote:
> [email protected] wrote:
>> Did you see this patch at V9 series? This bug is fixed.
>> I also fixed this code according the latest Russel's comments and will send again at V9, just this patch.
>>
>>
>
> The V9 you sent me on the 15th was before Russell pointed out the
> dangling lock, and doesn't contain a fix for it.

Yes, I'm already fixed the latest Russel's comment. I'm preparing it to send again to you and LKML. Do you have other
comments? If the others patches are ok, I intend to send just this mmc_sysfs.diff patch.

Regards,

Anderson Briglia

2006-12-29 10:34:13

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH 4/4] Add MMC Password Protection (lock/unlock) support V8: mmc_sysfs.diff

Anderson Briglia wrote:
> Yes, I'm already fixed the latest Russel's comment. I'm preparing it to send again to you and LKML. Do you have other
> comments? If the others patches are ok, I intend to send just this mmc_sysfs.diff patch.
>

Everything else looks fine, so send away.

Rgds

--
-- Pierre Ossman

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