[v6,3/4] crypto: caam - add in-kernel interface for blob generator

Commit Message

Ahmad Fatoum March 16, 2022, 4:43 p.m. UTC

The NXP Cryptographic Acceleration and Assurance Module (CAAM)
can be used to protect user-defined data across system reboot:

  - When the system is fused and boots into secure state, the master
    key is a unique never-disclosed device-specific key
  - random key is encrypted by key derived from master key
  - data is encrypted using the random key
  - encrypted data and its encrypted random key are stored alongside
  - This blob can now be safely stored in non-volatile memory

On next power-on:
  - blob is loaded into CAAM
  - CAAM writes decrypted data either into memory or key register

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.

Reviewed-by: David Gstir <[e-mail]>
Reviewed-by: Pankaj Gupta <[e-mail]>
Tested-By: Tim Harvey <[e-mail]>
Tested-by: Matthias Schiffer <[e-mail]>
Signed-off-by: Steffen Trumtrar <[e-mail]>
Signed-off-by: Ahmad Fatoum <[e-mail]>
---
v5 -> v6:
  - Dropped caam_blob_alloc_desc() in favor of kzalloc() with fixed size.
    This simplifies code and wastes at most 12 bytes which are freed
    at the end of the function anyway.
  - Factored out common code between caam_encap_blob and caam_decap_blob
    as both functions were largely identical
  - use append_seq_(in|out)_ptr_intlen for both encap/decap as a result
  - use reverse christmas tree order for caam_process_blob variable
    definitions.
v4 -> v5:
  - Collected Reviewed-by's and Tested-by's
  - Note in CAAM patch what CAAM is (Jarkko)
v3 -> v4:
  - Collected Acked-by's, Reviewed-by's and Tested-by
  - Fixed typo spotted by David
v2 -> v3:
 - No change
v1 -> v2:
 - Enforce maximum keymod size (Horia)
 - Use append_seq_(in|out)_ptr_intlen instead of append_seq_(in|out)_ptr
   (Horia)
 - Make blobifier handle private to CAAM glue code file (Horia)

To: "Horia Geantă" <[e-mail]>
To: Pankaj Gupta <[e-mail]>
To: Herbert Xu <[e-mail]>
To: "David S. Miller" <[e-mail]>
Cc: James Bottomley <[e-mail]>
Cc: Jarkko Sakkinen <[e-mail]>
Cc: Mimi Zohar <[e-mail]>
Cc: David Howells <[e-mail]>
Cc: James Morris <[e-mail]>
Cc: Eric Biggers <[e-mail]>
Cc: "Serge E. Hallyn" <[e-mail]>
Cc: Jan Luebbe <[e-mail]>
Cc: David Gstir <[e-mail]>
Cc: Richard Weinberger <[e-mail]>
Cc: Franck LENORMAND <[e-mail]>
Cc: Matthias Schiffer <[e-mail]>
Cc: Sumit Garg <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
---
 drivers/crypto/caam/Kconfig    |   3 +
 drivers/crypto/caam/Makefile   |   1 +
 drivers/crypto/caam/blob_gen.c | 160 +++++++++++++++++++++++++++++++++
 include/soc/fsl/caam-blob.h    |  79 ++++++++++++++++
 4 files changed, 243 insertions(+)
 create mode 100644 drivers/crypto/caam/blob_gen.c
 create mode 100644 include/soc/fsl/caam-blob.h

Comments

Pankaj Gupta March 22, 2022, 6:25 a.m. UTC | #1

Hi Ahmad,

Suggested to define macro with more details.
Please find comments in-line.

