[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240130044320.GV2936378@hu-bjorande-lv.qualcomm.com>
Date: Mon, 29 Jan 2024 20:43:20 -0800
From: Bjorn Andersson <quic_bjorande@...cinc.com>
To: Gaurav Kashyap <quic_gaurkash@...cinc.com>
CC: <linux-arm-msm@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
<andersson@...nel.org>, <ebiggers@...gle.com>,
<neil.armstrong@...aro.org>, <srinivas.kandagatla@...aro.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<robh+dt@...nel.org>, <linux-kernel@...r.kernel.org>,
<linux-mmc@...r.kernel.org>, <kernel@...cinc.com>,
<linux-crypto@...r.kernel.org>, <devicetree@...r.kernel.org>,
<quic_omprsing@...cinc.com>, <quic_nguyenb@...cinc.com>,
<bartosz.golaszewski@...aro.org>, <konrad.dybcio@...aro.org>,
<ulf.hansson@...aro.org>, <jejb@...ux.ibm.com>,
<martin.petersen@...cle.com>, <mani@...nel.org>, <davem@...emloft.net>,
<herbert@...dor.apana.org.au>
Subject: Re: [PATCH v4 02/15] qcom_scm: scm call for deriving a software
secret
On Sat, Jan 27, 2024 at 03:14:00PM -0800, Gaurav Kashyap wrote:
The subject prefix does not match other changes to this file.
> Inline storage encryption may require deriving a software
> secret from storage keys added to the kernel.
>
> For non-wrapped keys, this can be directly done in the kernel as
> keys are in the clear.
>
> However, hardware wrapped keys can only be unwrapped by the wrapping
> entity. In case of Qualcomm's wrapped key solution, this is done by
> the Hardware Key Manager (HWKM) from Trustzone.
> Hence, adding a new SCM call which in the end provides a hook
> to the software secret crypto profile API provided by the block
> layer.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@...cinc.com>
> Tested-by: Neil Armstrong <neil.armstrong@...aro.org>
> ---
> drivers/firmware/qcom/qcom_scm.c | 65 ++++++++++++++++++++++++++
> drivers/firmware/qcom/qcom_scm.h | 1 +
> include/linux/firmware/qcom/qcom_scm.h | 2 +
> 3 files changed, 68 insertions(+)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 7e17fd662bda..4882f8a36453 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1220,6 +1220,71 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
> }
> EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
>
> +/**
> + * qcom_scm_derive_sw_secret() - Derive software secret from wrapped key
> + * @wkey: the hardware wrapped key inaccessible to software
> + * @wkey_size: size of the wrapped key
> + * @sw_secret: the secret to be derived which is exactly the secret size
> + * @sw_secret_size: size of the sw_secret
> + *
> + * Derive a software secret from a hardware wrapped key for software crypto
> + * operations.
> + * For wrapped keys, the key needs to be unwrapped, in order to derive a
> + * software secret, which can be done in the hardware from a secure execution
> + * environment.
> + *
> + * For more information on sw secret, please refer to "Hardware-wrapped keys"
> + * section of Documentation/block/inline-encryption.rst.
> + *
> + * Return: 0 on success; -errno on failure.
> + */
> +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> + u8 *sw_secret, size_t sw_secret_size)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_ES,
> + .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET,
> + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW,
> + QCOM_SCM_VAL, QCOM_SCM_RW,
> + QCOM_SCM_VAL),
> + .args[1] = wkey_size,
> + .args[3] = sw_secret_size,
> + .owner = ARM_SMCCC_OWNER_SIP,
> + };
> +
> + void *secret_buf;
> + void *wkey_buf;
> + int ret;
> +
> + wkey_buf = qcom_tzmem_alloc(__scm->mempool, wkey_size, GFP_KERNEL);
> + if (!wkey_buf)
> + return -ENOMEM;
> +
> + secret_buf = qcom_tzmem_alloc(__scm->mempool, sw_secret_size, GFP_KERNEL);
> + if (!secret_buf) {
> + ret = -ENOMEM;
> + goto err_free_wrapped;
> + }
> +
> + memcpy(wkey_buf, wkey, wkey_size);
> + desc.args[0] = qcom_tzmem_to_phys(wkey_buf);
> + desc.args[2] = qcom_tzmem_to_phys(secret_buf);
> +
> + ret = qcom_scm_call(__scm->dev, &desc, NULL);
> + if (!ret)
> + memcpy(sw_secret, secret_buf, sw_secret_size);
> +
> + memzero_explicit(secret_buf, sw_secret_size);
> + qcom_tzmem_free(secret_buf);
> +
> +err_free_wrapped:
This code path is shared between error path and normal path, prefixing
it "err_" is not helpful to the reader. Please change this to
out_free_wrapped:
The rest of the patch looks good to me.
Regards,
Bjorn
> + memzero_explicit(wkey_buf, wkey_size);
> + qcom_tzmem_free(wkey_buf);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_derive_sw_secret);
> +
> /**
> * qcom_scm_hdcp_available() - Check if secure environment supports HDCP.
> *
> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> index cb7273aa0a5e..56ff0806f5d2 100644
> --- a/drivers/firmware/qcom/qcom_scm.h
> +++ b/drivers/firmware/qcom/qcom_scm.h
> @@ -127,6 +127,7 @@ struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void);
> #define QCOM_SCM_SVC_ES 0x10 /* Enterprise Security */
> #define QCOM_SCM_ES_INVALIDATE_ICE_KEY 0x03
> #define QCOM_SCM_ES_CONFIG_SET_ICE_KEY 0x04
> +#define QCOM_SCM_ES_DERIVE_SW_SECRET 0x07
>
> #define QCOM_SCM_SVC_HDCP 0x11
> #define QCOM_SCM_HDCP_INVOKE 0x01
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index 9b6054813f59..89358478ac67 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -103,6 +103,8 @@ bool qcom_scm_ice_available(void);
> int qcom_scm_ice_invalidate_key(u32 index);
> int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
> enum qcom_scm_ice_cipher cipher, u32 data_unit_size);
> +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> + u8 *sw_secret, size_t sw_secret_size);
>
> bool qcom_scm_hdcp_available(void);
> int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
> --
> 2.43.0
>
Powered by blists - more mailing lists