Re: [patch 3/5] Add MMC password protection (lock/unlock) support V3

[Russell King <rmk+lkml@arm.linux.org.uk>]

On Mon, Jan 09, 2006 at 06:16:02PM -0400, Anderson Briglia wrote:
> +	dev = bus_find_device(&mmc_bus_type, NULL, NULL, mmc_match_lockable);
> +	if (!dev)
> +		goto error;
> +	card = dev_to_mmc_card(dev);
> +	
> +	if (operation == KEY_OP_INSTANTIATE) { /* KEY_OP_INSTANTIATE */
> +               if (mmc_card_locked(card)) {
> +                       ret = mmc_lock_unlock(card, key, MMC_LOCK_MODE_UNLOCK);
> +                       mmc_remove_card(card);
> +                       mmc_register_card(card);
> +               }
> +	       else
> +		       ret = mmc_lock_unlock(card, key, MMC_LOCK_MODE_SET_PWD);

I really don't like this - if the MMC card is not locked, we set a
password on it.  If it's locked, we unlock it.

That's a potential race condition if you're trying to unlock a card
and the card is changed beneath you while you slept waiting for
memory - you end up setting that password on the new card.

It's far better to have separate "unlock this card" and "set a
password on this card" commands rather than trying to combine the
two operations.

Also, removing and re-registering a card is an offence.  These
things are ref-counted, and mmc_remove_card() will drop the last
reference - so the memory associated with it will be freed.  Then
you re-register it.  Whoops.

If you merely want to try to attach a driver, use device_attach()
instead.

Also, what if you have multiple MMC cards?  I have a board here
with two MMC slots.  I'd rather not have it try to set the same
password on both devices.


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