2006-11-22 15:58:55

by Anderson Briglia

[permalink] [raw]
Subject: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

Implement card lock/unlock operation, using the MMC_LOCK_UNLOCK command.

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-omap-2.6.git/drivers/mmc/mmc.c
===================================================================
--- linux-omap-2.6.git.orig/drivers/mmc/mmc.c 2006-11-22 09:19:27.000000000 -0400
+++ linux-omap-2.6.git/drivers/mmc/mmc.c 2006-11-22 09:56:13.000000000 -0400
@@ -4,6 +4,8 @@
* Copyright (C) 2003-2004 Russell King, All Rights Reserved.
* SD support Copyright (C) 2004 Ian Molton, All Rights Reserved.
* SD support Copyright (C) 2005 Pierre Ossman, All Rights Reserved.
+ * MMC password protection (C) 2006 Instituto Nokia de Tecnologia (INdT),
+ * All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -19,6 +21,7 @@
#include <linux/err.h>
#include <asm/scatterlist.h>
#include <linux/scatterlist.h>
+#include <linux/key.h>

#include <linux/mmc/card.h>
#include <linux/mmc/host.h>
@@ -1159,6 +1162,122 @@ static void mmc_setup(struct mmc_host *h
mmc_read_scrs(host);
}

+/**
+ * mmc_lock_unlock - send LOCK_UNLOCK command to a specific card.
+ * @card: card to which the LOCK_UNLOCK command should be sent
+ * @key: key containing the MMC password
+ * @mode: LOCK_UNLOCK mode
+ *
+ */
+int mmc_lock_unlock(struct mmc_card *card, struct key *key, int mode)
+{
+ struct mmc_request mrq;
+ struct mmc_command cmd;
+ struct mmc_data data;
+ struct scatterlist sg;
+ struct mmc_key_payload *mpayload;
+ unsigned long erase_timeout;
+ int err, data_size;
+ u8 *data_buf;
+
+ mpayload = NULL;
+ data_size = 1;
+ if (mode != MMC_LOCK_MODE_ERASE) {
+ mpayload = rcu_dereference(key->payload.data);
+ data_size = 2 + mpayload->datalen;
+ }
+
+ data_buf = kmalloc(data_size, GFP_KERNEL);
+ if (!data_buf)
+ return -ENOMEM;
+ memset(data_buf, 0, data_size);
+
+ data_buf[0] = mode;
+ if (mode != MMC_LOCK_MODE_ERASE) {
+ data_buf[1] = mpayload->datalen;
+ memcpy(data_buf + 2, mpayload->data, mpayload->datalen);
+ }
+
+ err = mmc_card_claim_host(card);
+ if (err != MMC_ERR_NONE)
+ goto out;
+
+ memset(&cmd, 0, sizeof(struct mmc_command));
+
+ cmd.opcode = MMC_SET_BLOCKLEN;
+ cmd.arg = data_size;
+ cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+ err = mmc_wait_for_cmd(card->host, &cmd, CMD_RETRIES);
+ if (err != MMC_ERR_NONE)
+ goto error;
+
+ memset(&cmd, 0, sizeof(struct mmc_command));
+
+ cmd.opcode = MMC_LOCK_UNLOCK;
+ cmd.arg = 0;
+ cmd.flags = MMC_RSP_R1B | MMC_CMD_ADTC;
+
+ memset(&data, 0, sizeof(struct mmc_data));
+
+ mmc_set_data_timeout(&data, card, 1);
+
+ data.blksz = data_size;
+ data.blocks = 1;
+ data.flags = MMC_DATA_WRITE;
+ data.sg = &sg;
+ data.sg_len = 1;
+
+ memset(&mrq, 0, sizeof(struct mmc_request));
+
+ mrq.cmd = &cmd;
+ mrq.data = &data;
+
+ sg_init_one(&sg, data_buf, data_size);
+ err = mmc_wait_for_req(card->host, &mrq);
+ if (err != MMC_ERR_NONE)
+ goto error;
+
+ memset(&cmd, 0, sizeof(struct mmc_command));
+
+ cmd.opcode = MMC_SEND_STATUS;
+ cmd.arg = card->rca << 16;
+ cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+
+ /* set timeout for forced erase operation to 3 min. (see MMC spec) */
+ erase_timeout = jiffies + 180 * HZ;
+ do {
+ /* we cannot use "retries" here because the
+ * R1_LOCK_UNLOCK_FAILED bit is cleared by subsequent reads to
+ * the status register, hiding the error condition */
+ err = mmc_wait_for_cmd(card->host, &cmd, 0);
+ if (err != MMC_ERR_NONE)
+ break;
+ /* the other modes don't need timeout checking */
+ if (mode != MMC_LOCK_MODE_ERASE)
+ continue;
+ if (time_after(jiffies, erase_timeout)) {
+ dev_dbg(&card->dev, "forced erase timed out\n");
+ err = MMC_ERR_TIMEOUT;
+ break;
+ }
+ } while (!(cmd.resp[0] & R1_READY_FOR_DATA));
+ if (cmd.resp[0] & R1_LOCK_UNLOCK_FAILED) {
+ dev_dbg(&card->dev, "LOCK_UNLOCK operation failed\n");
+ err = MMC_ERR_FAILED;
+ }
+
+ if (cmd.resp[0] & R1_CARD_IS_LOCKED)
+ mmc_card_set_locked(card);
+ else
+ mmc_card_clear_locked(card);
+
+error:
+ mmc_card_release_host(card);
+out:
+ kfree(data_buf);
+
+ return err;
+}

