On 10.08.21 13:29, David Gstir wrote:
> Hi Ahmad,
>
>> On 21.07.2021, at 18:48, Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote:
>
>
> [...]
>
>> diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
>> new file mode 100644
>> index 000000000000..513d3f90e438
>> --- /dev/null
>> +++ b/drivers/crypto/caam/blob_gen.c
>> @@ -0,0 +1,230 @@
>
> [...]
>
>> +
>> +int caam_encap_blob(struct caam_blob_priv *priv, const char *keymod,
>> + void *input, void *output, size_t length)
>> +{
>> + u32 *desc;
>> + struct device *jrdev = &priv->jrdev;
>> + dma_addr_t dma_in, dma_out;
>> + struct caam_blob_job_result testres;
>> + size_t keymod_len = strlen(keymod);
>> + int ret;
>> +
>> + if (length <= CAAM_BLOB_OVERHEAD || keymod_len > CAAM_BLOB_KEYMOD_LENGTH)
>
> The docs for this function mention the length <= CAAM_BLOB_MAX_LEN
> restriction. This is not checked here. Is this intended?
Yes.
> Since you already assert that MAX_BLOB_SIZE <= CAAM_BLOB_MAX_LEN
> in security/keys/trusted-keys/trusted_caam.c, this will never
> be an issue for CAAM-based trusted-keys though.
I omitted checks in code, which are verified at compile-time.
Would you prefer a runtime check to be added as well?
>> + return -EINVAL;
>> +
>> + desc = caam_blob_alloc_desc(keymod_len);
>> + if (!desc) {
>> + dev_err(jrdev, "unable to allocate desc\n");
>> + return -ENOMEM;
>> + }
>> +
>
> [...]
>
>> diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
>> new file mode 100644
>> index 000000000000..aebbc9335f64
>> --- /dev/null
>> +++ b/include/soc/fsl/caam-blob.h
>> @@ -0,0 +1,56 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <kernel@xxxxxxxxxxxxxx>
>> + */
>> +
>> +#ifndef __CAAM_BLOB_GEN
>> +#define __CAAM_BLOB_GEN
>> +
>> +#include <linux/types.h>
>> +
>> +#define CAAM_BLOB_KEYMOD_LENGTH 16
>> +#define CAAM_BLOB_OVERHEAD (32 + 16)
>> +#define CAAM_BLOB_MAX_LEN 4096
>> +
>> +struct caam_blob_priv;
>> +
>> +/** caam_blob_gen_init - initialize blob generation
>> + *
>> + * returns either pointer to new caam_blob_priv instance
>> + * or error pointer
>> + */
>> +struct caam_blob_priv *caam_blob_gen_init(void);
>> +
>> +/** caam_blob_gen_init - free blob generation resources
>
> s/init/exit/
Oh, thanks for catching.
>> + *
>> + * @priv: instance returned by caam_blob_gen_init
>> + */
>> +void caam_blob_gen_exit(struct caam_blob_priv *priv);
>
>
> Except these minor things, I noticed no issues with this whole series:
>
> Reviewed-by: David Gstir <david@xxxxxxxxxxxxx>
Thanks! Will include it with the next iteration.
Cheers,
Ahmad
>
>
>
--
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 |
