Re: [PATCH v8 3/6] crypto: caam - add in-kernel interface for blob generator

From: Ahmad Fatoum
Date: Wed May 04 2022 - 02:39:57 EST


Hello Michael,

On 03.05.22 20:24, Michael Walle wrote:
>> Add functions to realize encrypting and decrypting into memory alongside
>> the CAAM driver.
>>
>> They will be used in a later commit as a source for the trusted key
>> seal/unseal mechanism.
>
> Thanks for the work on this and I'm excited to try this. I'm currently
> playing with this and one thing I've noticed is that an export restricted
> CAAM isn't handled properly.

I didn't know there are still crypto export restrictions in place ;o

> That is, there are CAAM's which aren't fully featured. Normally, the
> caam driver will take care of it. For example, see commit f20311cc9c58
> ("crypto: caam - disable pkc for non-E SoCs"). For the trusted keys case,
> it would be nice if the kernel will not even probe (or similar).
>
> Right now, everything seems to work fine, but once I try to add a new key,
> I'll get the following errros:
>
> # keyctl add trusted mykey "new 32" @u
> add_key: Invalid argument
> [ 23.138714] caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.
> [ 23.138740] trusted_key: key_seal failed (-22)

Trusted key core will attempt TPM and TEE if enabled before trying CAAM unless
CAAM was explicitly requested. Silently failing in this case would not be
helpful to users. I think an info message (not error, as it'd be annoying to
see it every time booting a restricted SoC) is a good idea.
Thanks for the feedback.

> Again this is expected, because I run it on a non-E version. IMHO, it
> should be that the trusted keys shouldn't be enabled at all. Like it is
> for example if an unknown rng is given:
>
> trusted_key: Unsupported RNG. Supported: kernel, default

Other backends return -ENODEV and Trusted key core will ignore and try next
in list. Please give below patch a try. I tested it on normal unrestricted
i.MX6. If that's what you had in mind, I can incorporate it into v9.
If you have any Tested-by's or the like you want me to add, please tell. :)

Cheers,
Ahmad

------------------------------ 8< ------------------------------

diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
index d0b1a0015308..1d07e056a5dd 100644
--- a/drivers/crypto/caam/blob_gen.c
+++ b/drivers/crypto/caam/blob_gen.c
@@ -4,6 +4,8 @@
* Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@xxxxxxxxxxxxxx>
*/

+#define pr_fmt(fmt) "caam blob_gen: " fmt
+
#include <linux/device.h>
#include <soc/fsl/caam-blob.h>

@@ -147,11 +149,27 @@ EXPORT_SYMBOL(caam_process_blob);

struct caam_blob_priv *caam_blob_gen_init(void)
{
+ struct caam_drv_private *ctrlpriv;
struct device *jrdev;

+ /*
+ * caam_blob_gen_init() may expectedly fail with -ENODEV, e.g. when
+ * CAAM driver didn't probe or when SoC lacks BLOB support. An
+ * error would be harsh in this case, so we stick to info level.
+ */
+
jrdev = caam_jr_alloc();
- if (IS_ERR(jrdev))
- return ERR_CAST(jrdev);
+ if (IS_ERR(jrdev)) {
+ pr_info("no job ring available\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ ctrlpriv = dev_get_drvdata(jrdev->parent);
+ if (!ctrlpriv->blob_present) {
+ dev_info(jrdev, "no hardware blob generation support\n");
+ caam_jr_free(jrdev);
+ return ERR_PTR(-ENODEV);
+ }

return container_of(jrdev, struct caam_blob_priv, jrdev);
}
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..a0a622ca5dd4 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -660,6 +660,10 @@ static int caam_probe(struct platform_device *pdev)

caam_little_end = !(bool)(rd_reg32(&ctrl->perfmon.status) &
(CSTA_PLEND | CSTA_ALT_PLEND));
+
+ comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ls);
+ ctrlpriv->blob_present = !!(comp_params & CTPR_LS_BLOB);
+
comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ms);
if (comp_params & CTPR_MS_PS && rd_reg32(&ctrl->mcr) & MCFGR_LONG_PTR)
caam_ptr_sz = sizeof(u64);
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 7d45b21bd55a..e92210e2ab76 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -92,6 +92,7 @@ struct caam_drv_private {
*/
u8 total_jobrs; /* Total Job Rings in device */
u8 qi_present; /* Nonzero if QI present in device */
+ u8 blob_present; /* Nonzero if BLOB support present in device */
u8 mc_en; /* Nonzero if MC f/w is active */
int secvio_irq; /* Security violation interrupt number */
int virt_en; /* Virtualization enabled in CAAM */
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 3738625c0250..b829066f5063 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -414,6 +414,7 @@ struct caam_perfmon {
#define CTPR_MS_PG_SZ_MASK 0x10
#define CTPR_MS_PG_SZ_SHIFT 4
u32 comp_parms_ms; /* CTPR - Compile Parameters Register */
+#define CTPR_LS_BLOB BIT(1)
u32 comp_parms_ls; /* CTPR - Compile Parameters Register */
u64 rsvd1[2];

diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
index ec57eec4f2d2..8e821bd56e54 100644
--- a/include/soc/fsl/caam-blob.h
+++ b/include/soc/fsl/caam-blob.h
@@ -38,8 +38,9 @@ struct caam_blob_info {

/**
* caam_blob_gen_init - initialize blob generation
- * Return: pointer to new caam_blob_priv instance on success
- * and error pointer otherwise
+ * Return: pointer to new &struct caam_blob_priv instance on success
+ * and ``ERR_PTR(-ENODEV)`` if CAAM has no hardware blobbing support
+ * or no job ring could be allocated.
*/
struct caam_blob_priv *caam_blob_gen_init(void);

diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
index 46cb2484ec36..e3415c520c0a 100644
--- a/security/keys/trusted-keys/trusted_caam.c
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -55,10 +55,8 @@ static int trusted_caam_init(void)
int ret;

blobifier = caam_blob_gen_init();
- if (IS_ERR(blobifier)) {
- pr_err("Job Ring Device allocation for transform failed\n");
+ if (IS_ERR(blobifier))
return PTR_ERR(blobifier);
- }

ret = register_key_type(&key_type_trusted);
if (ret)
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |