Re: [PATCH v6 6/9] scsi: ufs: Add inline encryption support to UFS

Eric Biggers
2020-01-18T03:58:08Z

On Wed, Dec 18, 2019 at 06:51:33AM -0800, Satya Tangirala wrote:
> @@ -4654,6 +4686,8 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
> if (ufshcd_is_rpm_autosuspend_allowed(hba))
> sdev->rpm_autosuspend = 1;
>
> + ufshcd_crypto_setup_rq_keyslot_manager(hba, q);
> +
> return 0;
> }
>
> @@ -4664,6 +4698,7 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
> static void ufshcd_slave_destroy(struct scsi_device *sdev)
> {
> struct ufs_hba *hba;
> + struct request_queue *q = sdev->request_queue;
>
> hba = shost_priv(sdev->host);
> /* Drop the reference as it won't be needed anymore */
> @@ -4674,6 +4709,8 @@ static void ufshcd_slave_destroy(struct scsi_device *sdev)
> hba->sdev_ufs_device = NULL;
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> }
> +
> + ufshcd_crypto_destroy_rq_keyslot_manager(hba, q);
> }

Just noticed this --- this is still destroying the keyslot manager when a SCSI
device is destroyed. The keyslot manager is associated with the host controller
(which might control multiple devices), so it must not be destroyed until the
ufs_hba is destroyed, i.e. in ufshcd_dealloc_host().

(I was also thinking about whether we could use devm so that the keyslot_manager
doesn't need to be explicitly freed. But that wouldn't actually help because we
still need to ensure that all the crypto keys get zeroed.)

- Eric