/**
* mmc_detect_change - process change of state on a MMC socket
Index: linux-omap-2.6.git/drivers/mmc/mmc.h
===================================================================
--- linux-omap-2.6.git.orig/drivers/mmc/mmc.h 2006-11-22 09:19:27.000000000 -0400
+++ linux-omap-2.6.git/drivers/mmc/mmc.h 2006-11-22 09:19:27.000000000 -0400
@@ -27,6 +27,10 @@ struct mmc_key_payload {
char data[0]; /* actual data */
};

+struct key;
+
+extern int mmc_lock_unlock(struct mmc_card *card, struct key *key, int mode);
+
int mmc_schedule_work(struct work_struct *work);
int mmc_schedule_delayed_work(struct work_struct *work, unsigned long delay);
void mmc_flush_scheduled_work(void);
Index: linux-omap-2.6.git/include/linux/mmc/protocol.h
===================================================================
--- linux-omap-2.6.git.orig/include/linux/mmc/protocol.h 2006-11-22 09:19:13.000000000 -0400
+++ linux-omap-2.6.git/include/linux/mmc/protocol.h 2006-11-22 09:19:27.000000000 -0400
@@ -244,5 +244,13 @@ struct _mmc_csd {
#define SD_BUS_WIDTH_1 0
#define SD_BUS_WIDTH_4 2

+/*
+ * MMC_LOCK_UNLOCK modes
+ */
+#define MMC_LOCK_MODE_ERASE (1<<3)
+#define MMC_LOCK_MODE_UNLOCK (0<<2)
+#define MMC_LOCK_MODE_CLR_PWD (1<<1)
+#define MMC_LOCK_MODE_SET_PWD (1<<0)
+
#endif /* MMC_MMC_PROTOCOL_H */


2006-11-25 08:46:57

by Pierre Ossman

[permalink] [raw]
Subject: Re: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

Anderson Briglia wrote:
> @@ -244,5 +244,13 @@ struct _mmc_csd {
> #define SD_BUS_WIDTH_1 0
> #define SD_BUS_WIDTH_4 2
>
> +/*
> + * MMC_LOCK_UNLOCK modes
> + */
> +#define MMC_LOCK_MODE_ERASE (1<<3)
> +#define MMC_LOCK_MODE_UNLOCK (0<<2)
> +#define MMC_LOCK_MODE_CLR_PWD (1<<1)
> +#define MMC_LOCK_MODE_SET_PWD (1<<0)
> +
> #endif /* MMC_MMC_PROTOCOL_H */
>

This definition makes them look like bits, which is not how they are used.

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-25 08:53:23

by Pierre Ossman

[permalink] [raw]
Subject: Re: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

Seems I missed this...

Anderson Briglia wrote:
> +
> + err = mmc_card_claim_host(card);
> + if (err != MMC_ERR_NONE)
> + goto out;
> +

As I said before, no locking in the command abstractions. The host
should be claimed somewhere further up the chain (in
mmc_lockable_store() right now).

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-27 13:50:41

by Anderson Briglia

[permalink] [raw]
Subject: Re: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

ext Pierre Ossman wrote:
> Anderson Briglia wrote:
>> @@ -244,5 +244,13 @@ struct _mmc_csd {
>> #define SD_BUS_WIDTH_1 0
>> #define SD_BUS_WIDTH_4 2
>>
>> +/*
>> + * MMC_LOCK_UNLOCK modes
>> + */
>> +#define MMC_LOCK_MODE_ERASE (1<<3)
>> +#define MMC_LOCK_MODE_UNLOCK (0<<2)
>> +#define MMC_LOCK_MODE_CLR_PWD (1<<1)
>> +#define MMC_LOCK_MODE_SET_PWD (1<<0)
>> +
>> #endif /* MMC_MMC_PROTOCOL_H */
>>
>
> This definition makes them look like bits, which is not how they are used.

How can I improve this? Defining using integers directly?

>
> Rgds
>

2006-12-01 21:20:08

by Pierre Ossman

[permalink] [raw]
Subject: Re: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

Anderson Briglia wrote:
> ext Pierre Ossman wrote:
>>
>> This definition makes them look like bits, which is not how they are
>> used.
>
> How can I improve this? Defining using integers directly?
>

Precisely.

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-04 18:44:19

by Anderson Briglia

[permalink] [raw]
Subject: Re: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

ext Pierre Ossman wrote:
> Anderson Briglia wrote:
>> @@ -244,5 +244,13 @@ struct _mmc_csd {
>> #define SD_BUS_WIDTH_1 0
>> #define SD_BUS_WIDTH_4 2
>>
>> +/*
>> + * MMC_LOCK_UNLOCK modes
>> + */
>> +#define MMC_LOCK_MODE_ERASE (1<<3)
>> +#define MMC_LOCK_MODE_UNLOCK (0<<2)
>> +#define MMC_LOCK_MODE_CLR_PWD (1<<1)
>> +#define MMC_LOCK_MODE_SET_PWD (1<<0)
>> +
>> #endif /* MMC_MMC_PROTOCOL_H */
>>
>
> This definition makes them look like bits, which is not how they are used.

Actually they represent the bits regarding the modes and it is used when we
have to send the LOCK/UNLOCK mode on the command data block, according to the MMC Spec.
If you take a look at mmc_lock_unlock function, we use those modes to set the right bit
when composing the command data block.
This definition makes the code more legible and simple.

Regards,

Anderson Briglia

2006-12-04 20:23:22

by Pierre Ossman

[permalink] [raw]
Subject: Re: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

Anderson Briglia wrote:
>
> Actually they represent the bits regarding the modes and it is used
> when we
> have to send the LOCK/UNLOCK mode on the command data block, according
> to the MMC Spec.
> If you take a look at mmc_lock_unlock function, we use those modes to
> set the right bit
> when composing the command data block.
> This definition makes the code more legible and simple.

In that case you need to change the code to make sure it is clear that
it is bits and not values. Also, your definition for
MMC_LOCK_MODE_UNLOCK is wrong.

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-05 18:03:55

by Anderson Briglia

[permalink] [raw]
Subject: Re: [patch 3/5] [RFC] Add MMC Password Protection (lock/unlock) support V7: mmc_lock_unlock.diff

ext Pierre Ossman wrote:

> Anderson Briglia wrote:
>> Actually they represent the bits regarding the modes and it is used
>> when we
>> have to send the LOCK/UNLOCK mode on the command data block, according
>> to the MMC Spec.
>> If you take a look at mmc_lock_unlock function, we use those modes to
>> set the right bit
>> when composing the command data block.
>> This definition makes the code more legible and simple.
>
> In that case you need to change the code to make sure it is clear that
> it is bits and not values.

Yes. As I sent another version after read your comment, I'll fix the V8 code just for this patch, ok? I don't see that
we have to send another version.


> Also, your definition for
> MMC_LOCK_MODE_UNLOCK is wrong.

This definition:

#define MMC_LOCK_MODE_UNLOCK (0<<2)

also tries to make the code more clear and legible. It is indicating that this bit is on the position 3 into the bits
array, according to MMC Spec.
But, why zero? Well, the bit must be 1 for locking and 0 for unlocking. We are using the unlocking action, so we used zero.
Is there any issue using this piece of code?
If you prefer another approach we can adjust our patch.

Regards,

Anderson Briglia