| 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
| ||
|
Message-ID: <ju5mbbuowwfcfvx7mtxazthhmg2qwciw4nsivaevucwkvapwmb@lei2evlubjdo>
Date: Mon, 2 Dec 2024 14:15:00 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Jens Axboe <axboe@...nel.dk>, Jonathan Corbet <corbet@....net>,
Alasdair Kergon <agk@...hat.com>, Mike Snitzer <snitzer@...nel.org>,
Mikulas Patocka <mpatocka@...hat.com>, Adrian Hunter <adrian.hunter@...el.com>,
Asutosh Das <quic_asutoshd@...cinc.com>, Ritesh Harjani <ritesh.list@...il.com>,
Ulf Hansson <ulf.hansson@...aro.org>, Alim Akhtar <alim.akhtar@...sung.com>,
Avri Altman <avri.altman@....com>, Bart Van Assche <bvanassche@....org>,
"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>, Gaurav Kashyap <quic_gaurkash@...cinc.com>,
Neil Armstrong <neil.armstrong@...aro.org>, "Martin K. Petersen" <martin.petersen@...cle.com>,
Eric Biggers <ebiggers@...nel.org>, "Theodore Y. Ts'o" <tytso@....edu>,
Jaegeuk Kim <jaegeuk@...nel.org>, Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
linux-block@...r.kernel.org, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
dm-devel@...ts.linux.dev, linux-mmc@...r.kernel.org, linux-scsi@...r.kernel.org,
linux-fscrypt@...r.kernel.org, linux-fsdevel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, Om Prakash Singh <quic_omprsing@...cinc.com>
Subject: Re: [PATCH RESEND v7 05/17] ice, ufs, mmc: use the blk_crypto_key
struct when programming the key
On Mon, Dec 02, 2024 at 01:02:21PM +0100, Bartosz Golaszewski wrote:
> From: Gaurav Kashyap <quic_gaurkash@...cinc.com>
>
> The program key ops in the storage controller does not pass on the
> blk_crypto_key structure to ICE, this is okay with raw keys of standard
> AES XTS sizes. However, wrapped keyblobs can be of any size and in
> preparation for that, modify the ICE and storage controller APIs to
> accept blk_crypto_key which can carry larger keys and indicate their
> size.
>
> Reviewed-by: Om Prakash Singh <quic_omprsing@...cinc.com>
> Tested-by: Neil Armstrong <neil.armstrong@...aro.org>
> Acked-by: Ulf Hansson <ulf.hansson@...aro.org> # For MMC
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@...cinc.com>
> Reviewed-by: Konrad Dybcio <konradybcio@...nel.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> ---
> drivers/mmc/host/cqhci-crypto.c | 7 ++++---
> drivers/mmc/host/cqhci.h | 2 ++
> drivers/mmc/host/sdhci-msm.c | 6 ++++--
> drivers/soc/qcom/ice.c | 6 +++---
> drivers/ufs/core/ufshcd-crypto.c | 7 ++++---
> drivers/ufs/host/ufs-qcom.c | 6 ++++--
> include/soc/qcom/ice.h | 5 +++--
> include/ufs/ufshcd.h | 1 +
> 8 files changed, 25 insertions(+), 15 deletions(-)
I think this needs to be split on a per-subsystem basis. Otherwise it
might be impossible to merge.
>
> diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
> index 6652982410ec5..91da6de1d6501 100644
> --- a/drivers/mmc/host/cqhci-crypto.c
> +++ b/drivers/mmc/host/cqhci-crypto.c
> @@ -32,6 +32,7 @@ cqhci_host_from_crypto_profile(struct blk_crypto_profile *profile)
> }
>
> static int cqhci_crypto_program_key(struct cqhci_host *cq_host,
> + const struct blk_crypto_key *bkey,
> const union cqhci_crypto_cfg_entry *cfg,
> int slot)
> {
> @@ -39,7 +40,7 @@ static int cqhci_crypto_program_key(struct cqhci_host *cq_host,
> int i;
>
> if (cq_host->ops->program_key)
> - return cq_host->ops->program_key(cq_host, cfg, slot);
> + return cq_host->ops->program_key(cq_host, bkey, cfg, slot);
>
> /* Clear CFGE */
> cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
> @@ -99,7 +100,7 @@ static int cqhci_crypto_keyslot_program(struct blk_crypto_profile *profile,
> memcpy(cfg.crypto_key, key->raw, key->size);
> }
>
> - err = cqhci_crypto_program_key(cq_host, &cfg, slot);
> + err = cqhci_crypto_program_key(cq_host, key, &cfg, slot);
>
> memzero_explicit(&cfg, sizeof(cfg));
> return err;
> @@ -113,7 +114,7 @@ static int cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
> */
> union cqhci_crypto_cfg_entry cfg = {};
>
> - return cqhci_crypto_program_key(cq_host, &cfg, slot);
> + return cqhci_crypto_program_key(cq_host, NULL, &cfg, slot);
> }
>
> static int cqhci_crypto_keyslot_evict(struct blk_crypto_profile *profile,
> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> index fab9d74445ba7..06099fd32f23e 100644
> --- a/drivers/mmc/host/cqhci.h
> +++ b/drivers/mmc/host/cqhci.h
> @@ -12,6 +12,7 @@
> #include <linux/completion.h>
> #include <linux/wait.h>
> #include <linux/irqreturn.h>
> +#include <linux/blk-crypto.h>
> #include <asm/io.h>
>
> /* registers */
> @@ -291,6 +292,7 @@ struct cqhci_host_ops {
> void (*post_disable)(struct mmc_host *mmc);
> #ifdef CONFIG_MMC_CRYPTO
> int (*program_key)(struct cqhci_host *cq_host,
> + const struct blk_crypto_key *bkey,
> const union cqhci_crypto_cfg_entry *cfg, int slot);
> #endif
> void (*set_tran_desc)(struct cqhci_host *cq_host, u8 **desc,
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e00208535bd1c..b8770524c0087 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1859,6 +1859,7 @@ static __maybe_unused int sdhci_msm_ice_suspend(struct sdhci_msm_host *msm_host)
> * vendor-specific SCM calls for this; it doesn't support the standard way.
> */
> static int sdhci_msm_program_key(struct cqhci_host *cq_host,
> + const struct blk_crypto_key *bkey,
> const union cqhci_crypto_cfg_entry *cfg,
> int slot)
> {
> @@ -1866,6 +1867,7 @@ static int sdhci_msm_program_key(struct cqhci_host *cq_host,
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> union cqhci_crypto_cap_entry cap;
> + u8 ice_key_size;
>
> /* Only AES-256-XTS has been tested so far. */
> cap = cq_host->crypto_cap_array[cfg->crypto_cap_idx];
> @@ -1873,11 +1875,11 @@ static int sdhci_msm_program_key(struct cqhci_host *cq_host,
> cap.key_size != CQHCI_CRYPTO_KEY_SIZE_256)
> return -EINVAL;
>
> + ice_key_size = QCOM_ICE_CRYPTO_KEY_SIZE_256;
> if (cfg->config_enable & CQHCI_CRYPTO_CONFIGURATION_ENABLE)
> return qcom_ice_program_key(msm_host->ice,
> QCOM_ICE_CRYPTO_ALG_AES_XTS,
> - QCOM_ICE_CRYPTO_KEY_SIZE_256,
> - cfg->crypto_key,
> + ice_key_size, bkey,
> cfg->data_unit_size, slot);
> else
> return qcom_ice_evict_key(msm_host->ice, slot);
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 393d2d1d275f1..e89baaf574bc1 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -163,8 +163,8 @@ EXPORT_SYMBOL_GPL(qcom_ice_suspend);
>
> int qcom_ice_program_key(struct qcom_ice *ice,
> u8 algorithm_id, u8 key_size,
> - const u8 crypto_key[], u8 data_unit_size,
> - int slot)
> + const struct blk_crypto_key *bkey,
> + u8 data_unit_size, int slot)
> {
> struct device *dev = ice->dev;
> union {
> @@ -183,7 +183,7 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> return -EINVAL;
> }
>
> - memcpy(key.bytes, crypto_key, AES_256_XTS_KEY_SIZE);
> + memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
>
> /* The SCM call requires that the key words are encoded in big endian */
> for (i = 0; i < ARRAY_SIZE(key.words); i++)
> diff --git a/drivers/ufs/core/ufshcd-crypto.c b/drivers/ufs/core/ufshcd-crypto.c
> index 7d3a3e228db0d..33083e0cad6e1 100644
> --- a/drivers/ufs/core/ufshcd-crypto.c
> +++ b/drivers/ufs/core/ufshcd-crypto.c
> @@ -18,6 +18,7 @@ static const struct ufs_crypto_alg_entry {
> };
>
> static int ufshcd_program_key(struct ufs_hba *hba,
> + const struct blk_crypto_key *bkey,
> const union ufs_crypto_cfg_entry *cfg, int slot)
> {
> int i;
> @@ -27,7 +28,7 @@ static int ufshcd_program_key(struct ufs_hba *hba,
> ufshcd_hold(hba);
>
> if (hba->vops && hba->vops->program_key) {
> - err = hba->vops->program_key(hba, cfg, slot);
> + err = hba->vops->program_key(hba, bkey, cfg, slot);
> goto out;
> }
>
> @@ -89,7 +90,7 @@ static int ufshcd_crypto_keyslot_program(struct blk_crypto_profile *profile,
> memcpy(cfg.crypto_key, key->raw, key->size);
> }
>
> - err = ufshcd_program_key(hba, &cfg, slot);
> + err = ufshcd_program_key(hba, key, &cfg, slot);
>
> memzero_explicit(&cfg, sizeof(cfg));
> return err;
> @@ -107,7 +108,7 @@ static int ufshcd_crypto_keyslot_evict(struct blk_crypto_profile *profile,
> */
> union ufs_crypto_cfg_entry cfg = {};
>
> - return ufshcd_program_key(hba, &cfg, slot);
> + return ufshcd_program_key(hba, NULL, &cfg, slot);
> }
>
> /*
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 68040b2ab5f82..44fb4a4c0f2d7 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -150,6 +150,7 @@ static inline int ufs_qcom_ice_suspend(struct ufs_qcom_host *host)
> }
>
> static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
> + const struct blk_crypto_key *bkey,
> const union ufs_crypto_cfg_entry *cfg,
> int slot)
> {
> @@ -157,6 +158,7 @@ static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
> union ufs_crypto_cap_entry cap;
> bool config_enable =
> cfg->config_enable & UFS_CRYPTO_CONFIGURATION_ENABLE;
> + u8 ice_key_size;
>
> /* Only AES-256-XTS has been tested so far. */
> cap = hba->crypto_cap_array[cfg->crypto_cap_idx];
> @@ -164,11 +166,11 @@ static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
> cap.key_size != UFS_CRYPTO_KEY_SIZE_256)
> return -EOPNOTSUPP;
>
> + ice_key_size = QCOM_ICE_CRYPTO_KEY_SIZE_256;
> if (config_enable)
> return qcom_ice_program_key(host->ice,
> QCOM_ICE_CRYPTO_ALG_AES_XTS,
> - QCOM_ICE_CRYPTO_KEY_SIZE_256,
> - cfg->crypto_key,
> + ice_key_size, bkey,
> cfg->data_unit_size, slot);
> else
> return qcom_ice_evict_key(host->ice, slot);
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 5870a94599a25..9dd835dba2a78 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -7,6 +7,7 @@
> #define __QCOM_ICE_H__
>
> #include <linux/types.h>
> +#include <linux/blk-crypto.h>
>
> struct qcom_ice;
>
> @@ -30,8 +31,8 @@ int qcom_ice_resume(struct qcom_ice *ice);
> int qcom_ice_suspend(struct qcom_ice *ice);
> int qcom_ice_program_key(struct qcom_ice *ice,
> u8 algorithm_id, u8 key_size,
> - const u8 crypto_key[], u8 data_unit_size,
> - int slot);
> + const struct blk_crypto_key *bkey,
> + u8 data_unit_size, int slot);
> int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> struct qcom_ice *of_qcom_ice_get(struct device *dev);
> #endif /* __QCOM_ICE_H__ */
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index d7aca9e61684f..bc6f08397769c 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -373,6 +373,7 @@ struct ufs_hba_variant_ops {
> struct devfreq_dev_profile *profile,
> struct devfreq_simple_ondemand_data *data);
> int (*program_key)(struct ufs_hba *hba,
> + const struct blk_crypto_key *bkey,
> const union ufs_crypto_cfg_entry *cfg, int slot);
> int (*fill_crypto_prdt)(struct ufs_hba *hba,
> const struct bio_crypt_ctx *crypt_ctx,
>
> --
> 2.45.2
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists