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

Commit 582a710a authored by Eric Biggers's avatar Eric Biggers
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>
parent 0b145f91

block/blk-crypto-fallback.c

+1 −1
Original line number Diff line number Diff line
@@ -608,7 +608,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,

						crypto_mode_supported, NULL);

	if (!blk_crypto_ksm)

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>
@@ -45,6 +46,11 @@ struct keyslot_manager { 
	unsigned int crypto_mode_supported[BLK_ENCRYPTION_MODE_MAX];

	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;
@@ -71,8 +77,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
@@ -92,7 +150,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,

	const unsigned int crypto_mode_supported[BLK_ENCRYPTION_MODE_MAX],

	void *ll_priv_data)
@@ -118,6 +178,7 @@ struct keyslot_manager *keyslot_manager_create(unsigned int num_slots, 
	memcpy(ksm->crypto_mode_supported, crypto_mode_supported,

	       sizeof(ksm->crypto_mode_supported));

	ksm->ll_priv_data = ll_priv_data;

	keyslot_manager_set_dev(ksm, dev);



	init_rwsem(&ksm->lock);
@@ -226,10 +287,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;

		}
@@ -240,7 +301,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));

	}
@@ -252,7 +313,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;

	}
@@ -266,7 +327,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;

}
@@ -368,15 +429,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;
@@ -396,7 +458,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;

}
@@ -416,6 +478,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];
@@ -455,6 +518,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

 * @crypto_mode_supported: Bitmasks for supported encryption modes

 * @ll_priv_data: Private data passed as is to the functions in ksm_ll_ops.
@@ -471,6 +535,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,

	const unsigned int crypto_mode_supported[BLK_ENCRYPTION_MODE_MAX],

	void *ll_priv_data)
@@ -485,6 +550,7 @@ struct keyslot_manager *keyslot_manager_create_passthrough( 
	memcpy(ksm->crypto_mode_supported, crypto_mode_supported,

	       sizeof(ksm->crypto_mode_supported));

	ksm->ll_priv_data = ll_priv_data;

	keyslot_manager_set_dev(ksm, dev);



	init_rwsem(&ksm->lock);
@@ -544,15 +610,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
@@ -2287,7 +2287,8 @@ static int dm_init_inline_encryption(struct mapped_device *md) 
	 */

	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,

							    mode_masks, md);

	if (!md->queue->ksm)

		return -ENOMEM;

drivers/scsi/ufs/ufshcd-crypto.c

+2 −4
Original line number Diff line number Diff line
@@ -125,7 +125,6 @@ static int ufshcd_program_key(struct ufs_hba *hba, 
	u32 slot_offset = hba->crypto_cfg_register + slot * sizeof(*cfg);

	int err;



	pm_runtime_get_sync(hba->dev);

	ufshcd_hold(hba, false);



	if (hba->vops->program_key) {
@@ -155,7 +154,6 @@ static int ufshcd_program_key(struct ufs_hba *hba, 
	err = 0;

out:

	ufshcd_release(hba);

	pm_runtime_put_sync(hba->dev);

	return err;

}
@@ -337,8 +335,8 @@ int ufshcd_hba_init_crypto_spec(struct ufs_hba *hba, 


	ufshcd_clear_all_keyslots(hba);



	hba->ksm = keyslot_manager_create(ufshcd_num_keyslots(hba), ksm_ops,

					  crypto_modes_supported, hba);

	hba->ksm = keyslot_manager_create(hba->dev, ufshcd_num_keyslots(hba),

					  ksm_ops, crypto_modes_supported, hba);



	if (!hba->ksm) {

		err = -ENOMEM;

include/linux/keyslot-manager.h

+4 −1
Original line number Diff line number Diff line
@@ -41,7 +41,9 @@ struct keyslot_mgmt_ll_ops { 
				 u8 *secret, unsigned int secret_size);

};



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

	const unsigned int crypto_mode_supported[BLK_ENCRYPTION_MODE_MAX],

	void *ll_priv_data);
@@ -67,6 +69,7 @@ void *keyslot_manager_private(struct keyslot_manager *ksm); 
void keyslot_manager_destroy(struct keyslot_manager *ksm);



struct keyslot_manager *keyslot_manager_create_passthrough(

	struct device *dev,

	const struct keyslot_mgmt_ll_ops *ksm_ops,

	const unsigned int crypto_mode_supported[BLK_ENCRYPTION_MODE_MAX],

	void *ll_priv_data);