lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <BD7FB879CC136E419544C8E2AE7B9C3020C816@IN01WEMBXA.internal.synopsys.com>
Date:   Thu, 7 Jun 2018 07:00:50 +0000
From:   Ladvine D Almeida <Ladvine.DAlmeida@...opsys.com>
To:     Jaegeuk Kim <jaegeuk@...nel.org>,
        Ladvine D Almeida <Ladvine.DAlmeida@...opsys.com>
CC:     "Vinayak Holikatti <vinholikatti@...il.com>; \"James E.J. Bottomley\"
        <jejb@...ux.vnet.ibm.com>; \"Martin K. Petersen\"" 
        <martin.petersen@...cle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-scsi@...r.kernel.org; Manjunath M Bettegowda
        <manjumb@...opsys.com>; Prabu Thangamuthu <prabut@...opsys.com>; Tejas
        Joglekar <joglekar@...opsys.com>; Joao Pinto" 
        <Joao.Pinto@...opsys.com>
Subject: Re: [PATCH 5/5] scsi: ufs: Add hooks in UFS HC driver for crypto
 support

On Monday 04 June 2018 10:22 PM, Jaegeuk Kim wrote:
> Hi Ladvine,
>
> On 05/28, Ladvine D Almeida wrote:
>> This patch adds the crypto support in the UFS Host Controller
>> driver like allocation of the crypto resources, registration of
>> crypto algorithm, preparing UTRD requests for inline encryption job,
>> freeing the resources during exit stage.
>> The crypto support is enabled in the UFS Host Controller driver only if the
>> crypto capability is detected in the hardware during the init stage.
>> The changes done for the UFS HC crypto support are guarded with
>> CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION.
>>
>> Signed-off-by: Ladvine D Almeida <ladvine@...opsys.com>
>> ---
>>  drivers/scsi/ufs/ufshcd.c | 63 ++++++++++++++++++++++++++++++++++++++++++-----
>>  drivers/scsi/ufs/ufshcd.h | 29 ++++++++++++++++++++++
>>  2 files changed, 86 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 00e7905..7bac5a3 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -1839,6 +1839,10 @@ static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
>>  	hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS) + 1;
>>  	hba->nutmrs =
>>  	((hba->capabilities & MASK_TASK_MANAGEMENT_REQUEST_SLOTS) >> 16) + 1;
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	/* read crypto capabilities of host controller */
>> +	ufshcd_read_crypto_capabilities(hba);
>> +#endif
>>  }
>>  
>>  /**
>> @@ -2076,7 +2080,7 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
>>  {
>>  	struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
>>  	u32 data_direction;
>> -	u32 dword_0;
>> +	u32 dword_0 = 0;
>>  
>>  	if (cmd_dir == DMA_FROM_DEVICE) {
>>  		data_direction = UTP_DEVICE_TO_HOST;
>> @@ -2094,10 +2098,22 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
>>  	if (lrbp->intr_cmd)
>>  		dword_0 |= UTP_REQ_DESC_INT_CMD;
>>  
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>>  	/* Transfer request descriptor header fields */
>> -	req_desc->header.dword_0 = cpu_to_le32(dword_0);
>> -	/* dword_1 is reserved, hence it is set to 0 */
>> +	if (lrbp->ccfg_idx >= 0) {
>> +		dword_0 |= (UTP_REQ_CRYPT_EN_CMD | (lrbp->ccfg_idx & 0xff));
>> +		req_desc->header.dword_1 = lrbp->lba & 0xffffffff;
>> +		req_desc->header.dword_3 = (lrbp->lba >> 32) & 0xffffffff;
> Please assign a logical value to DUN. (e.g., inode_number << 32 | page->offset)
> Otherwise upper layers like F2FS won't be able to move any block from one LBA to
> another LBA without the key. We couldn't do upstream tho, it'd be doable to add
> one DUN value in bio structure and manage its sequentiality by disallowing bio
> merges, which looks like requiring to modify ufshcd_prepare_for_crypto().

Kim,

I will consider the change in the new patch.

Regards,
Ladvine

>
> Thanks,
>
>> +	} else {
>> +		req_desc->header.dword_1 = 0;
>> +		req_desc->header.dword_3 = 0;
>> +	}
>> +#else
>>  	req_desc->header.dword_1 = 0;
>> +	req_desc->header.dword_3 = 0;
>> +#endif
>> +	req_desc->header.dword_0 = cpu_to_le32(dword_0);
>> +
>>  	/*
>>  	 * assigning invalid value for command status. Controller
>>  	 * updates OCS on command completion, with the command
>> @@ -2105,8 +2121,6 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
>>  	 */
>>  	req_desc->header.dword_2 =
>>  		cpu_to_le32(OCS_INVALID_COMMAND_STATUS);
>> -	/* dword_3 is reserved, hence it is set to 0 */
>> -	req_desc->header.dword_3 = 0;
>>  
>>  	req_desc->prd_table_length = 0;
>>  }
>> @@ -2355,6 +2369,12 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
>>  	lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun);
>>  	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false;
>>  	lrbp->req_abort_skip = false;
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	lrbp->lba = 0;
>> +	lrbp->ccfg_idx = -1;
>> +	/* prepare block for crypto */
>> +	ufshcd_prepare_for_crypto(hba, lrbp);
>> +#endif
>>  
>>  	ufshcd_comp_scsi_upiu(hba, lrbp);
>>  
>> @@ -3215,6 +3235,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
>>  static int ufshcd_memory_alloc(struct ufs_hba *hba)
>>  {
>>  	size_t utmrdl_size, utrdl_size, ucdl_size;
>> +	int ret = 0;
>>  
>>  	/* Allocate memory for UTP command descriptors */
>>  	ucdl_size = (sizeof(struct utp_transfer_cmd_desc) * hba->nutrs);
>> @@ -3233,6 +3254,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
>>  	    WARN_ON(hba->ucdl_dma_addr & (PAGE_SIZE - 1))) {
>>  		dev_err(hba->dev,
>>  			"Command Descriptor Memory allocation failed\n");
>> +		ret = -ENOMEM;
>>  		goto out;
>>  	}
>>  
>> @@ -3249,6 +3271,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
>>  	    WARN_ON(hba->utrdl_dma_addr & (PAGE_SIZE - 1))) {
>>  		dev_err(hba->dev,
>>  			"Transfer Descriptor Memory allocation failed\n");
>> +		ret = -ENOMEM;
>>  		goto out;
>>  	}
>>  
>> @@ -3265,6 +3288,7 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
>>  	    WARN_ON(hba->utmrdl_dma_addr & (PAGE_SIZE - 1))) {
>>  		dev_err(hba->dev,
>>  		"Task Management Descriptor Memory allocation failed\n");
>> +		ret = -ENOMEM;
>>  		goto out;
>>  	}
>>  
>> @@ -3274,11 +3298,21 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
>>  				GFP_KERNEL);
>>  	if (!hba->lrb) {
>>  		dev_err(hba->dev, "LRB Memory allocation failed\n");
>> +		ret = -ENOMEM;
>> +		goto out;
>> +	}
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	/* Allocate memory for crypto operations */
>> +	if (ufshcd_is_encryption_supported(hba))
>> +		ret = ufshcd_crypto_memory_alloc(hba);
>> +	if (ret != 0) {
>> +		dev_err(hba->dev, "Crypto Memory allocation failed\n");
>>  		goto out;
>>  	}
>> +#endif
>>  	return 0;
>>  out:
>> -	return -ENOMEM;
>> +	return ret;
>>  }
>>  
>>  /**
>> @@ -4208,6 +4242,19 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
>>  		goto out;
>>  
>>  	ret = ufshcd_make_hba_operational(hba);
>> +	if (ret)
>> +		goto out;
>> +
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	/* Enable algorithm if crypto capability is present */
>> +	if (ufshcd_is_encryption_supported(hba))
>> +		ret = ufshcd_enable_crypt_alg(hba);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Crypto setup failed (%d)\n",
>> +			ret);
>> +		ret = 0;
>> +	}
>> +#endif
>>  out:
>>  	if (ret) {
>>  		dev_err(hba->dev, "link startup failed %d\n", ret);
>> @@ -7661,6 +7708,10 @@ EXPORT_SYMBOL(ufshcd_shutdown);
>>   */
>>  void ufshcd_remove(struct ufs_hba *hba)
>>  {
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	ufshcd_disable_crypt_alg(hba);
>> +	ufshcd_remove_crypto_memory(hba);
>> +#endif
>>  	ufs_sysfs_remove_nodes(hba->dev);
>>  	scsi_remove_host(hba->host);
>>  	/* disable interrupts */
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index 8110dcd..b1356ca 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -68,6 +68,9 @@
>>  
>>  #include "ufs.h"
>>  #include "ufshci.h"
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	#include "ufshcd-crypto.h"
>> +#endif
>>  
>>  #define UFSHCD "ufshcd"
>>  #define UFSHCD_DRIVER_VERSION "0.2"
>> @@ -168,6 +171,8 @@ struct ufs_pm_lvl_states {
>>   * @issue_time_stamp: time stamp for debug purposes
>>   * @compl_time_stamp: time stamp for statistics
>>   * @req_abort_skip: skip request abort task flag
>> + * @lba: LBA value for crypto engine
>> + * @ccfg_idx: key configuration index for crypto engine
>>   */
>>  struct ufshcd_lrb {
>>  	struct utp_transfer_req_desc *utr_descriptor_ptr;
>> @@ -193,6 +198,10 @@ struct ufshcd_lrb {
>>  	ktime_t compl_time_stamp;
>>  
>>  	bool req_abort_skip;
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	u64 lba;
>> +	int ccfg_idx;
>> +#endif
>>  };
>>  
>>  /**
>> @@ -499,6 +508,8 @@ struct ufs_stats {
>>   * @urgent_bkops_lvl: keeps track of urgent bkops level for device
>>   * @is_urgent_bkops_lvl_checked: keeps track if the urgent bkops level for
>>   *  device is known or not.
>> + * @cc: stores the crypto capability information.
>> + * @ccxp: stores the crypto configuration information.
>>   */
>>  struct ufs_hba {
>>  	void __iomem *mmio_base;
>> @@ -683,6 +694,17 @@ struct ufs_hba {
>>  
>>  	struct rw_semaphore clk_scaling_lock;
>>  	struct ufs_desc_size desc_size;
>> +
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +	/*
>> +	 * Thus capability allows the host controller to perform
>> +	 * inline encryption once it is configured.
>> +	 */
>> +#define UFSHCD_CAP_INLINE_ENCRYPTION (1 << 6)
>> +
>> +	struct ufshcd_ccap cc;
>> +	struct ufshcd_x_crypto_cap *ccxp;
>> +#endif
>>  };
>>  
>>  /* Returns true if clocks can be gated. Otherwise false */
>> @@ -717,6 +739,13 @@ return true;
>>  #endif
>>  }
>>  
>> +#ifdef CONFIG_SCSI_UFSHCD_INLINE_ENCRYPTION
>> +static inline int ufshcd_is_encryption_supported(struct ufs_hba *hba)
>> +{
>> +	return hba->caps & UFSHCD_CAP_INLINE_ENCRYPTION;
>> +}
>> +#endif
>> +
>>  #define ufshcd_writel(hba, val, reg)	\
>>  	writel((val), (hba)->mmio_base + (reg))
>>  #define ufshcd_readl(hba, reg)	\
>> -- 
>> 2.7.4


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