Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d831b408 authored by Eric Biggers's avatar Eric Biggers Committed by Vaibhav Agrawal
Browse files

ANDROID: ufs, block: fix crypto power management and move into block layer 



The call to pm_runtime_get_sync() in ufshcd_program_key() can deadlock
because it waits for the UFS controller to be resumed, but it can itself
be reached while resuming the UFS controller via:

- ufshcd_runtime_resume()
  - ufshcd_resume()
    - ufshcd_reset_and_restore()
      - ufshcd_host_reset_and_restore()
        - ufshcd_hba_enable()
          - ufshcd_hba_execute_hce()
            - ufshcd_hba_start()
              - ufshcd_crypto_enable()
                - keyslot_manager_reprogram_all_keys()
                  - ufshcd_crypto_keyslot_program()
                    - ufshcd_program_key()

But pm_runtime_get_sync() *is* needed when evicting a key.  Also, on
pre-4.20 kernels it's needed when programming a keyslot for a bio since
the block layer used to resume the device in a different place.

Thus, it's hard for drivers to know what to do in .keyslot_program() and
.keyslot_evict().  In old kernels it may even be impossible unless we
were to pass more information down from the keyslot_manager.

There's also another possible deadlock: keyslot programming and eviction
take ksm->lock for write and then resume the device, which may result in
ksm->lock being taken again via the above call stack.  To fix this, we
should resume the device before taking ksm->lock.

Fix these problems by moving to a better design where the block layer
(namely, the keyslot manager) handles runtime power management instead
of drivers.  This is analogous to the block layer's existing runtime
power management support (blk-pm), which handles resuming devices when
bios are submitted to them so that drivers don't need to handle it.

Test: Tested on coral with:
        echo 5 > /sys/bus/platform/devices/1d84000.ufshc/rpm_lvl
        sleep 30
        touch /data && sync  # hangs before this fix
  Also verified via kvm-xfstests that blk-crypto-fallback continues
  to work both with and without CONFIG_PM=y.

Bug: 137270441
Bug: 149368295
Change-Id: I6bc9fb81854afe7edf490d71796ee68a61f7cbc8
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Git-Commit: 8d97219e
Git-Repo: https://android.googlesource.com/kernel/common


[neersoni@codeaurora.org]: fixed compilation issues.
Signed-off-by: default avatarNeeraj Soni <neersoni@codeaurora.org>
[vagrawa@codeaurora.org]: fix merge conflicts.
Signed-off-by: default avatarVaibhav Agrawal <vagrawa@codeaurora.org>
parent 87ceb869

block/blk-crypto-fallback.c