Regards
Pankaj
> -----Original Message-----
> From: Ahmad Fatoum <[e-mail]>
> Sent: Wednesday, March 16, 2022 10:14 PM
> To: Horia Geanta <[e-mail]>; Pankaj Gupta
> <[e-mail]>; Herbert Xu <[e-mail]>; David
> S. Miller <[e-mail]>
> Cc: [e-mail]; David Gstir <[e-mail]>;
> [e-mail]; Matthias Schiffer <[e-mail]-
> group.com>; Ahmad Fatoum <[e-mail]>; James Bottomley
> <[e-mail]>; Jarkko Sakkinen <[e-mail]>; Mimi Zohar
> <[e-mail]>; David Howells <[e-mail]>; James Morris
> <[e-mail]>; Eric Biggers <[e-mail]>; Serge E. Hallyn
> <[e-mail]>; Jan Luebbe <[e-mail]>; Richard
> Weinberger <[e-mail]>; Franck Lenormand
> <[e-mail]>; Sumit Garg <[e-mail]>; linux-
> [e-mail]; [e-mail]; linux-
> [e-mail]; [e-mail]; linux-security-
> [e-mail]
> Subject: [EXT] [PATCH v6 3/4] crypto: caam - add in-kernel interface for blob
> generator
> 
> Caution: EXT Email
> 
> The NXP Cryptographic Acceleration and Assurance Module (CAAM) can be used
> to protect user-defined data across system reboot:
> 
>   - When the system is fused and boots into secure state, the master
>     key is a unique never-disclosed device-specific key
>   - random key is encrypted by key derived from master key
>   - data is encrypted using the random key
>   - encrypted data and its encrypted random key are stored alongside
>   - This blob can now be safely stored in non-volatile memory
> 
> On next power-on:
>   - blob is loaded into CAAM
>   - CAAM writes decrypted data either into memory or key register
> 
> 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.
> 
> Reviewed-by: David Gstir <[e-mail]>
> Reviewed-by: Pankaj Gupta <[e-mail]>
> Tested-By: Tim Harvey <[e-mail]>
> Tested-by: Matthias Schiffer <[e-mail]>
> Signed-off-by: Steffen Trumtrar <[e-mail]>
> Signed-off-by: Ahmad Fatoum <[e-mail]>
> ---
> v5 -> v6:
>   - Dropped caam_blob_alloc_desc() in favor of kzalloc() with fixed size.
>     This simplifies code and wastes at most 12 bytes which are freed
>     at the end of the function anyway.
>   - Factored out common code between caam_encap_blob and
> caam_decap_blob
>     as both functions were largely identical
>   - use append_seq_(in|out)_ptr_intlen for both encap/decap as a result
>   - use reverse christmas tree order for caam_process_blob variable
>     definitions.
> v4 -> v5:
>   - Collected Reviewed-by's and Tested-by's
>   - Note in CAAM patch what CAAM is (Jarkko)
> v3 -> v4:
>   - Collected Acked-by's, Reviewed-by's and Tested-by
>   - Fixed typo spotted by David
> v2 -> v3:
>  - No change
> v1 -> v2:
>  - Enforce maximum keymod size (Horia)
>  - Use append_seq_(in|out)_ptr_intlen instead of append_seq_(in|out)_ptr
>    (Horia)
>  - Make blobifier handle private to CAAM glue code file (Horia)
> 
> To: "Horia Geantă" <[e-mail]>
> To: Pankaj Gupta <[e-mail]>
> To: Herbert Xu <[e-mail]>
> To: "David S. Miller" <[e-mail]>
> Cc: James Bottomley <[e-mail]>
> Cc: Jarkko Sakkinen <[e-mail]>
> Cc: Mimi Zohar <[e-mail]>
> Cc: David Howells <[e-mail]>
> Cc: James Morris <[e-mail]>
> Cc: Eric Biggers <[e-mail]>
> Cc: "Serge E. Hallyn" <[e-mail]>
> Cc: Jan Luebbe <[e-mail]>
> Cc: David Gstir <[e-mail]>
> Cc: Richard Weinberger <[e-mail]>
> Cc: Franck LENORMAND <[e-mail]>
> Cc: Matthias Schiffer <[e-mail]>
> Cc: Sumit Garg <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  drivers/crypto/caam/Kconfig    |   3 +
>  drivers/crypto/caam/Makefile   |   1 +
>  drivers/crypto/caam/blob_gen.c | 160 +++++++++++++++++++++++++++++++++
>  include/soc/fsl/caam-blob.h    |  79 ++++++++++++++++
>  4 files changed, 243 insertions(+)
>  create mode 100644 drivers/crypto/caam/blob_gen.c  create mode 100644
> include/soc/fsl/caam-blob.h
> 
> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig index
> 84ea7cba5ee5..ea9f8b1ae981 100644
> --- a/drivers/crypto/caam/Kconfig
> +++ b/drivers/crypto/caam/Kconfig
> @@ -151,6 +151,9 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API
>           Selecting this will register the SEC4 hardware rng to
>           the hw_random API for supplying the kernel entropy pool.
> 
> +config CRYPTO_DEV_FSL_CAAM_BLOB_GEN
> +       bool
> +
>  endif # CRYPTO_DEV_FSL_CAAM_JR
> 
>  endif # CRYPTO_DEV_FSL_CAAM
> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile index
> 3570286eb9ce..25f7ae5a4642 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -21,6 +21,7 @@ caam_jr-
> $(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caampkc.o
> pkc_desc.o
> +caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o
> 
>  caam-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += qi.o  ifneq
> ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),)
> diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
> new file mode 100644 index 000000000000..2564e63c8fe0
> --- /dev/null
> +++ b/drivers/crypto/caam/blob_gen.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2015 Pengutronix, Steffen Trumtrar
> +<[e-mail]>
> + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <[e-mail]>
> +*/
> +
> +#include <linux/device.h>
> +#include <soc/fsl/caam-blob.h>
> +
> +#include "compat.h"
> +#include "desc_constr.h"
> +#include "desc.h"
> +#include "error.h"
> +#include "intern.h"
> +#include "jr.h"
> +#include "regs.h"
> +

len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + 
>>>> + CAAM_PTR_SZ_MAX) + 4;

