Re: [PATCH v1 3/3] crypto: cavium - Register the CNN55XX supported crypto algorithms.
Hi Stephan,
On Thursday 11 May 2017 05:52 PM, Stephan Müller wrote:
> Am Donnerstag, 11. Mai 2017, 14:18:34 CEST schrieb srikanth jampala:
>
> Hi srikanth,
>
>> Hi Stephan,
>>
>> On Wednesday 10 May 2017 07:26 PM, Stephan Müller wrote:
>>> Am Mittwoch, 10. Mai 2017, 15:06:40 CEST schrieb Srikanth Jampala:
>>>
>>> Hi Srikanth,
>>>
>>> In general: you are using the ablkcipher API. I think it is on its way out
>>> and being replaced with skcipher.
>>>
>>> Maybe it makes sense to replace it here too. It could be as simple as s/
>>> ablkcipher/skcipher/g
>>
>> Sure, I will do the changes accordingly.
>> As per my understanding, I see the following changes,
>> 1. CRYPTO_ALG_TYPE_ABLKCIPHER changed to CRYPTO_ALG_TYPE_SKCIPHER
>> 2. nitrox_ablkcipher_foo() changed to nitrox_skcipher_foo()
>>
>> Please let me know, any other changes I have to consider?
>
> What about all crypto_ablkcipher* function calls / structs? Also all
> ablkcipher_request function calls/structs?
I will work on the changes suggested.
>>
+static inline int nitrox_ablkcipher_setkey(struct crypto_ablkcipher
*cipher,
+ int aes_keylen, const u8 *key,
+ unsigned int keylen)
+{
+ struct crypto_tfm *tfm = crypto_ablkcipher_tfm(cipher);
+ struct nitrox_crypto_instance *inst = crypto_tfm_ctx(tfm);
+ struct flexi_crypto_context *fctx;
+ enum flexi_cipher cipher_type;
+ const char *name;
+
+ name = crypto_tfm_alg_name(tfm);
+ cipher_type = flexi_cipher_type(name);
+ if (cipher_type == CIPHER_INVALID) {
+ pr_err("unsupported cipher: %s\n", name);
+ return -EINVAL;
+ }
+
+ /* fill crypto context */
+ fctx = inst->u.fctx;
+ fctx->flags = 0;
+ fctx->w0.cipher_type = cipher_type;
+ fctx->w0.aes_keylen = aes_keylen;
+ fctx->w0.iv_source = IV_FROM_DPTR;
+ fctx->flags = cpu_to_be64(*(u64 *)>w0);
+ /* copy the key to context */
+ memcpy(fctx->crypto.u.key, key, keylen);
>>>
>>> Could you help me finding the location where this memory is zeroized upon
>>> release?
>>
>> Currently, we are not zeroized the context in release.
>> We are doing it at the time of allocation.
>
> You store a key in a piece of heap memory. I guess you want to zeroize that
> key memory before releasing?
>
I will do the changes.
>
> Ciao
> Stephan
>
Re: [PATCH v1 3/3] crypto: cavium - Register the CNN55XX supported crypto algorithms.
Am Donnerstag, 11. Mai 2017, 14:18:34 CEST schrieb srikanth jampala:
Hi srikanth,
> Hi Stephan,
>
> On Wednesday 10 May 2017 07:26 PM, Stephan Müller wrote:
> > Am Mittwoch, 10. Mai 2017, 15:06:40 CEST schrieb Srikanth Jampala:
> >
> > Hi Srikanth,
> >
> > In general: you are using the ablkcipher API. I think it is on its way out
> > and being replaced with skcipher.
> >
> > Maybe it makes sense to replace it here too. It could be as simple as s/
> > ablkcipher/skcipher/g
>
> Sure, I will do the changes accordingly.
> As per my understanding, I see the following changes,
> 1. CRYPTO_ALG_TYPE_ABLKCIPHER changed to CRYPTO_ALG_TYPE_SKCIPHER
> 2. nitrox_ablkcipher_foo() changed to nitrox_skcipher_foo()
>
> Please let me know, any other changes I have to consider?
What about all crypto_ablkcipher* function calls / structs? Also all
ablkcipher_request function calls/structs?
>
> >> +static inline int nitrox_ablkcipher_setkey(struct crypto_ablkcipher
> >> *cipher,
> >> + int aes_keylen, const u8 *key,
> >> + unsigned int keylen)
> >> +{
> >> + struct crypto_tfm *tfm = crypto_ablkcipher_tfm(cipher);
> >> + struct nitrox_crypto_instance *inst = crypto_tfm_ctx(tfm);
> >> + struct flexi_crypto_context *fctx;
> >> + enum flexi_cipher cipher_type;
> >> + const char *name;
> >> +
> >> + name = crypto_tfm_alg_name(tfm);
> >> + cipher_type = flexi_cipher_type(name);
> >> + if (cipher_type == CIPHER_INVALID) {
> >> + pr_err("unsupported cipher: %s\n", name);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + /* fill crypto context */
> >> + fctx = inst->u.fctx;
> >> + fctx->flags = 0;
> >> + fctx->w0.cipher_type = cipher_type;
> >> + fctx->w0.aes_keylen = aes_keylen;
> >> + fctx->w0.iv_source = IV_FROM_DPTR;
> >> + fctx->flags = cpu_to_be64(*(u64 *)>w0);
> >> + /* copy the key to context */
> >> + memcpy(fctx->crypto.u.key, key, keylen);
> >
> > Could you help me finding the location where this memory is zeroized upon
> > release?
>
> Currently, we are not zeroized the context in release.
> We are doing it at the time of allocation.
You store a key in a piece of heap memory. I guess you want to zeroize that
key memory before releasing?
Ciao
Stephan
Re: [PATCH v1 3/3] crypto: cavium - Register the CNN55XX supported crypto algorithms.
Hi Stephan,
On Wednesday 10 May 2017 07:26 PM, Stephan Müller wrote:
> Am Mittwoch, 10. Mai 2017, 15:06:40 CEST schrieb Srikanth Jampala:
>
> Hi Srikanth,
>
> In general: you are using the ablkcipher API. I think it is on its way out
> and
> being replaced with skcipher.
>
> Maybe it makes sense to replace it here too. It could be as simple as s/
> ablkcipher/skcipher/g
>
Sure, I will do the changes accordingly.
As per my understanding, I see the following changes,
1. CRYPTO_ALG_TYPE_ABLKCIPHER changed to CRYPTO_ALG_TYPE_SKCIPHER
2. nitrox_ablkcipher_foo() changed to nitrox_skcipher_foo()
Please let me know, any other changes I have to consider?
>> +static inline int nitrox_ablkcipher_setkey(struct crypto_ablkcipher
>> *cipher,
>> + int aes_keylen, const u8 *key,
>> + unsigned int keylen)
>> +{
>> + struct crypto_tfm *tfm = crypto_ablkcipher_tfm(cipher);
>> + struct nitrox_crypto_instance *inst = crypto_tfm_ctx(tfm);
>> + struct flexi_crypto_context *fctx;
>> + enum flexi_cipher cipher_type;
>> + const char *name;
>> +
>> + name = crypto_tfm_alg_name(tfm);
>> + cipher_type = flexi_cipher_type(name);
>> + if (cipher_type == CIPHER_INVALID) {
>> + pr_err("unsupported cipher: %s\n", name);
>> + return -EINVAL;
>> + }
>> +
>> + /* fill crypto context */
>> + fctx = inst->u.fctx;
>> + fctx->flags = 0;
>> + fctx->w0.cipher_type = cipher_type;
>> + fctx->w0.aes_keylen = aes_keylen;
>> + fctx->w0.iv_source = IV_FROM_DPTR;
>> + fctx->flags = cpu_to_be64(*(u64 *)>w0);
>> + /* copy the key to context */
>> + memcpy(fctx->crypto.u.key, key, keylen);
>
> Could you help me finding the location where this memory is zeroized upon
> release?
Currently, we are not zeroized the context in release.
We are doing it at the time of allocation.
+void *crypto_alloc_context(struct nitrox_device *ndev)
+{
+ struct ctx_hdr *ctx;
+ void *vaddr;
+ dma_addr_t dma;
+
+ vaddr = dma_pool_alloc(ndev->ctx_pool, (GFP_ATOMIC | __GFP_ZERO), );
+ if (!vaddr)
+ return NULL;
+
+ /* fill meta data */
+ ctx = vaddr;
+ ctx->pool = ndev->ctx_pool;
+ ctx->dma = dma;
+ ctx->ctx_dma = dma + sizeof(struct ctx_hdr);
+
+ return ((u8 *)vaddr + sizeof(struct ctx_hdr));
+}
>> +
>> + return 0;
>> +}
Re: [PATCH v1 3/3] crypto: cavium - Register the CNN55XX supported crypto algorithms.
Hi Srikanth,
[auto build test WARNING on cryptodev/master]
[also build test WARNING on next-20170510]
[cannot apply to v4.11]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Srikanth-Jampala/crypto-cavium-Add-support-for-CNN55XX-adapters/20170510-211638
base:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from drivers/crypto/cavium/nitrox/nitrox_algs.c:12:0:
drivers/crypto/cavium/nitrox/nitrox_dev.h: In function 'nitrox_read_csr':
drivers/crypto/cavium/nitrox/nitrox_dev.h:159:9: error: implicit declaration
of function 'readq' [-Werror=implicit-function-declaration]
return readq(ndev->bar_addr + offset);
^
drivers/crypto/cavium/nitrox/nitrox_dev.h: In function 'nitrox_write_csr':
drivers/crypto/cavium/nitrox/nitrox_dev.h:171:2: error: implicit declaration
of function 'writeq' [-Werror=implicit-function-declaration]
writeq(value, (ndev->bar_addr + offset));
^~
drivers/crypto/cavium/nitrox/nitrox_algs.c: In function
'nitrox_ablkcipher_exit':
>> drivers/crypto/cavium/nitrox/nitrox_algs.c:117:23: warning: cast to pointer
>> from integer of different size [-Wint-to-pointer-cast]
crypto_free_context((void *)inst->u.ctx_handle);
^
cc1: some warnings being treated as errors
vim +117 drivers/crypto/cavium/nitrox/nitrox_algs.c
101 ctx = crypto_alloc_context(inst->ndev);
102 if (!ctx) {
103 nitrox_put_device(inst->ndev);
104 return -ENOMEM;
105 }
106 inst->u.ctx_handle = (uintptr_t)ctx;
107
108 return 0;
109 }
110
111 static void nitrox_ablkcipher_exit(struct crypto_tfm *tfm)
112 {
113 struct nitrox_crypto_instance *inst = crypto_tfm_ctx(tfm);
114
115 /* free the crypto context */
116 if (inst->u.ctx_handle)
> 117 crypto_free_context((void *)inst->u.ctx_handle);
118
119 nitrox_put_device(inst->ndev);
120
121 inst->u.ctx_handle = 0;
122 inst->ndev = NULL;
123 }
124
125 static inline int nitrox_ablkcipher_setkey(struct crypto_ablkcipher
*cipher,
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH v1 3/3] crypto: cavium - Register the CNN55XX supported crypto algorithms.
Am Mittwoch, 10. Mai 2017, 15:06:40 CEST schrieb Srikanth Jampala:
Hi Srikanth,
In general: you are using the ablkcipher API. I think it is on its way out and
being replaced with skcipher.
Maybe it makes sense to replace it here too. It could be as simple as s/
ablkcipher/skcipher/g
> +static inline int nitrox_ablkcipher_setkey(struct crypto_ablkcipher
> *cipher,
> + int aes_keylen, const u8 *key,
> + unsigned int keylen)
> +{
> + struct crypto_tfm *tfm = crypto_ablkcipher_tfm(cipher);
> + struct nitrox_crypto_instance *inst = crypto_tfm_ctx(tfm);
> + struct flexi_crypto_context *fctx;
> + enum flexi_cipher cipher_type;
> + const char *name;
> +
> + name = crypto_tfm_alg_name(tfm);
> + cipher_type = flexi_cipher_type(name);
> + if (cipher_type == CIPHER_INVALID) {
> + pr_err("unsupported cipher: %s\n", name);
> + return -EINVAL;
> + }
> +
> + /* fill crypto context */
> + fctx = inst->u.fctx;
> + fctx->flags = 0;
> + fctx->w0.cipher_type = cipher_type;
> + fctx->w0.aes_keylen = aes_keylen;
> + fctx->w0.iv_source = IV_FROM_DPTR;
> + fctx->flags = cpu_to_be64(*(u64 *)>w0);
> + /* copy the key to context */
> + memcpy(fctx->crypto.u.key, key, keylen);
Could you help me finding the location where this memory is zeroized upon
release?
> +
> + return 0;
> +}
> +
> +static int nitrox_3des_setkey(struct crypto_ablkcipher *cipher, const u8
> *key, + unsigned int keylen)
> +{
> + if (keylen != DES3_EDE_KEY_SIZE) {
> + crypto_ablkcipher_set_flags(cipher, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return -EINVAL;
> + }
Please import the check from __des3_ede_setkey (or better, extract that key
check into a generic function like xts_check_key).
Ciao
Stephan