+1 −1
Original line number Diff line number Diff line
@@ -600,7 +600,7 @@ int __init blk_crypto_fallback_init(void) 
		crypto_mode_supported[i] = 0xFFFFFFFF;

	crypto_mode_supported[BLK_ENCRYPTION_MODE_INVALID] = 0;



	blk_crypto_ksm = keyslot_manager_create(blk_crypto_num_keyslots,

	blk_crypto_ksm = keyslot_manager_create(NULL, blk_crypto_num_keyslots,

				&blk_crypto_ksm_ll_ops,

				BLK_CRYPTO_FEATURE_STANDARD_KEYS,

				crypto_mode_supported, NULL);

block/keyslot-manager.c

+78 −12
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ 
#include <linux/keyslot-manager.h>

#include <linux/atomic.h>

#include <linux/mutex.h>

#include <linux/pm_runtime.h>

#include <linux/wait.h>

#include <linux/blkdev.h>

#include <linux/overflow.h>
@@ -48,6 +49,11 @@ struct keyslot_manager { 
	unsigned int max_dun_bytes_supported;

	void *ll_priv_data;



#ifdef CONFIG_PM

	/* Device for runtime power management (NULL if none) */

	struct device *dev;

#endif



	/* Protects programming and evicting keys from the device */

	struct rw_semaphore lock;
@@ -74,8 +80,60 @@ static inline bool keyslot_manager_is_passthrough(struct keyslot_manager *ksm) 
	return ksm->num_slots == 0;

}



#ifdef CONFIG_PM

static inline void keyslot_manager_set_dev(struct keyslot_manager *ksm,

					   struct device *dev)

{

	ksm->dev = dev;

}



/* If there's an underlying device and it's suspended, resume it. */

static inline void keyslot_manager_pm_get(struct keyslot_manager *ksm)

{

	if (ksm->dev)

		pm_runtime_get_sync(ksm->dev);

}



static inline void keyslot_manager_pm_put(struct keyslot_manager *ksm)

{

	if (ksm->dev)

		pm_runtime_put_sync(ksm->dev);

}

#else /* CONFIG_PM */

static inline void keyslot_manager_set_dev(struct keyslot_manager *ksm,

					   struct device *dev)

{

}



static inline void keyslot_manager_pm_get(struct keyslot_manager *ksm)

{

}



static inline void keyslot_manager_pm_put(struct keyslot_manager *ksm)

{

}

#endif /* !CONFIG_PM */



static inline void keyslot_manager_hw_enter(struct keyslot_manager *ksm)

{

	/*

	 * Calling into the driver requires ksm->lock held and the device

	 * resumed.  But we must resume the device first, since that can acquire

	 * and release ksm->lock via keyslot_manager_reprogram_all_keys().

	 */

	keyslot_manager_pm_get(ksm);

	down_write(&ksm->lock);

}



static inline void keyslot_manager_hw_exit(struct keyslot_manager *ksm)

{

	up_write(&ksm->lock);

	keyslot_manager_pm_put(ksm);

}



/**

 * keyslot_manager_create() - Create a keyslot manager

 * @dev: Device for runtime power management (NULL if none)

 * @num_slots: The number of key slots to manage.

 * @ksm_ll_ops: The struct keyslot_mgmt_ll_ops for the device that this keyslot

 *		manager will use to perform operations like programming and
@@ -97,7 +155,9 @@ static inline bool keyslot_manager_is_passthrough(struct keyslot_manager *ksm) 
 * Context: May sleep

 * Return: Pointer to constructed keyslot manager or NULL on error.

 */

struct keyslot_manager *keyslot_manager_create(unsigned int num_slots,

struct keyslot_manager *keyslot_manager_create(

	struct device *dev,

	unsigned int num_slots,

	const struct keyslot_mgmt_ll_ops *ksm_ll_ops,

	unsigned int features,

	const unsigned int crypto_mode_supported[BLK_ENCRYPTION_MODE_MAX],
@@ -126,6 +186,7 @@ struct keyslot_manager *keyslot_manager_create(unsigned int num_slots, 
	       sizeof(ksm->crypto_mode_supported));

	ksm->max_dun_bytes_supported = BLK_CRYPTO_MAX_IV_SIZE;

	ksm->ll_priv_data = ll_priv_data;

	keyslot_manager_set_dev(ksm, dev);



	init_rwsem(&ksm->lock);
@@ -242,10 +303,10 @@ int keyslot_manager_get_slot_for_key(struct keyslot_manager *ksm, 
		return slot;



	for (;;) {

		down_write(&ksm->lock);

		keyslot_manager_hw_enter(ksm);

		slot = find_and_grab_keyslot(ksm, key);

		if (slot != -ENOKEY) {

			up_write(&ksm->lock);

			keyslot_manager_hw_exit(ksm);

			return slot;

		}
@@ -256,7 +317,7 @@ int keyslot_manager_get_slot_for_key(struct keyslot_manager *ksm, 
		if (!list_empty(&ksm->idle_slots))

			break;



		up_write(&ksm->lock);

		keyslot_manager_hw_exit(ksm);

		wait_event(ksm->idle_slots_wait_queue,

			   !list_empty(&ksm->idle_slots));

	}
@@ -268,7 +329,7 @@ int keyslot_manager_get_slot_for_key(struct keyslot_manager *ksm, 
	err = ksm->ksm_ll_ops.keyslot_program(ksm, key, slot);

	if (err) {

		wake_up(&ksm->idle_slots_wait_queue);

		up_write(&ksm->lock);

		keyslot_manager_hw_exit(ksm);

		return err;

	}
@@ -282,7 +343,7 @@ int keyslot_manager_get_slot_for_key(struct keyslot_manager *ksm, 


	remove_slot_from_lru_list(ksm, slot);



	up_write(&ksm->lock);

	keyslot_manager_hw_exit(ksm);

	return slot;

}
@@ -397,15 +458,16 @@ int keyslot_manager_evict_key(struct keyslot_manager *ksm, 


	if (keyslot_manager_is_passthrough(ksm)) {

		if (ksm->ksm_ll_ops.keyslot_evict) {

			down_write(&ksm->lock);

			keyslot_manager_hw_enter(ksm);

			err = ksm->ksm_ll_ops.keyslot_evict(ksm, key, -1);

			up_write(&ksm->lock);

			keyslot_manager_hw_exit(ksm);

			return err;

		}

		return 0;

	}



	down_write(&ksm->lock);

	keyslot_manager_hw_enter(ksm);



	slot = find_keyslot(ksm, key);

	if (slot < 0) {

		err = slot;
@@ -425,7 +487,7 @@ int keyslot_manager_evict_key(struct keyslot_manager *ksm, 
	memzero_explicit(&slotp->key, sizeof(slotp->key));

	err = 0;

out_unlock:

	up_write(&ksm->lock);

	keyslot_manager_hw_exit(ksm);

	return err;

}
@@ -445,6 +507,7 @@ void keyslot_manager_reprogram_all_keys(struct keyslot_manager *ksm) 
	if (WARN_ON(keyslot_manager_is_passthrough(ksm)))

		return;



	/* This is for device initialization, so don't resume the device */

	down_write(&ksm->lock);

	for (slot = 0; slot < ksm->num_slots; slot++) {

		const struct keyslot *slotp = &ksm->slots[slot];
@@ -484,6 +547,7 @@ EXPORT_SYMBOL_GPL(keyslot_manager_destroy); 


/**

 * keyslot_manager_create_passthrough() - Create a passthrough keyslot manager

 * @dev: Device for runtime power management (NULL if none)

 * @ksm_ll_ops: The struct keyslot_mgmt_ll_ops

 * @features: Bitmask of BLK_CRYPTO_FEATURE_* flags

 * @crypto_mode_supported: Bitmasks for supported encryption modes
@@ -501,6 +565,7 @@ EXPORT_SYMBOL_GPL(keyslot_manager_destroy); 
 * Return: Pointer to constructed keyslot manager or NULL on error.

 */

struct keyslot_manager *keyslot_manager_create_passthrough(

	struct device *dev,

	const struct keyslot_mgmt_ll_ops *ksm_ll_ops,

	unsigned int features,

	const unsigned int crypto_mode_supported[BLK_ENCRYPTION_MODE_MAX],
@@ -518,6 +583,7 @@ struct keyslot_manager *keyslot_manager_create_passthrough( 
	       sizeof(ksm->crypto_mode_supported));

	ksm->max_dun_bytes_supported = BLK_CRYPTO_MAX_IV_SIZE;

	ksm->ll_priv_data = ll_priv_data;

	keyslot_manager_set_dev(ksm, dev);



	init_rwsem(&ksm->lock);
@@ -583,15 +649,15 @@ int keyslot_manager_derive_raw_secret(struct keyslot_manager *ksm, 
{

	int err;



	down_write(&ksm->lock);

	if (ksm->ksm_ll_ops.derive_raw_secret) {

		keyslot_manager_hw_enter(ksm);

		err = ksm->ksm_ll_ops.derive_raw_secret(ksm, wrapped_key,

							wrapped_key_size,

							secret, secret_size);

		keyslot_manager_hw_exit(ksm);

	} else {

		err = -EOPNOTSUPP;

	}

	up_write(&ksm->lock);



	return err;

}

drivers/md/dm.c

+2 −1
Original line number Diff line number Diff line
@@ -2178,7 +2178,8 @@ static int dm_init_inline_encryption(struct mapped_device *md) 
		   BLK_CRYPTO_FEATURE_WRAPPED_KEYS;

	memset(mode_masks, 0xFF, sizeof(mode_masks));



	md->queue->ksm = keyslot_manager_create_passthrough(&dm_ksm_ll_ops,

	md->queue->ksm = keyslot_manager_create_passthrough(NULL,

							    &dm_ksm_ll_ops,

							    features,

							    mode_masks, md);

	if (!md->queue->ksm)

drivers/mmc/host/cmdq_hci-crypto-qti.c

+7 −11
Original line number Diff line number Diff line
@@ -108,12 +108,9 @@ static int cmdq_crypto_qti_keyslot_program(struct keyslot_manager *ksm, 
	crypto_alg_id = cmdq_crypto_cap_find(host, key->crypto_mode,

					       key->data_unit_size);



	pm_runtime_get_sync(&host->mmc->card->dev);



	if (!cmdq_is_crypto_enabled(host) ||

	    !cmdq_keyslot_valid(host, slot) ||

	    !ice_cap_idx_valid(host, crypto_alg_id)) {

		pm_runtime_put_sync(&host->mmc->card->dev);

		return -EINVAL;

	}
@@ -121,7 +118,6 @@ static int cmdq_crypto_qti_keyslot_program(struct keyslot_manager *ksm, 


	if (!(data_unit_mask &

	      host->crypto_cap_array[crypto_alg_id].sdus_mask)) {

		pm_runtime_put_sync(&host->mmc->card->dev);

		return -EINVAL;

	}
@@ -130,8 +126,6 @@ static int cmdq_crypto_qti_keyslot_program(struct keyslot_manager *ksm, 
	if (err)

		pr_err("%s: failed with error %d\n", __func__, err);



	pm_runtime_put_sync(&host->mmc->card->dev);



	return err;

}
@@ -225,7 +219,8 @@ int cmdq_host_init_crypto_qti_spec(struct cmdq_host *host, 


	crypto_modes_supported[blk_mode_num] |= CRYPTO_CDU_SIZE * 512;



	host->ksm = keyslot_manager_create(cmdq_num_keyslots(host), ksm_ops,

	host->ksm = keyslot_manager_create(host->mmc->parent,

					   cmdq_num_keyslots(host), ksm_ops,

					   BLK_CRYPTO_FEATURE_STANDARD_KEYS |

					   BLK_CRYPTO_FEATURE_WRAPPED_KEYS,

					   crypto_modes_supported, host);
@@ -306,7 +301,8 @@ int cmdq_host_init_crypto_qti_spec(struct cmdq_host *host, 
				host->crypto_cap_array[cap_idx].sdus_mask * 512;

	}



	host->ksm = keyslot_manager_create(cmdq_num_keyslots(host), ksm_ops,

	host->ksm = keyslot_manager_create(host->mmc->parent,

					   cmdq_num_keyslots(host), ksm_ops,

					   BLK_CRYPTO_FEATURE_STANDARD_KEYS |

					   BLK_CRYPTO_FEATURE_WRAPPED_KEYS,

					   crypto_modes_supported, host);

drivers/mmc/host/cmdq_hci-crypto.c

+2 −1
Original line number Diff line number Diff line
@@ -342,7 +342,8 @@ int cmdq_host_init_crypto_spec(struct cmdq_host *host, 


	cmdq_crypto_clear_all_keyslots(host);



	host->ksm = keyslot_manager_create(cmdq_num_keyslots(host), ksm_ops,

	host->ksm = keyslot_manager_create(host->mmc->parent,

					   cmdq_num_keyslots(host), ksm_ops,

					   BLK_CRYPTO_FEATURE_STANDARD_KEYS,

					   crypto_modes_supported, host);