> +/* header + (key mod immediate) + 2x seq_intlen pointers + op */
> +#define CAAM_BLOB_DESC_BYTES_MAX \
> +       (CAAM_CMD_SZ + \
> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \
> +        2 * (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \
> +        CAAM_CMD_SZ)
> +

Suggested to replace the above macro like below:

+#define CAAM_BLOB_DESC_BYTES_MAX \			
+       (CAAM_CMD_SZ + \					/* Command to initialize & stating length of  descriptor */
+        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \	/* Command to append the key-modifier + followed by the key-modifier data */
+        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include input plain key and pointer to the input key */
+        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include output-key blob and pointer to the output-key blob */
+        CAAM_CMD_SZ)						/* Command describing the Operation to perform */



> +struct caam_blob_priv {
> +       struct device jrdev;
> +};
> +
> +struct caam_blob_job_result {
> +       int err;
> +       struct completion completion;
> +};
> +
> +static void caam_blob_job_done(struct device *dev, u32 *desc, u32 err,
> +void *context) {
> +       struct caam_blob_job_result *res = context;
> +       int ecode = 0;
> +
> +       dev_dbg(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
> +
> +       if (err)
> +               ecode = caam_jr_strstatus(dev, err);
> +
> +       res->err = ecode;
> +
> +       /*
> +        * Upon completion, desc points to a buffer containing a CAAM job
> +        * descriptor which encapsulates data into an externally-storable
> +        * blob.
> +        */
> +       complete(&res->completion);
> +}
> +
> +int caam_process_blob(struct caam_blob_priv *priv, bool encap,
> +                     const char *keymod, void *input, void *output,
> +                     size_t length)
> +{
> +       size_t in_len = length, out_len = length;
> +       struct caam_blob_job_result testres;
> +       struct device *jrdev = &priv->jrdev;
> +       dma_addr_t dma_in, dma_out;
> +       int op = OP_PCLID_BLOB;
> +       size_t keymod_len;
> +       u32 *desc;
> +       int ret;
> +
> +       keymod_len = strlen(keymod);
> +
> +       if (length <= CAAM_BLOB_OVERHEAD || keymod_len >
> CAAM_BLOB_KEYMOD_LENGTH)
> +               return -EINVAL;
> +
> +       if (encap) {
> +               op |= OP_TYPE_ENCAP_PROTOCOL;
> +               in_len -= CAAM_BLOB_OVERHEAD;
> +       } else {
> +               op |= OP_TYPE_DECAP_PROTOCOL;
> +               out_len -= CAAM_BLOB_OVERHEAD;
> +       }
> +
> +       desc = kzalloc(CAAM_BLOB_DESC_BYTES_MAX, GFP_KERNEL | GFP_DMA);
> +       if (!desc) {
> +               dev_err(jrdev, "unable to allocate desc\n");
> +               return -ENOMEM;
> +       }
> +
> +       dma_in = dma_map_single(jrdev, input, in_len, DMA_TO_DEVICE);
> +       if (dma_mapping_error(jrdev, dma_in)) {
> +               dev_err(jrdev, "unable to map input DMA buffer\n");
> +               ret = -ENOMEM;
> +               goto out_free;
> +       }
> +
> +       dma_out = dma_map_single(jrdev, output, out_len, DMA_FROM_DEVICE);
> +       if (dma_mapping_error(jrdev, dma_out)) {
> +               dev_err(jrdev, "unable to map output DMA buffer\n");
> +               ret = -ENOMEM;
> +               goto out_unmap_in;
> +       }
> +
> +       /*
> +        * A data blob is encrypted using a blob key (BK); a random number.
> +        * The BK is used as an AES-CCM key. The initial block (B0) and the
> +        * initial counter (Ctr0) are generated automatically and stored in
> +        * Class 1 Context DWords 0+1+2+3. The random BK is stored in the
> +        * Class 1 Key Register. Operation Mode is set to AES-CCM.
> +        */
> +
> +       init_job_desc(desc, 0);
> +       append_key_as_imm(desc, keymod, keymod_len, keymod_len,
> +                         CLASS_2 | KEY_DEST_CLASS_REG);
> +       append_seq_in_ptr_intlen(desc, dma_in, in_len, 0);
> +       append_seq_out_ptr_intlen(desc, dma_out, out_len, 0);
> +       append_operation(desc, op);
> +
> +       print_hex_dump_debug("data@"__stringify(__LINE__)": ",
> +                            DUMP_PREFIX_ADDRESS, 16, 1, input,
> +                            in_len, false);
> +       print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
> +                            DUMP_PREFIX_ADDRESS, 16, 1, desc,
> +                            desc_bytes(desc), false);
> +
> +       testres.err = 0;
> +       init_completion(&testres.completion);
> +
> +       ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, &testres);
> +       if (ret == -EINPROGRESS) {
> +               wait_for_completion(&testres.completion);
> +               ret = testres.err;
> +               print_hex_dump_debug("output@"__stringify(__LINE__)": ",
> +                                    DUMP_PREFIX_ADDRESS, 16, 1, output,
> +                                    out_len, false);
> +       }
> +
> +       dma_unmap_single(jrdev, dma_out, out_len, DMA_FROM_DEVICE);
> +out_unmap_in:
> +       dma_unmap_single(jrdev, dma_in, in_len, DMA_TO_DEVICE);
> +out_free:
> +       kfree(desc);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL(caam_process_blob);
> +
> +struct caam_blob_priv *caam_blob_gen_init(void) {
> +       struct device *jrdev;
> +
> +       jrdev = caam_jr_alloc();
> +       if (IS_ERR(jrdev))
> +               return ERR_CAST(jrdev);
> +
> +       return container_of(jrdev, struct caam_blob_priv, jrdev); }
> +EXPORT_SYMBOL(caam_blob_gen_init);
> +
> +void caam_blob_gen_exit(struct caam_blob_priv *priv) {
> +       caam_jr_free(&priv->jrdev);
> +}
> +EXPORT_SYMBOL(caam_blob_gen_exit);
> diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h new file
> mode 100644 index 000000000000..2c4a05dc59cd
> --- /dev/null
> +++ b/include/soc/fsl/caam-blob.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <[e-mail]>
> +*/
> +
> +#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_exit - free blob generation resources
> + *
> + * @priv: instance returned by caam_blob_gen_init  */ void
> +caam_blob_gen_exit(struct caam_blob_priv *priv);
> +
> +/** caam_process_blob - encapsulate or decapsulate blob
> + *
> + * @priv:   instance returned by caam_blob_gen_init
> + * @encap:  true for encapsulation, false for decapsulation
> + * @keymod: string to use as key modifier for blob capsulation
> + *         can't be longer than CAAM_BLOB_KEYMOD_LENGTH
> + * @input:  buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
> + * @length: buffer length including blob overhead
> + *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
> + */
> +int caam_process_blob(struct caam_blob_priv *priv, bool encap,
> +                     const char *keymod, void *input, void *output,
> +                     size_t length);
> +
> +/** caam_encap_blob - encapsulate blob
> + *
> + * @priv:   instance returned by caam_blob_gen_init
> + * @keymod: string to use as key modifier for blob encapsulation
> + *         can't be longer than CAAM_BLOB_KEYMOD_LENGTH
> + * @input:  buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
> + * @length: buffer length including blob overhead
> + *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
> + */
> +static inline int caam_encap_blob(struct caam_blob_priv *priv,
> +                                 const char *keymod,
> +                                 void *input, void *output, size_t
> +length) {
> +       return caam_process_blob(priv, true, keymod, input, output,
> +length); }
> +
> +/** caam_decap_blob - decapsulate blob
> + *
> + * @priv:   instance returned by caam_blob_gen_init
> + * @keymod: string to use as key modifier for blob decapsulation
> + *         can't be longer than CAAM_BLOB_KEYMOD_LENGTH
> + * @input:  buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
> + * @length: buffer length including blob overhead
> + *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
> + */
> +static inline int caam_decap_blob(struct caam_blob_priv *priv,
> +                                 const char *keymod,
> +                                 void *input, void *output, size_t
> +length) {
> +       return caam_process_blob(priv, false, keymod, input, output,
> +length); }
> +
> +#endif
> --
> 2.30.2

Ahmad Fatoum March 22, 2022, 7:32 a.m. UTC | #2

Hello Pankaj,

On 22.03.22 07:25, Pankaj Gupta wrote:
> Hi Ahmad,
> 
> Suggested to define macro with more details.
> Please find comments in-line.
> 

> len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + 
>>>>> + CAAM_PTR_SZ_MAX) + 4;
> 
>> +/* header + (key mod immediate) + 2x seq_intlen pointers + op */
>> +#define CAAM_BLOB_DESC_BYTES_MAX \
>> +       (CAAM_CMD_SZ + \
>> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \
>> +        2 * (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \
>> +        CAAM_CMD_SZ)
>> +
> 
> Suggested to replace the above macro like below:
> 
> +#define CAAM_BLOB_DESC_BYTES_MAX \			
> +       (CAAM_CMD_SZ + \					/* Command to initialize & stating length of  descriptor */
> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \	/* Command to append the key-modifier + followed by the key-modifier data */
> +        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include input plain key and pointer to the input key */
> +        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include output-key blob and pointer to the output-key blob */
> +        CAAM_CMD_SZ)						/* Command describing the Operation to perform */


Sure thing, will do for v7. Otherwise, if all looks good to you,
can I have your Reviewed-by?

Thanks,
Ahmad

Ahmad Fatoum March 22, 2022, 9:37 a.m. UTC | #3

Hello Pankaj,

On 22.03.22 08:32, Ahmad Fatoum wrote:
> Hello Pankaj,
> 
> On 22.03.22 07:25, Pankaj Gupta wrote:
>> Hi Ahmad,
>>
>> Suggested to define macro with more details.
>> Please find comments in-line.
>>
> 
>> len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + 
>>>>>> + CAAM_PTR_SZ_MAX) + 4;
>>
>>> +/* header + (key mod immediate) + 2x seq_intlen pointers + op */
>>> +#define CAAM_BLOB_DESC_BYTES_MAX \
>>> +       (CAAM_CMD_SZ + \
>>> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \
>>> +        2 * (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \
>>> +        CAAM_CMD_SZ)
>>> +
>>
>> Suggested to replace the above macro like below:
>>
>> +#define CAAM_BLOB_DESC_BYTES_MAX \			
>> +       (CAAM_CMD_SZ + \					/* Command to initialize & stating length of  descriptor */
>> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \	/* Command to append the key-modifier + followed by the key-modifier data */
>> +        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include input plain key and pointer to the input key */
>> +        (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \		/* Command to include output-key blob and pointer to the output-key blob */
>> +        CAAM_CMD_SZ)						/* Command describing the Operation to perform */
> 
> 
> Sure thing, will do for v7. Otherwise, if all looks good to you,
> can I have your Reviewed-by?
This doesn't compile as-is and it leads to quite long lines.
The description isn't accurate also, because what's plain and what's blob
changes depending on whether we encapsulate or decapsulate.

Here's my revised macro version:

#define CAAM_BLOB_DESC_BYTES_MAX                                        \
        /* Command to initialize & stating length of descriptor */      \
        (CAAM_CMD_SZ +                                                  \
        /* Command to append the key-modifier + key-modifier data */    \
         CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH +                        \
        /* Command to include input key + pointer to the input key */   \
         CAAM_CMD_SZ + CAAM_PTR_SZ_MAX +                                \
        /* Command to include output key + pointer to the output key */ \
         CAAM_CMD_SZ + CAAM_PTR_SZ_MAX +                                \
        /* Command describing the Operation to perform */               \
         CAAM_CMD_SZ)

Alternatively, I can change it back into a function:

static inline u32 *caam_blob_desc_alloc(void)
{
        size_t size = 0;

        /* Command to initialize & stating length of descriptor */
        size += CAAM_CMD_SZ;
        /* Command to append the key-modifier + key-modifier data */
        size += CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH;
        /* Command to include input plain key + pointer to the input key */
        size += CAAM_CMD_SZ + CAAM_PTR_SZ_MAX;
        /* Command to include output-key blob + pointer to the output key */
        size += CAAM_CMD_SZ + CAAM_PTR_SZ_MAX;
        /* Command describing the Operation to perform */
        size += CAAM_CMD_SZ;

        return kzalloc(size, GFP_KERNEL | GFP_DMA);
}

Let me know what works better for you.

Cheers,
Ahmad

> 
> Thanks,
> Ahmad
>

Pankaj Gupta March 24, 2022, 9:55 a.m. UTC | #4

Hi Ahmad,

Please find the comments in-line.

Regards
Pankaj

> -----Original Message-----
> From: Ahmad Fatoum <[e-mail]>
> Sent: Wednesday, March 16, 2022 10:14 PM
> To: Horia Geanta <[e-mail]>; Pankaj Gupta
> <[e-mail]>; Herbert Xu <[e-mail]>;
> David S. Miller <[e-mail]>
> Cc: [e-mail]; David Gstir <[e-mail]>;
> [e-mail]; Matthias Schiffer <[e-mail]-
> group.com>; Ahmad Fatoum <[e-mail]>; James Bottomley
> <[e-mail]>; Jarkko Sakkinen <[e-mail]>; Mimi Zohar
> <[e-mail]>; David Howells <[e-mail]>; James
> Morris <[e-mail]>; Eric Biggers <[e-mail]>; Serge E.
> Hallyn <[e-mail]>; Jan Luebbe <[e-mail]>;
> Richard Weinberger <[e-mail]>; Franck Lenormand
> <[e-mail]>; Sumit Garg <[e-mail]>; linux-
> [e-mail]; [e-mail]; linux-
> [e-mail]; [e-mail]; linux-security-
> [e-mail]
> Subject: [EXT] [PATCH v6 3/4] crypto: caam - add in-kernel interface for blob
> generator
> 
> Caution: EXT Email
> 
> The NXP Cryptographic Acceleration and Assurance Module (CAAM) can be
> used to protect user-defined data across system reboot:
> 
>   - When the system is fused and boots into secure state, the master
>     key is a unique never-disclosed device-specific key
>   - random key is encrypted by key derived from master key
>   - data is encrypted using the random key
>   - encrypted data and its encrypted random key are stored alongside
>   - This blob can now be safely stored in non-volatile memory
> 
> On next power-on:
>   - blob is loaded into CAAM
>   - CAAM writes decrypted data either into memory or key register
> 
> 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.
> 
> Reviewed-by: David Gstir <[e-mail]>
> Reviewed-by: Pankaj Gupta <[e-mail]>
> Tested-By: Tim Harvey <[e-mail]>
> Tested-by: Matthias Schiffer <[e-mail]>
> Signed-off-by: Steffen Trumtrar <[e-mail]>
> Signed-off-by: Ahmad Fatoum <[e-mail]>
> ---
> v5 -> v6:
>   - Dropped caam_blob_alloc_desc() in favor of kzalloc() with fixed size.
>     This simplifies code and wastes at most 12 bytes which are freed
>     at the end of the function anyway.
>   - Factored out common code between caam_encap_blob and
> caam_decap_blob

Suggest to continue to use two separate descriptor-creation-function for 'encap' and 'decap'.
This will help these API(s) to be maintained easily going forward. 

>     as both functions were largely identical
>   - use append_seq_(in|out)_ptr_intlen for both encap/decap as a result
>   - use reverse christmas tree order for caam_process_blob variable
>     definitions.
> v4 -> v5:
>   - Collected Reviewed-by's and Tested-by's
>   - Note in CAAM patch what CAAM is (Jarkko)
> v3 -> v4:
>   - Collected Acked-by's, Reviewed-by's and Tested-by
>   - Fixed typo spotted by David
> v2 -> v3:
>  - No change
> v1 -> v2:
>  - Enforce maximum keymod size (Horia)
>  - Use append_seq_(in|out)_ptr_intlen instead of append_seq_(in|out)_ptr
>    (Horia)
>  - Make blobifier handle private to CAAM glue code file (Horia)
> 
> To: "Horia Geantă" <[e-mail]>
> To: Pankaj Gupta <[e-mail]>
> To: Herbert Xu <[e-mail]>
> To: "David S. Miller" <[e-mail]>
> Cc: James Bottomley <[e-mail]>
> Cc: Jarkko Sakkinen <[e-mail]>
> Cc: Mimi Zohar <[e-mail]>
> Cc: David Howells <[e-mail]>
> Cc: James Morris <[e-mail]>
> Cc: Eric Biggers <[e-mail]>
> Cc: "Serge E. Hallyn" <[e-mail]>
> Cc: Jan Luebbe <[e-mail]>
> Cc: David Gstir <[e-mail]>
> Cc: Richard Weinberger <[e-mail]>
> Cc: Franck LENORMAND <[e-mail]>
> Cc: Matthias Schiffer <[e-mail]>
> Cc: Sumit Garg <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  drivers/crypto/caam/Kconfig    |   3 +
>  drivers/crypto/caam/Makefile   |   1 +
>  drivers/crypto/caam/blob_gen.c | 160
> +++++++++++++++++++++++++++++++++
>  include/soc/fsl/caam-blob.h    |  79 ++++++++++++++++
>  4 files changed, 243 insertions(+)
>  create mode 100644 drivers/crypto/caam/blob_gen.c  create mode 100644
> include/soc/fsl/caam-blob.h
> 
> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
> index 84ea7cba5ee5..ea9f8b1ae981 100644
> --- a/drivers/crypto/caam/Kconfig
> +++ b/drivers/crypto/caam/Kconfig
> @@ -151,6 +151,9 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API
>           Selecting this will register the SEC4 hardware rng to
>           the hw_random API for supplying the kernel entropy pool.
> 
> +config CRYPTO_DEV_FSL_CAAM_BLOB_GEN
> +       bool
> +
>  endif # CRYPTO_DEV_FSL_CAAM_JR
> 
>  endif # CRYPTO_DEV_FSL_CAAM
> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
> index 3570286eb9ce..25f7ae5a4642 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -21,6 +21,7 @@ caam_jr-
> $(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o
>  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caampkc.o
> pkc_desc.o
> +caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o
> 
>  caam-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += qi.o  ifneq
> ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),)
> diff --git a/drivers/crypto/caam/blob_gen.c
> b/drivers/crypto/caam/blob_gen.c new file mode 100644 index
> 000000000000..2564e63c8fe0
> --- /dev/null
> +++ b/drivers/crypto/caam/blob_gen.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2015 Pengutronix, Steffen Trumtrar
> +<[e-mail]>
> + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <[e-mail]>
> +*/
> +
> +#include <linux/device.h>
> +#include <soc/fsl/caam-blob.h>
> +
> +#include "compat.h"
> +#include "desc_constr.h"
> +#include "desc.h"
> +#include "error.h"
> +#include "intern.h"
> +#include "jr.h"
> +#include "regs.h"
> +
> +/* header + (key mod immediate) + 2x seq_intlen pointers + op */
> +#define CAAM_BLOB_DESC_BYTES_MAX \
> +       (CAAM_CMD_SZ + \
> +        CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \
> +        2 * (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \
> +        CAAM_CMD_SZ)
> +
> +struct caam_blob_priv {
> +       struct device jrdev;
> +};
> +
> +struct caam_blob_job_result {
> +       int err;
> +       struct completion completion;
> +};
> +
> +static void caam_blob_job_done(struct device *dev, u32 *desc, u32 err,
> +void *context) {
> +       struct caam_blob_job_result *res = context;
> +       int ecode = 0;
> +
> +       dev_dbg(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
> +
> +       if (err)
> +               ecode = caam_jr_strstatus(dev, err);
> +
> +       res->err = ecode;
> +
> +       /*
> +        * Upon completion, desc points to a buffer containing a CAAM job
> +        * descriptor which encapsulates data into an externally-storable
> +        * blob.
> +        */
> +       complete(&res->completion);
> +}
> +
> +int caam_process_blob(struct caam_blob_priv *priv, bool encap,
> +                     const char *keymod, void *input, void *output,
> +                     size_t length)
> +{
> +       size_t in_len = length, out_len = length;
> +       struct caam_blob_job_result testres;
> +       struct device *jrdev = &priv->jrdev;
> +       dma_addr_t dma_in, dma_out;
> +       int op = OP_PCLID_BLOB;
> +       size_t keymod_len;
> +       u32 *desc;
> +       int ret;
> +
> +       keymod_len = strlen(keymod);
> +
> +       if (length <= CAAM_BLOB_OVERHEAD || keymod_len >
> CAAM_BLOB_KEYMOD_LENGTH)
> +               return -EINVAL;
> +
> +       if (encap) {
> +               op |= OP_TYPE_ENCAP_PROTOCOL;
> +               in_len -= CAAM_BLOB_OVERHEAD;
> +       } else {
> +               op |= OP_TYPE_DECAP_PROTOCOL;
> +               out_len -= CAAM_BLOB_OVERHEAD;
> +       }
> +
> +       desc = kzalloc(CAAM_BLOB_DESC_BYTES_MAX, GFP_KERNEL |
> GFP_DMA);
> +       if (!desc) {
> +               dev_err(jrdev, "unable to allocate desc\n");
> +               return -ENOMEM;
> +       }
> +
> +       dma_in = dma_map_single(jrdev, input, in_len, DMA_TO_DEVICE);
> +       if (dma_mapping_error(jrdev, dma_in)) {
> +               dev_err(jrdev, "unable to map input DMA buffer\n");
> +               ret = -ENOMEM;
> +               goto out_free;
> +       }
> +
> +       dma_out = dma_map_single(jrdev, output, out_len,
> DMA_FROM_DEVICE);
> +       if (dma_mapping_error(jrdev, dma_out)) {
> +               dev_err(jrdev, "unable to map output DMA buffer\n");
> +               ret = -ENOMEM;
> +               goto out_unmap_in;
> +       }
> +
> +       /*
> +        * A data blob is encrypted using a blob key (BK); a random number.
> +        * The BK is used as an AES-CCM key. The initial block (B0) and the
> +        * initial counter (Ctr0) are generated automatically and stored in
> +        * Class 1 Context DWords 0+1+2+3. The random BK is stored in the
> +        * Class 1 Key Register. Operation Mode is set to AES-CCM.
> +        */
> +
> +       init_job_desc(desc, 0);
> +       append_key_as_imm(desc, keymod, keymod_len, keymod_len,
> +                         CLASS_2 | KEY_DEST_CLASS_REG);
> +       append_seq_in_ptr_intlen(desc, dma_in, in_len, 0);
> +       append_seq_out_ptr_intlen(desc, dma_out, out_len, 0);
> +       append_operation(desc, op);
> +
> +       print_hex_dump_debug("data@"__stringify(__LINE__)": ",
> +                            DUMP_PREFIX_ADDRESS, 16, 1, input,
> +                            in_len, false);
> +       print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
> +                            DUMP_PREFIX_ADDRESS, 16, 1, desc,
> +                            desc_bytes(desc), false);
> +
> +       testres.err = 0;
> +       init_completion(&testres.completion);
> +
> +       ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, &testres);
> +       if (ret == -EINPROGRESS) {
> +               wait_for_completion(&testres.completion);
> +               ret = testres.err;
> +               print_hex_dump_debug("output@"__stringify(__LINE__)": ",
> +                                    DUMP_PREFIX_ADDRESS, 16, 1, output,
> +                                    out_len, false);
> +       }
> +
> +       dma_unmap_single(jrdev, dma_out, out_len, DMA_FROM_DEVICE);
> +out_unmap_in:
> +       dma_unmap_single(jrdev, dma_in, in_len, DMA_TO_DEVICE);
> +out_free:
> +       kfree(desc);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL(caam_process_blob);
> +
> +struct caam_blob_priv *caam_blob_gen_init(void) {
> +       struct device *jrdev;
> +
> +       jrdev = caam_jr_alloc();
> +       if (IS_ERR(jrdev))
> +               return ERR_CAST(jrdev);
> +
> +       return container_of(jrdev, struct caam_blob_priv, jrdev); }
> +EXPORT_SYMBOL(caam_blob_gen_init);
> +
> +void caam_blob_gen_exit(struct caam_blob_priv *priv) {
> +       caam_jr_free(&priv->jrdev);
> +}
> +EXPORT_SYMBOL(caam_blob_gen_exit);
> diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h new
> file mode 100644 index 000000000000..2c4a05dc59cd
> --- /dev/null
> +++ b/include/soc/fsl/caam-blob.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <[e-mail]>
> +*/
> +
> +#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_exit - free blob generation resources
> + *
> + * @priv: instance returned by caam_blob_gen_init  */ void
> +caam_blob_gen_exit(struct caam_blob_priv *priv);
> +
> +/** caam_process_blob - encapsulate or decapsulate blob
> + *
> + * @priv:   instance returned by caam_blob_gen_init
> + * @encap:  true for encapsulation, false for decapsulation
> + * @keymod: string to use as key modifier for blob capsulation
> + *         can't be longer than CAAM_BLOB_KEYMOD_LENGTH
> + * @input:  buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
> + * @length: buffer length including blob overhead
> + *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
> + */
> +int caam_process_blob(struct caam_blob_priv *priv, bool encap,
> +                     const char *keymod, void *input, void *output,
> +                     size_t length);
> +
> +/** caam_encap_blob - encapsulate blob
> + *
> + * @priv:   instance returned by caam_blob_gen_init
> + * @keymod: string to use as key modifier for blob encapsulation
> + *         can't be longer than CAAM_BLOB_KEYMOD_LENGTH
> + * @input:  buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
> + * @length: buffer length including blob overhead
> + *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
> + */
> +static inline int caam_encap_blob(struct caam_blob_priv *priv,
> +                                 const char *keymod,
> +                                 void *input, void *output, size_t
> +length) {
> +       return caam_process_blob(priv, true, keymod, input, output,
> +length); }
> +

In continuation to the previous comment, there is another suggestion:

Either: 
struct keyblob_info {
        void *key;
        size_t key_len;

        void *blob;
        size_t blob_len;

        size_t key_mod_len;
        const void *key_mod;
};

Or:

struct keyblob_info {        
        void *input;
        void *input_len;

        void *output;
        void *out_len;

        size_t key_mod_len;
        const void *key_mod;
};


int caam_blob_encap(struct caam_blob_priv *priv, struct keyblob_info *info);

int caam_blob_decap(struct caam_blob_priv *priv, struct keyblob_info *info);

> +/** caam_decap_blob - decapsulate blob
> + *
> + * @priv:   instance returned by caam_blob_gen_init
> + * @keymod: string to use as key modifier for blob decapsulation
> + *         can't be longer than CAAM_BLOB_KEYMOD_LENGTH
> + * @input:  buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
> + * @length: buffer length including blob overhead
> + *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
> + */
> +static inline int caam_decap_blob(struct caam_blob_priv *priv,
> +                                 const char *keymod,
> +                                 void *input, void *output, size_t
> +length) {
> +       return caam_process_blob(priv, false, keymod, input, output,
> +length); }
> +
> +#endif
> --
> 2.30.2

Ahmad Fatoum March 24, 2022, 10:10 a.m. UTC | #5

Hello Pankaj,

On 24.03.22 10:55, Pankaj Gupta wrote:
> Hi Ahmad,
> 
> Please find the comments in-line.

Thanks for you review.

> Suggest to continue to use two separate descriptor-creation-function for 'encap' and 'decap'.
> This will help these API(s) to be maintained easily going forward. 

We can still split them up in future once there is a real need.
But currently they are exactly the same, except for input/output length,
so I think it's correct to not introduce duplication unless needed.

>>   - use append_seq_(in|out)_ptr_intlen for both encap/decap as a result

Case in point. The intlen omission was because the two functions are largely
identical and I only fixed up one of them. This is prone to repeat when
we go back to have identical code with minor differences.

> In continuation to the previous comment, there is another suggestion:
> 
> Either: 
> struct keyblob_info {
>         void *key;
>         size_t key_len;
> 
>         void *blob;
>         size_t blob_len;
> 
>         size_t key_mod_len;
>         const void *key_mod;
> };

I can do that.

Whats your opinion on the desc size computation? Comment the macro or add
the static inline helper?

Cheers,
Ahmad

Pankaj Gupta March 28, 2022, 9:29 a.m. UTC | #6

Hi Ahmad,

Please find the comments in-line.

Regards
Pankaj

> -----Original Message-----
> From: Ahmad Fatoum <[e-mail]>
> Sent: Thursday, March 24, 2022 3:40 PM
> To: Pankaj Gupta <[e-mail]>; Horia Geanta
> <[e-mail]>; Herbert Xu <[e-mail]>;
> David S. Miller <[e-mail]>
> Cc: [e-mail]; Eric Biggers
> <[e-mail]>; David Gstir <[e-mail]>; Matthias
> Schiffer <[e-mail]>; Sumit Garg
> <[e-mail]>; Jan Luebbe <[e-mail]>; Richard
> Weinberger <[e-mail]>; [e-mail]; Franck Lenormand
> <[e-mail]>; James Morris <[e-mail]>; Mimi
> Zohar <[e-mail]>; [e-mail]; David Howells
> <[e-mail]>; Jarkko Sakkinen <[e-mail]>;
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; James Bottomley
> <[e-mail]>; Serge E. Hallyn <[e-mail]>
> Subject: Re: [EXT] [PATCH v6 3/4] crypto: caam - add in-kernel interface for
> blob generator
> 
> Caution: EXT Email
> 
> Hello Pankaj,
> 
> On 24.03.22 10:55, Pankaj Gupta wrote:
> > Hi Ahmad,
> >
> > Please find the comments in-line.
> 
> Thanks for you review.
> 
> > Suggest to continue to use two separate descriptor-creation-function for
> 'encap' and 'decap'.
> > This will help these API(s) to be maintained easily going forward.
> 
> We can still split them up in future once there is a real need.
> But currently they are exactly the same, except for input/output length, so I
> think it's correct to not introduce duplication unless needed.
> 
> >>   - use append_seq_(in|out)_ptr_intlen for both encap/decap as a
> >> result
> 
> Case in point. The intlen omission was because the two functions are largely
> identical and I only fixed up one of them. This is prone to repeat when we go
> back to have identical code with minor differences.
> 
> > In continuation to the previous comment, there is another suggestion:
> >
> > Either:
> > struct keyblob_info {
> >         void *key;
> >         size_t key_len;
> >
> >         void *blob;
> >         size_t blob_len;
> >
> >         size_t key_mod_len;
> >         const void *key_mod;
> > };
> 
> I can do that.
> 

Please do. Thanks.
Patch 4/4, needs to be re-worked to.

> Whats your opinion on the desc size computation? Comment the macro or
> add the static inline helper?
> 

Comment the macro is fine.

> Cheers,
> Ahmad
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&amp;data=04%7C01%7Cpankaj.gupta%40nxp.com%7C4
> d60f0d524a04b7cbd7b08da0d7e7d21%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C0%7C637837134158793951%7CUnknown%7CTWFpbGZsb3d8eyJWI
> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000&amp;sdata=PetvZm8teusBwQ4BeZ1VLEOvBlCrZ2k2bNG3SJBEXPw%3D&
> amp;reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |

Ahmad Fatoum April 15, 2022, 8:07 p.m. UTC | #7

On 28.03.22 11:29, Pankaj Gupta wrote:
>>> struct keyblob_info {
>>>         void *key;
>>>         size_t key_len;
>>>
>>>         void *blob;
>>>         size_t blob_len;
>>>
>>>         size_t key_mod_len;
>>>         const void *key_mod;
>>> };
>>
>> I can do that.
>>
> 
> Please do. Thanks.

I went with your other suggestion instead: void *input and void *output.
This keeps the code simple and avoids having to do:

if (encap) {
  in = info->key;
  in_len = info->key_len;
  out = info->blob_len;
  out_len = info->key_len + CAAM_BLOB_OVERHEAD; 
} else {
  in = info->blob;
  in_len = info->blob_len;
  out = info->key_len;
  out_len = info->blob_len - CAAM_BLOB_OVERHEAD; 
}

> Patch 4/4, needs to be re-worked to.
> 
>> Whats your opinion on the desc size computation? Comment the macro or
>> add the static inline helper?
>>
> 
> Comment the macro is fine.
> 
>> Cheers,
>> Ahmad
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       |
>> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
>> pengutronix.de%2F&data=04%7C01%7Cpankaj.gupta%40nxp.com%7C4
>> d60f0d524a04b7cbd7b08da0d7e7d21%7C686ea1d3bc2b4c6fa92cd99c5c30163
>> 5%7C0%7C0%7C637837134158793951%7CUnknown%7CTWFpbGZsb3d8eyJWI
>> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
>> 000&sdata=PetvZm8teusBwQ4BeZ1VLEOvBlCrZ2k2bNG3SJBEXPw%3D&
>> amp;reserved=0  |
>> 31137 Hildesheim, Germany                  | Phone: [phone]    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |
> 
>

Patch

diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
index 84ea7cba5ee5..ea9f8b1ae981 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -151,6 +151,9 @@  config CRYPTO_DEV_FSL_CAAM_RNG_API
 	  Selecting this will register the SEC4 hardware rng to
 	  the hw_random API for supplying the kernel entropy pool.
 
+config CRYPTO_DEV_FSL_CAAM_BLOB_GEN
+	bool
+
 endif # CRYPTO_DEV_FSL_CAAM_JR
 
 endif # CRYPTO_DEV_FSL_CAAM
diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
index 3570286eb9ce..25f7ae5a4642 100644
--- a/drivers/crypto/caam/Makefile
+++ b/drivers/crypto/caam/Makefile
@@ -21,6 +21,7 @@  caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o
 caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
 caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o
 caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caampkc.o pkc_desc.o
+caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o
 
 caam-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += qi.o
 ifneq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),)
diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
new file mode 100644
index 000000000000..2564e63c8fe0
--- /dev/null
+++ b/drivers/crypto/caam/blob_gen.c
@@ -0,0 +1,160 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2015 Pengutronix, Steffen Trumtrar <[e-mail]>
+ * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <[e-mail]>
+ */
+
+#include <linux/device.h>
+#include <soc/fsl/caam-blob.h>
+
+#include "compat.h"
+#include "desc_constr.h"
+#include "desc.h"
+#include "error.h"
+#include "intern.h"
+#include "jr.h"
+#include "regs.h"
+
+/* header + (key mod immediate) + 2x seq_intlen pointers + op */
+#define CAAM_BLOB_DESC_BYTES_MAX \
+	(CAAM_CMD_SZ + \
+	 CAAM_CMD_SZ + CAAM_BLOB_KEYMOD_LENGTH + \
+	 2 * (CAAM_CMD_SZ + CAAM_PTR_SZ_MAX) + \
+	 CAAM_CMD_SZ)
+
+struct caam_blob_priv {
+	struct device jrdev;
+};
+
+struct caam_blob_job_result {
+	int err;
+	struct completion completion;
+};
+
+static void caam_blob_job_done(struct device *dev, u32 *desc, u32 err, void *context)
+{
+	struct caam_blob_job_result *res = context;
+	int ecode = 0;
+
+	dev_dbg(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
+
+	if (err)
+		ecode = caam_jr_strstatus(dev, err);
+
+	res->err = ecode;
+
+	/*
+	 * Upon completion, desc points to a buffer containing a CAAM job
+	 * descriptor which encapsulates data into an externally-storable
+	 * blob.
+	 */
+	complete(&res->completion);
+}
+
+int caam_process_blob(struct caam_blob_priv *priv, bool encap,
+		      const char *keymod, void *input, void *output,
+		      size_t length)
+{
+	size_t in_len = length, out_len = length;
+	struct caam_blob_job_result testres;
+	struct device *jrdev = &priv->jrdev;
+	dma_addr_t dma_in, dma_out;
+	int op = OP_PCLID_BLOB;
+	size_t keymod_len;
+	u32 *desc;
+	int ret;
+
+	keymod_len = strlen(keymod);
+
+	if (length <= CAAM_BLOB_OVERHEAD || keymod_len > CAAM_BLOB_KEYMOD_LENGTH)
+		return -EINVAL;
+
+	if (encap) {
+		op |= OP_TYPE_ENCAP_PROTOCOL;
+		in_len -= CAAM_BLOB_OVERHEAD;
+	} else {
+		op |= OP_TYPE_DECAP_PROTOCOL;
+		out_len -= CAAM_BLOB_OVERHEAD;
+	}
+
+	desc = kzalloc(CAAM_BLOB_DESC_BYTES_MAX, GFP_KERNEL | GFP_DMA);
+	if (!desc) {
+		dev_err(jrdev, "unable to allocate desc\n");
+		return -ENOMEM;
+	}
+
+	dma_in = dma_map_single(jrdev, input, in_len, DMA_TO_DEVICE);
+	if (dma_mapping_error(jrdev, dma_in)) {
+		dev_err(jrdev, "unable to map input DMA buffer\n");
+		ret = -ENOMEM;
+		goto out_free;
+	}
+
+	dma_out = dma_map_single(jrdev, output, out_len, DMA_FROM_DEVICE);
+	if (dma_mapping_error(jrdev, dma_out)) {
+		dev_err(jrdev, "unable to map output DMA buffer\n");
+		ret = -ENOMEM;
+		goto out_unmap_in;
+	}
+
+	/*
+	 * A data blob is encrypted using a blob key (BK); a random number.
+	 * The BK is used as an AES-CCM key. The initial block (B0) and the
+	 * initial counter (Ctr0) are generated automatically and stored in
+	 * Class 1 Context DWords 0+1+2+3. The random BK is stored in the
+	 * Class 1 Key Register. Operation Mode is set to AES-CCM.
+	 */
+
+	init_job_desc(desc, 0);
+	append_key_as_imm(desc, keymod, keymod_len, keymod_len,
+			  CLASS_2 | KEY_DEST_CLASS_REG);
+	append_seq_in_ptr_intlen(desc, dma_in, in_len, 0);
+	append_seq_out_ptr_intlen(desc, dma_out, out_len, 0);
+	append_operation(desc, op);
+
+	print_hex_dump_debug("data@"__stringify(__LINE__)": ",
+			     DUMP_PREFIX_ADDRESS, 16, 1, input,
+			     in_len, false);
+	print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
+			     DUMP_PREFIX_ADDRESS, 16, 1, desc,
+			     desc_bytes(desc), false);
+
+	testres.err = 0;
+	init_completion(&testres.completion);
+
+	ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, &testres);
+	if (ret == -EINPROGRESS) {
+		wait_for_completion(&testres.completion);
+		ret = testres.err;
+		print_hex_dump_debug("output@"__stringify(__LINE__)": ",
+				     DUMP_PREFIX_ADDRESS, 16, 1, output,
+				     out_len, false);
+	}
+
+	dma_unmap_single(jrdev, dma_out, out_len, DMA_FROM_DEVICE);
+out_unmap_in:
+	dma_unmap_single(jrdev, dma_in, in_len, DMA_TO_DEVICE);
+out_free:
+	kfree(desc);
+
+	return ret;
+}
+EXPORT_SYMBOL(caam_process_blob);
+
+struct caam_blob_priv *caam_blob_gen_init(void)
+{
+	struct device *jrdev;
+
+	jrdev = caam_jr_alloc();
+	if (IS_ERR(jrdev))
+		return ERR_CAST(jrdev);
+
+	return container_of(jrdev, struct caam_blob_priv, jrdev);
+}
+EXPORT_SYMBOL(caam_blob_gen_init);
+
+void caam_blob_gen_exit(struct caam_blob_priv *priv)
+{
+	caam_jr_free(&priv->jrdev);
+}
+EXPORT_SYMBOL(caam_blob_gen_exit);
diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
new file mode 100644
index 000000000000..2c4a05dc59cd
--- /dev/null
+++ b/include/soc/fsl/caam-blob.h
@@ -0,0 +1,79 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <[e-mail]>
+ */
+
+#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_exit - free blob generation resources
+ *
+ * @priv: instance returned by caam_blob_gen_init
+ */
+void caam_blob_gen_exit(struct caam_blob_priv *priv);
+
+/** caam_process_blob - encapsulate or decapsulate blob
+ *
+ * @priv:   instance returned by caam_blob_gen_init
+ * @encap:  true for encapsulation, false for decapsulation
+ * @keymod: string to use as key modifier for blob capsulation
+ *	    can't be longer than CAAM_BLOB_KEYMOD_LENGTH
+ * @input:  buffer which CAAM will DMA from
+ * @output: buffer which CAAM will DMA to
+ * @length: buffer length including blob overhead
+ *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
+ */
+int caam_process_blob(struct caam_blob_priv *priv, bool encap,
+		      const char *keymod, void *input, void *output,
+		      size_t length);
+
+/** caam_encap_blob - encapsulate blob
+ *
+ * @priv:   instance returned by caam_blob_gen_init
+ * @keymod: string to use as key modifier for blob encapsulation
+ *	    can't be longer than CAAM_BLOB_KEYMOD_LENGTH
+ * @input:  buffer which CAAM will DMA from
+ * @output: buffer which CAAM will DMA to
+ * @length: buffer length including blob overhead
+ *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
+ */
+static inline int caam_encap_blob(struct caam_blob_priv *priv,
+				  const char *keymod,
+				  void *input, void *output, size_t length)
+{
+	return caam_process_blob(priv, true, keymod, input, output, length);
+}
+
+/** caam_decap_blob - decapsulate blob
+ *
+ * @priv:   instance returned by caam_blob_gen_init
+ * @keymod: string to use as key modifier for blob decapsulation
+ *	    can't be longer than CAAM_BLOB_KEYMOD_LENGTH
+ * @input:  buffer which CAAM will DMA from
+ * @output: buffer which CAAM will DMA to
+ * @length: buffer length including blob overhead
+ *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
+ */
+static inline int caam_decap_blob(struct caam_blob_priv *priv,
+				  const char *keymod,
+				  void *input, void *output, size_t length)
+{
+	return caam_process_blob(priv, false, keymod, input, output, length);
+}
+
+#endif