Re: Crash in crypto mcryptd

2016-12-01 Thread Eric Biggers 
On Thu, Dec 01, 2016 at 05:47:02PM -0800, Tim Chen wrote:
> On Thu, 2016-12-01 at 19:00 -0500, Mikulas Patocka wrote:
> > Hi
> > 
> > There is a bug in mcryptd initialization.
> > 
> > This is a test module that tries various hash algorithms. When you load 
> > the module with "insmod test.ko 'alg=mcryptd(md5)'", the machine crashes.
> 
> I don't think your test setup is right.  The mcryptd supports only 
> multi-buffer
> algorithm.  I don't think there is such an implementation for md5.
> 
> Please refer to arch/x86/crypto/sha1-mb 
> multi-buffer implementation of sha1 to see the proper
> setup and usage with mcryptd.  You can also run tcrypt test to
> exercise this code.
> 
> Tim

No, mcryptd must not crash the kernel if it's passed the wrong algorithm.
Users can try to instantiate it with any algorithm using AF_ALG, for example:

struct sockaddr_alg addr = {
.salg_type = "hash",
.salg_name = "mcryptd(md5)",
};

int fd = socket(AF_ALG, SOCK_SEQPACKET, 0);

bind(fd, (struct sockaddr *), sizeof(addr));

Currently, this instantly crashes the kernel.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] crypto: chcr - checking for IS_ERR() instead of NULL

2016-12-01 Thread Harsh Jain 


On 02-12-2016 02:19, Dan Carpenter wrote:
> The create_hash_wr() function never returns error pointers.  It returns
> NULL on error.
Will fix the same, Thanks
>
> Fixes: 358961d1cd1e ("crypto: chcr - Added new structure chcr_wr")
> Signed-off-by: Dan Carpenter 
>
> diff --git a/drivers/crypto/chelsio/chcr_algo.c 
> b/drivers/crypto/chelsio/chcr_algo.c
> index 32361dd..2ed1e24 100644
> --- a/drivers/crypto/chelsio/chcr_algo.c
> +++ b/drivers/crypto/chelsio/chcr_algo.c
> @@ -958,9 +958,8 @@ static int chcr_ahash_update(struct ahash_request *req)
>   req_ctx->result = 0;
>   req_ctx->data_len += params.sg_len + params.bfr_len;
>   skb = create_hash_wr(req, );
> -
> - if (IS_ERR(skb))
> - return PTR_ERR(skb);
> + if (!skb)
> + return -ENOMEM;
>  
>   if (remainder) {
>   u8 *temp;
> @@ -1023,8 +1022,8 @@ static int chcr_ahash_final(struct ahash_request *req)
>   params.more = 0;
>   }
>   skb = create_hash_wr(req, );
> - if (IS_ERR(skb))
> - return PTR_ERR(skb);
> + if (!skb)
> + return -ENOMEM;
>  
>   skb->dev = u_ctx->lldi.ports[0];
>   set_wr_txq(skb, CPL_PRIORITY_DATA, ctx->tx_channel_id);
> @@ -1074,8 +1073,8 @@ static int chcr_ahash_finup(struct ahash_request *req)
>   }
>  
>   skb = create_hash_wr(req, );
> - if (IS_ERR(skb))
> - return PTR_ERR(skb);
> + if (!skb)
> + return -ENOMEM;
>  
>   skb->dev = u_ctx->lldi.ports[0];
>   set_wr_txq(skb, CPL_PRIORITY_DATA, ctx->tx_channel_id);
> @@ -1125,8 +1124,8 @@ static int chcr_ahash_digest(struct ahash_request *req)
>   }
>  
>   skb = create_hash_wr(req, );
> - if (IS_ERR(skb))
> - return PTR_ERR(skb);
> + if (!skb)
> + return -ENOMEM;
>  
>   skb->dev = u_ctx->lldi.ports[0];
>   set_wr_txq(skb, CPL_PRIORITY_DATA, ctx->tx_channel_id);

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] crypto: chcr - checking for IS_ERR() instead of NULL

2016-12-01 Thread Harsh Jain 


On 02-12-2016 11:29, Harsh Jain wrote:
>
> On 02-12-2016 02:19, Dan Carpenter wrote:
>> The create_hash_wr() function never returns error pointers.  It returns
>> NULL on error.
> Will fix the same, Thanks
It's a patch mail not bug reporting!  Sorry for confusion. You have already 
fixed it. Thanks.
>> Fixes: 358961d1cd1e ("crypto: chcr - Added new structure chcr_wr")
>> Signed-off-by: Dan Carpenter 
>>
>> diff --git a/drivers/crypto/chelsio/chcr_algo.c 
>> b/drivers/crypto/chelsio/chcr_algo.c
>> index 32361dd..2ed1e24 100644
>> --- a/drivers/crypto/chelsio/chcr_algo.c
>> +++ b/drivers/crypto/chelsio/chcr_algo.c
>> @@ -958,9 +958,8 @@ static int chcr_ahash_update(struct ahash_request *req)
>>  req_ctx->result = 0;
>>  req_ctx->data_len += params.sg_len + params.bfr_len;
>>  skb = create_hash_wr(req, );
>> -
>> -if (IS_ERR(skb))
>> -return PTR_ERR(skb);
>> +if (!skb)
>> +return -ENOMEM;
>>  
>>  if (remainder) {
>>  u8 *temp;
>> @@ -1023,8 +1022,8 @@ static int chcr_ahash_final(struct ahash_request *req)
>>  params.more = 0;
>>  }
>>  skb = create_hash_wr(req, );
>> -if (IS_ERR(skb))
>> -return PTR_ERR(skb);
>> +if (!skb)
>> +return -ENOMEM;
>>  
>>  skb->dev = u_ctx->lldi.ports[0];
>>  set_wr_txq(skb, CPL_PRIORITY_DATA, ctx->tx_channel_id);
>> @@ -1074,8 +1073,8 @@ static int chcr_ahash_finup(struct ahash_request *req)
>>  }
>>  
>>  skb = create_hash_wr(req, );
>> -if (IS_ERR(skb))
>> -return PTR_ERR(skb);
>> +if (!skb)
>> +return -ENOMEM;
>>  
>>  skb->dev = u_ctx->lldi.ports[0];
>>  set_wr_txq(skb, CPL_PRIORITY_DATA, ctx->tx_channel_id);
>> @@ -1125,8 +1124,8 @@ static int chcr_ahash_digest(struct ahash_request *req)
>>  }
>>  
>>  skb = create_hash_wr(req, );
>> -if (IS_ERR(skb))
>> -return PTR_ERR(skb);
>> +if (!skb)
>> +return -ENOMEM;
>>  
>>  skb->dev = u_ctx->lldi.ports[0];
>>  set_wr_txq(skb, CPL_PRIORITY_DATA, ctx->tx_channel_id);

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] Add crypto driver support for some MediaTek chips

2016-12-01 Thread Ryder Lee 
This adds support for the MediaTek hardware accelerator on
mt7623/mt2701/mt8521p SoC.

This driver currently implement:
- SHA1 and SHA2 family(HMAC) hash alogrithms.
- AES block cipher in CBC/ECB mode with 128/196/256 bits keys.

Signed-off-by: Ryder Lee 
---
 drivers/crypto/Kconfig |   17 +
 drivers/crypto/Makefile|1 +
 drivers/crypto/mediatek/Makefile   |2 +
 drivers/crypto/mediatek/mtk-aes.c  |  734 +
 drivers/crypto/mediatek/mtk-platform.c |  575 +
 drivers/crypto/mediatek/mtk-platform.h |  230 ++
 drivers/crypto/mediatek/mtk-regs.h |  194 +
 drivers/crypto/mediatek/mtk-sha.c  | 1384 
 8 files changed, 3137 insertions(+)
 create mode 100644 drivers/crypto/mediatek/Makefile
 create mode 100644 drivers/crypto/mediatek/mtk-aes.c
 create mode 100644 drivers/crypto/mediatek/mtk-platform.c
 create mode 100644 drivers/crypto/mediatek/mtk-platform.h
 create mode 100644 drivers/crypto/mediatek/mtk-regs.h
 create mode 100644 drivers/crypto/mediatek/mtk-sha.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f..5d9c803 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -553,6 +553,23 @@ config CRYPTO_DEV_ROCKCHIP
  This driver interfaces with the hardware crypto accelerator.
  Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode.
 
+config CRYPTO_DEV_MEDIATEK
+   tristate "MediaTek's Cryptographic Engine driver"
+   depends on ARM && ARCH_MEDIATEK
+   select NEON
+   select KERNEL_MODE_NEON
+   select ARM_CRYPTO
+   select CRYPTO_AES
+   select CRYPTO_BLKCIPHER
+   select CRYPTO_SHA1_ARM_NEON
+   select CRYPTO_SHA256_ARM
+   select CRYPTO_SHA512_ARM
+   select CRYPTO_HMAC
+   help
+ This driver allows you to utilize the hardware crypto accelerator
+ which can be found on the MT7623 MT2701, MT8521p, etc 
+ Select this if you want to use it for AES/SHA1/SHA2 algorithms.
+
 source "drivers/crypto/chelsio/Kconfig"
 
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index ad7250f..272b51a 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_CRYPTO_DEV_IMGTEC_HASH) += img-hash.o
 obj-$(CONFIG_CRYPTO_DEV_IXP4XX) += ixp4xx_crypto.o
 obj-$(CONFIG_CRYPTO_DEV_MV_CESA) += mv_cesa.o
 obj-$(CONFIG_CRYPTO_DEV_MARVELL_CESA) += marvell/
+obj-$(CONFIG_CRYPTO_DEV_MEDIATEK) += mediatek/
 obj-$(CONFIG_CRYPTO_DEV_MXS_DCP) += mxs-dcp.o
 obj-$(CONFIG_CRYPTO_DEV_NIAGARA2) += n2_crypto.o
 n2_crypto-y := n2_core.o n2_asm.o
diff --git a/drivers/crypto/mediatek/Makefile b/drivers/crypto/mediatek/Makefile
new file mode 100644
index 000..187be79
--- /dev/null
+++ b/drivers/crypto/mediatek/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_CRYPTO_DEV_MEDIATEK) += mtk-crypto.o
+mtk-crypto-objs:= mtk-platform.o mtk-aes.o mtk-sha.o
diff --git a/drivers/crypto/mediatek/mtk-aes.c 
b/drivers/crypto/mediatek/mtk-aes.c
new file mode 100644
index 000..feb0e57
--- /dev/null
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -0,0 +1,734 @@
+/*
+ * Cryptographic API.
+ *
+ * Support for MediaTek AES hardware accelerator.
+ *
+ * Copyright (c) 2016 MediaTek Inc.
+ * Author: Ryder Lee 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Some ideas are from atmel-aes.c drivers.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "mtk-platform.h"
+#include "mtk-regs.h"
+
+#define AES_QUEUE_LENGTH   512
+#define AES_BUFFER_ORDER   2
+#define AES_BUFFER_SIZE((PAGE_SIZE << AES_BUFFER_ORDER) \
+   & ~(AES_BLOCK_SIZE - 1))
+
+/* AES command token */
+#define AES_CT_SIZE_ECB2
+#define AES_CT_SIZE_CBC3
+#define AES_CT_CTRL_HDR0x0022
+#define AES_COMMAND0   0x0500
+#define AES_COMMAND1   0x2d06
+#define AES_COMMAND2   0xe4a63806
+
+/* AES transform information */
+#define AES_TFM_ECB(0x0 << 0)
+#define AES_TFM_CBC(0x1 << 0)
+#define AES_TFM_DECRYPT(0x5 << 0)
+#define AES_TFM_ENCRYPT(0x4 << 0)
+#define AES_TFM_SIZE(x)((x) << 8)
+#define AES_TFM_128BITS(0xb << 16)
+#define AES_TFM_192BITS(0xd << 16)
+#define AES_TFM_256BITS(0xf << 16)
+#define AES_TFM_FULL_IV(0xf << 5)
+
+/* AES flags */
+#define AES_FLAGS_MODE_MSK GENMASK(2, 0)
+#define AES_FLAGS_ECB  BIT(0)
+#define AES_FLAGS_CBC  BIT(1)
+#define AES_FLAGS_ENCRYPT  BIT(2)
+#define AES_FLAGS_BUSY BIT(3)
+
+/* AES command token */
+struct mtk_aes_ct {
+   u32 ct_ctrl0;
+

[PATCH 2/2] crypto: mediatek - add DT bindings documentation

2016-12-01 Thread Ryder Lee 
Add DT bindings documentation for the crypto driver

Signed-off-by: Ryder Lee 
---
 .../devicetree/bindings/crypto/mediatek-crypto.txt | 32 ++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/mediatek-crypto.txt

diff --git a/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt 
b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
new file mode 100644
index 000..8b1db08
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
@@ -0,0 +1,32 @@
+MediaTek cryptographic accelerators
+
+Required properties:
+- compatible: Should be "mediatek,mt7623-crypto"
+- reg: Address and length of the register set for the device
+- interrupts: Should contain the five crypto engines interrupts in numeric
+   order. These are global system and four descriptor rings.
+- clocks: the clock used by the core
+- clock-names: the names of the clock listed in the clocks property. These are
+   "ethif", "cryp"
+- power-domains: Must contain a reference to the PM domain.
+
+
+Optional properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+  that services interrupts for this device
+
+
+Example:
+   crypto: crypto@1b24 {
+   compatible = "mediatek,mt7623-crypto";
+   reg = <0 0x1b24 0 0x2>;
+   interrupts = ,
+,
+,
+,
+;
+   clocks = < CLK_TOP_ETHIF_SEL>,
+< CLK_ETHSYS_CRYPTO>;
+   clock-names = "ethif","cryp";
+   power-domains = < MT2701_POWER_DOMAIN_ETH>;
+   };
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] Add MediaTek crypto acclelrator driver

2016-12-01 Thread Ryder Lee 
This adds support for the MediaTek hardware accelerator on 
mt7623 SoC.

This driver currently implement: 
- SHA1 and SHA2 family(HMAC) hash alogrithms.
- AES block cipher in CBC/ECB mode with 128/196/256 bits keys.

Ryder Lee (2):
  Add crypto driver support for some MediaTek chips
  crypto: mediatek - add DT bindings documentation

 .../devicetree/bindings/crypto/mediatek-crypto.txt |   32 +
 drivers/crypto/Kconfig |   17 +
 drivers/crypto/Makefile|1 +
 drivers/crypto/mediatek/Makefile   |2 +
 drivers/crypto/mediatek/mtk-aes.c  |  734 +++
 drivers/crypto/mediatek/mtk-platform.c |  575 
 drivers/crypto/mediatek/mtk-platform.h |  230 
 drivers/crypto/mediatek/mtk-regs.h |  194 +++
 drivers/crypto/mediatek/mtk-sha.c  | 1384 
 9 files changed, 3169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
 create mode 100644 drivers/crypto/mediatek/Makefile
 create mode 100644 drivers/crypto/mediatek/mtk-aes.c
 create mode 100644 drivers/crypto/mediatek/mtk-platform.c
 create mode 100644 drivers/crypto/mediatek/mtk-platform.h
 create mode 100644 drivers/crypto/mediatek/mtk-regs.h
 create mode 100644 drivers/crypto/mediatek/mtk-sha.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Crash in crypto mcryptd

2016-12-01 Thread Tim Chen 
On Thu, 2016-12-01 at 19:00 -0500, Mikulas Patocka wrote:
> Hi
> 
> There is a bug in mcryptd initialization.
> 
> This is a test module that tries various hash algorithms. When you load 
> the module with "insmod test.ko 'alg=mcryptd(md5)'", the machine crashes.

I don't think your test setup is right.  The mcryptd supports only multi-buffer
algorithm.  I don't think there is such an implementation for md5.

Please refer to arch/x86/crypto/sha1-mb 
multi-buffer implementation of sha1 to see the proper
setup and usage with mcryptd.  You can also run tcrypt test to
exercise this code.

Tim

> 
> Mikulas
> 
> 
> #include 
> #include 
> #include 
> 
> static char *alg = "md5";
> 
> module_param_named(alg, alg, charp, 0444);
> MODULE_PARM_DESC(alg, "the algorith to test");
> 
> static bool sync = true;
> 
> module_param_named(sync, sync, bool, 0444);
> MODULE_PARM_DESC(alg, "sync flag");
> 
> static int __init dump_init(void)
> {
>     struct crypto_shash *h;
>     char key[4];
>     int r;
>     printk("testing algorithm '%s'\n", alg);
>     h = crypto_alloc_shash(alg, 0, sync ? CRYPTO_ALG_ASYNC : 0);
>     if (IS_ERR(h)) {
>     printk("error %d\n", (int)PTR_ERR(h));
>     return PTR_ERR(h);
>     }
>     printk("setting key\n");
>     r = crypto_shash_setkey(h, key, sizeof key);
>     if (r)
>     printk("setkey: %d\n", r);
>     crypto_free_shash(h);
>     printk("module loaded\n");
>     return 0;
> }
> 
> static void __exit dump_exit(void)
> {
>     printk("dump exit\n");
> }
> 
> module_init(dump_init)
> module_exit(dump_exit)
> MODULE_LICENSE("GPL");
> 
> 
> [898029.802035] BUG: unable to handle kernel NULL pointer dereference at  
>  
> (null)
> [898029.806060] IP: [] md5_final+0xad/0x210 [md5]
> [898029.808156] PGD 11a5d8067 [898029.809051] PUD 11a491067 
> PMD 0 [898029.810280] 
> [898029.810904] Oops: 0002 [#1] PREEMPT SMP
> [898029.812239] Modules linked in: md5 testdump(O+) mcryptd uvesafb 
> cfbfillrect cfbimgblt cn cfbcopyarea fbcon bitblit fbcon_rotate fbcon_ccw 
> fbcon_ud fbcon_cw softcursor fb fbdev font ipv6 binfmt_misc mousedev 
> af_packet psmouse pcspkr virtio_net virtio_balloon button ext4 crc16 jbd2 
> mbcache dm_mod virtio_blk evdev virtio_pci virtio_ring virtio
> [898029.817178] CPU: 9 PID: 187 Comm: kworker/9:1 Tainted: G   O
> 4.9.0-rc7+ #6
> [898029.818066] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [898029.818732] Workqueue: crypto mcryptd_queue_worker [mcryptd]
> [898029.819394] task: 88011aa2bd80 task.stack: 88011848
> [898029.820077] RIP: 0010:[]  [] 
> md5_final+0xad/0x210 [md5]
> [898029.821050] RSP: 0018:880118483d48  EFLAGS: 00010286
> [898029.821661] RAX: 04b2008fd98c1dd4 RBX: 880119cd7f28 RCX: 
> 980980e9
> [898029.822464] RDX: 7e42f8ec980980e9 RSI: ef1c4f74 RDI: 
> 880119cd7f30
> [898029.823293] RBP: 880118483d68 R08: 1b99d513 R09: 
> 
> [898029.824117] R10:  R11: b8b56373 R12: 
> 880119cd7f18
> [898029.824944] R13:  R14: 880119cd7f38 R15: 
> a01ee43c
> [898029.825776] FS:  () GS:88011fd2() 
> knlGS:
> [898029.826712] CS:  0010 DS:  ES:  CR0: 80050033
> [898029.827376] CR2:  CR3: 00011a6c9000 CR4: 
> 06a0
> [898029.828204] Stack:
> [898029.828452]  880119cd7f18 88011fd3bb00  
> 880119cd7e00
> [898029.829351]  880118483da0 8119f281 880119cd7f18 
> 88011fd3bb00
> [898029.830242]  88011fd3bae0 880119cd7e00 a01ee43c 
> 880119cd7ec8
> [898029.831141] Call Trace:
> [898029.831460]  [] ? crypto_shash_final+0x31/0xb0
> [898029.832151]  [] ? mcryptd_queue_worker+0x1c/0x190 
> [mcryptd]
> [898029.832980]  [] ? shash_ahash_finup+0x73/0x80
> [898029.833672]  [] ? __switch_to+0x27f/0x460
> [898029.834305]  [] ? mcryptd_hash_digest+0x4f/0x80 
> [mcryptd]
> [898029.835125]  [] ? mcryptd_queue_worker+0x47/0x190 
> [mcryptd]
> [898029.835963]  [] ? process_one_work+0x1bf/0x3f0
> [898029.836681]  [] ? worker_thread+0x42/0x4c0
> [898029.837362]  [] ? process_one_work+0x3f0/0x3f0
> [898029.838045]  [] ? process_one_work+0x3f0/0x3f0
> [898029.838739]  [] ? kthread+0xb9/0xd0
> [898029.839318]  [] ? kthread_park+0x70/0x70
> [898029.839959]  [] ? ret_from_fork+0x25/0x30
> [898029.840594] Code: 14 c5 00 00 00 00 48 c1 e8 1d 41 89 44 24 5c 41 89 
> 54 24 58 e8 45 ea 0e e1 49 8b 44 24 10 49 8b 54 24 18 48 8d 7b 08 48 83 e7 
> f8 <49> 89 45 00 49 89 55 08 31 c0 49 c7 44 24 10 00 00 00 00 48 c7 
> [898029.843633] RIP  [] md5_final+0xad/0x210 [md5]
> [898029.844354]  RSP 
> [898029.844769] CR2: 
> [898029.845166] ---[ end trace 2ecde0bf66717337 ]---
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org

Crash in crypto mcryptd

2016-12-01 Thread Mikulas Patocka 
Hi

There is a bug in mcryptd initialization.

This is a test module that tries various hash algorithms. When you load 
the module with "insmod test.ko 'alg=mcryptd(md5)'", the machine crashes.

Mikulas


#include 
#include 
#include 

static char *alg = "md5";

module_param_named(alg, alg, charp, 0444);
MODULE_PARM_DESC(alg, "the algorith to test");

static bool sync = true;

module_param_named(sync, sync, bool, 0444);
MODULE_PARM_DESC(alg, "sync flag");

static int __init dump_init(void)
{
struct crypto_shash *h;
char key[4];
int r;
printk("testing algorithm '%s'\n", alg);
h = crypto_alloc_shash(alg, 0, sync ? CRYPTO_ALG_ASYNC : 0);
if (IS_ERR(h)) {
printk("error %d\n", (int)PTR_ERR(h));
return PTR_ERR(h);
}
printk("setting key\n");
r = crypto_shash_setkey(h, key, sizeof key);
if (r)
printk("setkey: %d\n", r);
crypto_free_shash(h);
printk("module loaded\n");
return 0;
}

static void __exit dump_exit(void)
{
printk("dump exit\n");
}

module_init(dump_init)
module_exit(dump_exit)
MODULE_LICENSE("GPL");


[898029.802035] BUG: unable to handle kernel NULL pointer dereference at
   
(null)
[898029.806060] IP: [] md5_final+0xad/0x210 [md5]
[898029.808156] PGD 11a5d8067 [898029.809051] PUD 11a491067 
PMD 0 [898029.810280] 
[898029.810904] Oops: 0002 [#1] PREEMPT SMP
[898029.812239] Modules linked in: md5 testdump(O+) mcryptd uvesafb 
cfbfillrect cfbimgblt cn cfbcopyarea fbcon bitblit fbcon_rotate fbcon_ccw 
fbcon_ud fbcon_cw softcursor fb fbdev font ipv6 binfmt_misc mousedev 
af_packet psmouse pcspkr virtio_net virtio_balloon button ext4 crc16 jbd2 
mbcache dm_mod virtio_blk evdev virtio_pci virtio_ring virtio
[898029.817178] CPU: 9 PID: 187 Comm: kworker/9:1 Tainted: G   O
4.9.0-rc7+ #6
[898029.818066] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[898029.818732] Workqueue: crypto mcryptd_queue_worker [mcryptd]
[898029.819394] task: 88011aa2bd80 task.stack: 88011848
[898029.820077] RIP: 0010:[]  [] 
md5_final+0xad/0x210 [md5]
[898029.821050] RSP: 0018:880118483d48  EFLAGS: 00010286
[898029.821661] RAX: 04b2008fd98c1dd4 RBX: 880119cd7f28 RCX: 
980980e9
[898029.822464] RDX: 7e42f8ec980980e9 RSI: ef1c4f74 RDI: 
880119cd7f30
[898029.823293] RBP: 880118483d68 R08: 1b99d513 R09: 

[898029.824117] R10:  R11: b8b56373 R12: 
880119cd7f18
[898029.824944] R13:  R14: 880119cd7f38 R15: 
a01ee43c
[898029.825776] FS:  () GS:88011fd2() 
knlGS:
[898029.826712] CS:  0010 DS:  ES:  CR0: 80050033
[898029.827376] CR2:  CR3: 00011a6c9000 CR4: 
06a0
[898029.828204] Stack:
[898029.828452]  880119cd7f18 88011fd3bb00  
880119cd7e00
[898029.829351]  880118483da0 8119f281 880119cd7f18 
88011fd3bb00
[898029.830242]  88011fd3bae0 880119cd7e00 a01ee43c 
880119cd7ec8
[898029.831141] Call Trace:
[898029.831460]  [] ? crypto_shash_final+0x31/0xb0
[898029.832151]  [] ? mcryptd_queue_worker+0x1c/0x190 
[mcryptd]
[898029.832980]  [] ? shash_ahash_finup+0x73/0x80
[898029.833672]  [] ? __switch_to+0x27f/0x460
[898029.834305]  [] ? mcryptd_hash_digest+0x4f/0x80 
[mcryptd]
[898029.835125]  [] ? mcryptd_queue_worker+0x47/0x190 
[mcryptd]
[898029.835963]  [] ? process_one_work+0x1bf/0x3f0
[898029.836681]  [] ? worker_thread+0x42/0x4c0
[898029.837362]  [] ? process_one_work+0x3f0/0x3f0
[898029.838045]  [] ? process_one_work+0x3f0/0x3f0
[898029.838739]  [] ? kthread+0xb9/0xd0
[898029.839318]  [] ? kthread_park+0x70/0x70
[898029.839959]  [] ? ret_from_fork+0x25/0x30
[898029.840594] Code: 14 c5 00 00 00 00 48 c1 e8 1d 41 89 44 24 5c 41 89 
54 24 58 e8 45 ea 0e e1 49 8b 44 24 10 49 8b 54 24 18 48 8d 7b 08 48 83 e7 
f8 <49> 89 45 00 49 89 55 08 31 c0 49 c7 44 24 10 00 00 00 00 48 c7 
[898029.843633] RIP  [] md5_final+0xad/0x210 [md5]
[898029.844354]  RSP 
[898029.844769] CR2: 
[898029.845166] ---[ end trace 2ecde0bf66717337 ]---

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] crypto: brcm: Add Broadcom SPU driver

2016-12-01 Thread kbuild test robot 
Hi Rob,

[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.9-rc7 next-20161201]
[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/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> ERROR: "des_ekey" [drivers/crypto/bcm/bcm_crypto_spu.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[bug report] crypto: chcr - Add AEAD algos.

2016-12-01 Thread Dan Carpenter 
Hello Harsh Jain,

This is a semi-automatic email about new static checker warnings.

The patch 2debd3325e55: "crypto: chcr - Add AEAD algos." from Nov 29, 
2016, leads to the following Smatch complaint:

drivers/crypto/chelsio/chcr_algo.c:2460 chcr_aead_op()
warn: variable dereferenced before check 'ctx' (see line 2457)

drivers/crypto/chelsio/chcr_algo.c:2460 chcr_aead_op()
warn: variable dereferenced before check 'ctx->dev' (see line 2457)

drivers/crypto/chelsio/chcr_algo.c
  2456  struct chcr_context *ctx = crypto_aead_ctx(tfm);
  2457  struct uld_ctx *u_ctx = ULD_CTX(ctx);

Derefences both.

  2458  struct sk_buff *skb;
  2459  
  2460  if (ctx && !ctx->dev) {

Check is too late.

  2461  pr_err("chcr : %s : No crypto device.\n", __func__);
  2462  return -ENXIO;

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: marvell - Don't copy hash operation twice into the SRAM

2016-12-01 Thread Gregory CLEMENT 
Hi Romain,
 
 On jeu., déc. 01 2016, Romain Perier  wrote:

> No need to copy the template of an hash operation twice into the SRAM
> from the step function.
>

Does this patch fix a bug ot it is jsute a cleanup/improvement?

If it is a bug you should CC stable and add use the Fixes tag.

Gregory

> Signed-off-by: Romain Perier 
> ---
>  drivers/crypto/marvell/hash.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
> index 2a92605..fbbcbf8 100644
> --- a/drivers/crypto/marvell/hash.c
> +++ b/drivers/crypto/marvell/hash.c
> @@ -172,9 +172,6 @@ static void mv_cesa_ahash_std_step(struct ahash_request 
> *req)
>   for (i = 0; i < digsize / 4; i++)
>   writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
>  
> - mv_cesa_adjust_op(engine, >op_tmpl);
> - memcpy_toio(engine->sram, >op_tmpl, sizeof(creq->op_tmpl));
> -
>   if (creq->cache_ptr)
>   memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,
>   creq->cache, creq->cache_ptr);
> -- 
> 2.9.3
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] crypto: marvell - Don't copy hash operation twice into the SRAM

2016-12-01 Thread Romain Perier 
No need to copy the template of an hash operation twice into the SRAM
from the step function.

Signed-off-by: Romain Perier 
---
 drivers/crypto/marvell/hash.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 2a92605..fbbcbf8 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -172,9 +172,6 @@ static void mv_cesa_ahash_std_step(struct ahash_request 
*req)
for (i = 0; i < digsize / 4; i++)
writel_relaxed(creq->state[i], engine->regs + CESA_IVDIG(i));
 
-   mv_cesa_adjust_op(engine, >op_tmpl);
-   memcpy_toio(engine->sram, >op_tmpl, sizeof(creq->op_tmpl));
-
if (creq->cache_ptr)
memcpy_toio(engine->sram + CESA_SA_DATA_SRAM_OFFSET,
creq->cache, creq->cache_ptr);
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: arm/aesbs - Select SIMD in Kconfig

2016-12-01 Thread Arnd Bergmann 
On Thursday, December 1, 2016 8:56:16 PM CET Herbert Xu wrote:
> On Thu, Dec 01, 2016 at 12:47:59AM +0100, Arnd Bergmann wrote:
> > Commit 585b5fa63da9 ("crypto: arm/aes - Select SIMD in Kconfig") added
> > the dependency for CRYPTO_AES_ARM_CE, but missed the same change
> > for CRYPTO_AES_ARM_BS:
> > 
> > arch/arm/crypto/aes-arm-bs.o: In function `aesbs_mod_init':
> > aesbs-glue.c:(.init.text+0x38): undefined reference to 
> > `simd_skcipher_create_compat'
> > 
> > Fixes: 211f41af534a ("crypto: aesbs - Convert to skcipher")
> > Signed-off-by: Arnd Bergmann 
> 
> Thanks Arnd.  This should already be fixed by 6fdf436fd854.

Right. I have discarded my patch on today's linux-next and I
get no more link errors.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] crypto: set error code when kcalloc fails

2016-12-01 Thread Herbert Xu 
On Thu, Dec 01, 2016 at 10:04:43AM +0800, Pan Bian wrote:
> Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188521. In function
> skcipher_recvmsg_async(), variable err takes the return value, and its
> value should be negative on failures. Because variable err may be
> reassigned and checked before calling kcalloc(), its value may be 0
> (indicates no error) even if kcalloc() fails. This patch fixes the bug
> by explicitly assigning -ENOMEM to err when kcalloc() returns a NULL
> pointer.
> 
> Signed-off-by: Pan Bian 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] crypto: AF_ALG - fix AEAD AIO handling of zero buffer

2016-12-01 Thread Herbert Xu 
On Thu, Dec 01, 2016 at 08:22:37AM +0100, Stephan Mueller wrote:
> Hi Herbert,
> 
> I split out the bug fix patch from the AD/tag formatting patch as they most 
> likely will come after the next merge window.
> 
> ---8<---
> 
> Handle the case when the caller provided a zero buffer to
> sendmsg/sendpage. Such scenario is legal for AEAD ciphers when no
> plaintext / ciphertext and no AAD is provided and the caller only
> requests the generation of the tag value.
> 
> Signed-off-by: Stephan Mueller 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] crypto: caam: pass key buffers with typesafe pointers

2016-12-01 Thread Herbert Xu 
On Wed, Nov 30, 2016 at 10:01:59PM +0100, Arnd Bergmann wrote:
> The 'key' field is defined as a 'u64' and used for two different
> pieces of information: either to store a pointer or a dma_addr_t.
> The former leads to a build error on 32-bit machines:
> 
> drivers/crypto/caam/caamalg_desc.c: In function 'cnstr_shdsc_aead_null_encap':
> drivers/crypto/caam/caamalg_desc.c:67:27: error: cast to pointer from integer 
> of different size [-Werror=int-to-pointer-cast]
> drivers/crypto/caam/caamalg_desc.c: In function 'cnstr_shdsc_aead_null_decap':
> drivers/crypto/caam/caamalg_desc.c:143:27: error: cast to pointer from 
> integer of different size [-Werror=int-to-pointer-cast]
> 
> Using a union to provide correct types gets rid of the warnings
> and as well as a couple of redundant casts.
> 
> Fixes: db57656b0072 ("crypto: caam - group algorithm related params")
> Signed-off-by: Arnd Bergmann 

Both patches applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: arm/aesbs - Select SIMD in Kconfig

2016-12-01 Thread Herbert Xu 
On Thu, Dec 01, 2016 at 12:47:59AM +0100, Arnd Bergmann wrote:
> Commit 585b5fa63da9 ("crypto: arm/aes - Select SIMD in Kconfig") added
> the dependency for CRYPTO_AES_ARM_CE, but missed the same change
> for CRYPTO_AES_ARM_BS:
> 
> arch/arm/crypto/aes-arm-bs.o: In function `aesbs_mod_init':
> aesbs-glue.c:(.init.text+0x38): undefined reference to 
> `simd_skcipher_create_compat'
> 
> Fixes: 211f41af534a ("crypto: aesbs - Convert to skcipher")
> Signed-off-by: Arnd Bergmann 

Thanks Arnd.  This should already be fixed by 6fdf436fd854.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/1] crypto: add virtio-crypto driver

2016-12-01 Thread Gonglei 
This patch introduces virtio-crypto driver for Linux Kernel.

The virtio crypto device is a virtual cryptography device
as well as a kind of virtual hardware accelerator for
virtual machines. The encryption anddecryption requests
are placed in the data queue and are ultimately handled by
thebackend crypto accelerators. The second queue is the
control queue used to create or destroy sessions for
symmetric algorithms and will control some advanced features
in the future. The virtio crypto device provides the following
cryptoservices: CIPHER, MAC, HASH, and AEAD.

For more information about virtio-crypto device, please see:
  http://qemu-project.org/Features/VirtioCrypto

CC: Michael S. Tsirkin 
CC: Cornelia Huck 
CC: Stefan Hajnoczi 
CC: Herbert Xu 
CC: Halil Pasic 
CC: David S. Miller 
CC: Zeng Xin 
Signed-off-by: Gonglei 
---
 MAINTAINERS  |   9 +
 drivers/crypto/Kconfig   |   2 +
 drivers/crypto/Makefile  |   1 +
 drivers/crypto/virtio/Kconfig|  10 +
 drivers/crypto/virtio/Makefile   |   5 +
 drivers/crypto/virtio/virtio_crypto_algs.c   | 537 +++
 drivers/crypto/virtio/virtio_crypto_common.h | 122 ++
 drivers/crypto/virtio/virtio_crypto_core.c   | 464 +++
 drivers/crypto/virtio/virtio_crypto_mgr.c| 264 +
 include/uapi/linux/Kbuild|   1 +
 include/uapi/linux/virtio_crypto.h   | 450 ++
 include/uapi/linux/virtio_ids.h  |   1 +
 12 files changed, 1866 insertions(+)
 create mode 100644 drivers/crypto/virtio/Kconfig
 create mode 100644 drivers/crypto/virtio/Makefile
 create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c
 create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h
 create mode 100644 drivers/crypto/virtio/virtio_crypto_core.c
 create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c
 create mode 100644 include/uapi/linux/virtio_crypto.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ad9b965..cccaaf0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12810,6 +12810,7 @@ F:  drivers/net/virtio_net.c
 F: drivers/block/virtio_blk.c
 F: include/linux/virtio_*.h
 F: include/uapi/linux/virtio_*.h
+F: drivers/crypto/virtio/
 
 VIRTIO DRIVERS FOR S390
 M: Christian Borntraeger 
@@ -12846,6 +12847,14 @@ S: Maintained
 F: drivers/virtio/virtio_input.c
 F: include/uapi/linux/virtio_input.h
 
+VIRTIO CRYPTO DRIVER
+M:  Gonglei 
+L:  virtualizat...@lists.linux-foundation.org
+L:  linux-crypto@vger.kernel.org
+S:  Maintained
+F:  drivers/crypto/virtio/
+F:  include/uapi/linux/virtio_crypto.h
+
 VIA RHINE NETWORK DRIVER
 S: Orphan
 F: drivers/net/ethernet/via/via-rhine.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f..7956478 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -555,4 +555,6 @@ config CRYPTO_DEV_ROCKCHIP
 
 source "drivers/crypto/chelsio/Kconfig"
 
+source "drivers/crypto/virtio/Kconfig"
+
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index ad7250f..bc53cb8 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
 obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
 obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
 obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
+obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/
diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig
new file mode 100644
index 000..d80f733
--- /dev/null
+++ b/drivers/crypto/virtio/Kconfig
@@ -0,0 +1,10 @@
+config CRYPTO_DEV_VIRTIO
+   tristate "VirtIO crypto driver"
+   depends on VIRTIO
+   select CRYPTO_AEAD
+   select CRYPTO_AUTHENC
+   select CRYPTO_BLKCIPHER
+   default m
+   help
+ This driver provides support for virtio crypto device. If you
+ choose 'M' here, this module will be called virtio_crypto.
diff --git a/drivers/crypto/virtio/Makefile b/drivers/crypto/virtio/Makefile
new file mode 100644
index 000..dd342c9
--- /dev/null
+++ b/drivers/crypto/virtio/Makefile
@@ -0,0 +1,5 @@
+obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio_crypto.o
+virtio_crypto-objs := \
+   virtio_crypto_algs.o \
+   virtio_crypto_mgr.o \
+   virtio_crypto_core.o
diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c 
b/drivers/crypto/virtio/virtio_crypto_algs.c
new file mode 100644
index 000..5100056
--- /dev/null
+++ b/drivers/crypto/virtio/virtio_crypto_algs.c
@@ -0,0 +1,537 @@
+ /* Algorithms supported by virtio crypto device
+  *
+  * Authors: Gonglei 
+  *
+  * Copyright 2016 HUAWEI TECHNOLOGIES CO.,

Re: [PATCH] crypto: caam - fix key pointer size warning

2016-12-01 Thread Herbert Xu 
On Tue, Nov 29, 2016 at 10:22:49AM +0200, Horia Geantă wrote:
> When building on 32-bit, compiler issues [-Wint-to-pointer-cast] warnings:
> 
> drivers/crypto/caam/caamalg_desc.c: In function 'cnstr_shdsc_aead_null_encap':
> drivers/crypto/caam/caamalg_desc.c:67:27: warning: cast to pointer from 
> integer of different size [-Wint-to-pointer-cast]
>append_key_as_imm(desc, (void *)adata->key, adata->keylen_pad,
> [...]
> 
> Since in all these cases the u64 "key" member of alginfo struct holds
> a uintptr_t value, we can safely remove the warnings by explicitly casting
> to uinptr_t (before any other existing cast).
> 
> Fixes: 8cea7b66b821 ("crypto: caam - refactor encryption descriptors 
> generation")
> Reported-by: kbuild test robot 
> Signed-off-by: Horia Geantă 

Thanks for the patch.  I think Arnd's version is cleaner though
so I'll take that instead.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v4 1/1] crypto: add virtio-crypto driver

2016-12-01 Thread Gonglei (Arei) 
> 
> On Thu, Dec 01, 2016 at 02:27:19AM +, Gonglei (Arei) wrote:
> > > On Tue, Nov 29, 2016 at 08:48:14PM +0800, Gonglei wrote:
> > > > diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
> > > b/drivers/crypto/virtio/virtio_crypto_algs.c
> > > > new file mode 100644
> > > > index 000..08b077f
> > > > --- /dev/null
> > > > +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> > > > @@ -0,0 +1,518 @@
> > > > + /* Algorithms supported by virtio crypto device
> > > > +  *
> > > > +  * Authors: Gonglei 
> > > > +  *
> > > > +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > > > +  *
> > > > +  * This program is free software; you can redistribute it and/or 
> > > > modify
> > > > +  * it under the terms of the GNU General Public License as published 
> > > > by
> > > > +  * the Free Software Foundation; either version 2 of the License, or
> > > > +  * (at your option) any later version.
> > > > +  *
> > > > +  * This program is distributed in the hope that it will be useful,
> > > > +  * but WITHOUT ANY WARRANTY; without even the implied warranty
> of
> > > > +  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> the
> > > > +  * GNU General Public License for more details.
> > > > +  *
> > > > +  * You should have received a copy of the GNU General Public License
> > > > +  * along with this program; if not, see
> .
> > > > +  */
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +#include 
> > > > +#include "virtio_crypto_common.h"
> > > > +
> > > > +static DEFINE_MUTEX(algs_lock);
> > >
> > > Did you run checkpatch.pl?  I think it encourages you to document what
> > > the lock protects.
> > >
> > Sure. Basically I run checkpatch.py each time. :)
> >
> > # ./scripts/checkpatch.pl 0001-crypto-add-virtio-crypto-driver.patch
> > total: 0 errors, 0 warnings, 1873 lines checked
> >
> > 0001-crypto-add-virtio-crypto-driver.patch has no obvious style problems and
> is ready for submission.
> 
> Looks like a bug in checkpatch.pl:
> 
> # check for spinlock_t definitions without a comment.
> if ($line =~ /^.\s*(struct\s+mutex|spinlock_t)\s+\S+;/ ||
> $line =~ /^.\s*(DEFINE_MUTEX)\s*\(/) {
> my $which = $1;
> if (!ctx_has_comment($first_line, $linenr)) {
> CHK("UNCOMMENTED_DEFINITION",
> "$1 definition without comment\n" . $herecurr);
> }
> }
> 
> Since your mutex definition has the 'static' keyword in front of it
> checkpatch.pl misses it!
> 
Anyway I added the comments. Thanks :)


Regards,
-Gonglei

> Andy: Is this checkpatch.pl behavior intentional?
> 
> Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 0/1] virtio-crypto: add Linux driver

2016-12-01 Thread Gonglei 
v5:
 - add comments for algs_lock and table_lock. [Stefan]
 - use kzfree instead of kfree for key material security. [Stefan]
 - drop unnecessary spin_lock for struct virtio_crypto_ablkcipher_ctx.
 - dynamically allocated memory for iv in order to avoid to do DMA from
   the stack memory in __virtio_crypto_ablkcipher_do_req().
 - add logs for error path in virtio_crypto_alg_validate_key().
 - add lock before calling virtio_break_device() in virtcrypto_update_status()

v4:
 - rework unknow status bit handler by calling virtio_break_device(). [Cornelia]
 - convert space to tab in Kconfig. [Stefan]
 - rename virtio_crypto.c to virtio_crypto_core.c and then make the
   moudle named virtio_crypto.ko for consistency. [Stefan]
 - don't call virtcrypto_dev_stop() on failure path. [Stefan]
 - don't add two empty lines. [Michael]
 - fix possible race by add spin_lock in 
virtio_crypto_alg_ablkcipher_init_session() [Michael and Halil]
 - drop virtcrypto_devmgr_get_first() calling in 
virtio_crypto_ablkcipher_setkey. [Michael]
 - drop superfluous assigned value for virtio_crypto_algs[i].cra_flags
   in virtio_crypto_algs_register(). [Stefan]
 - decrease virtio_crypto_active_devs if calling crypto_register_algs() failed. 
[Stefan]
 - fix some typos here and there. [Stefan]
 - fix missing table_lock usage in virtio_crypto_mgr.c. [Stefan]
 - drop confused comments in virtio_crypto_alg_ablkcipher_init_session()
   for virtqueue_kick(). [Halil]

v3:
 - set cpu affinity when data queues are not equal to the number of online 
cpus. [Michael]
 - add TODO comments for cpu hotplug (changing the relationship of binding 
virtqueue and cpu)
 - use __u32/64 in the config space since the virtio->get() doesn't support 
byte-swap yet. [Michael]
 - drop the whole patch 1 of v2 because the above reason.
 - add VERSION_1 check at the beginning of virtcrypto_probe()
 - s/-1/EPERM/g in virtcrypto_update_status(), don't change err to EFAULT then. 
[Michael]
 - add reset operation before delete the virtqueus. [Micheal]
 - drop an unnecessiry spin_lock calling in virtcrypto_freeze(), avoid possible 
dead lock. [Micheal]
 - redefine parameter alg's type in order to use a cast for it. [Michael]
 - pad all structures to have the same size in one union, and add a member to
   show the union's size in virtio_crypto.h. [Michael]
 - update MAINTAINER file to add virtio-crypto stuff to Michael's entry so that
   the corresponding patches can be CC'ed to Michael as well because the 
virtio-crypto
   doesn't lay in driver/virtio directory. 

The virtio crypto device is a virtual cryptography device
as well as a kind of virtual hardware accelerator for
virtual machines. The encryption anddecryption requests
are placed in the data queue and are ultimately handled by
thebackend crypto accelerators. The second queue is the
control queue used to create or destroy sessions for
symmetric algorithms and will control some advanced features
in the future. The virtio crypto device provides the following
cryptoservices: CIPHER, MAC, HASH, and AEAD.

For more information about virtio-crypto device, please see:
  http://qemu-project.org/Features/VirtioCrypto

For better reviewing, pls see below explaination.

The patch mainly includes five files:

 1) virtio_crypto.h is the header file for virtio-crypto device,
which is based on the virtio-crypto specification. 
 2) virtio_crypto_core.c is the entry of the driver module,
which is similar with other virtio devices, such as virtio-net,
virtio-input etc. 
 3) virtio_crypto_mgr.c is used to manage the virtio
crypto devices in the system. We support up to 32 virtio-crypto
devices currently. I use a global list to store the virtio crypto
devices which refer to Intel QAT driver. Meanwhile, the file
includs the functions of add/del/search/start/stop for virtio
crypto devices.
 4) virtio_crypto_common.h is a private header file for virtio
crypto driver, includes structure definations, and function declarations.
 5) virtio_crypto_algs.c is the realization of algs based on Linux Crypto 
Framwork,
which can register different crypto algorithms. Currently it's only support 
AES-CBC.
The Crypto guys can mainly focus on this file. 


v2:
 - stop doing DMA from the stack, CONFIG_VMAP_STACK=y [Salvatore]
 - convert __virtio32/64 to __le32/64 in virtio_crypto.h
 - remove VIRTIO_CRYPTO_S_STARTED based on the lastest virtio crypto spec.
 - introduces the little edian functions for VIRTIO_1 devices in patch 1.


Gonglei (1):
  crypto: add virtio-crypto driver

 MAINTAINERS  |   9 +
 drivers/crypto/Kconfig   |   2 +
 drivers/crypto/Makefile  |   1 +
 drivers/crypto/virtio/Kconfig|  10 +
 drivers/crypto/virtio/Makefile   |   5 +
 drivers/crypto/virtio/virtio_crypto_algs.c   | 537 +++
 drivers/crypto/virtio/virtio_crypto_common.h | 122 ++
 drivers/crypto/virtio/virtio_crypto_core.c   | 464 +++

RE: [PATCH v4 1/1] crypto: add virtio-crypto driver

2016-12-01 Thread Gonglei (Arei) 
>
> Subject: Re: [PATCH v4 1/1] crypto: add virtio-crypto driver
> 
> On Thu, Dec 01, 2016 at 02:27:19AM +, Gonglei (Arei) wrote:
> > > On Tue, Nov 29, 2016 at 08:48:14PM +0800, Gonglei wrote:
> > > > +static int virtio_crypto_alg_ablkcipher_init_session(
> > > > +   struct virtio_crypto_ablkcipher_ctx *ctx,
> > > > +   uint32_t alg, const uint8_t *key,
> > > > +   unsigned int keylen,
> > > > +   int encrypt)
> > > > +{
> > > > +   struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
> > > > +   unsigned int tmp;
> > > > +   struct virtio_crypto *vcrypto = ctx->vcrypto;
> > > > +   int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT :
> > > VIRTIO_CRYPTO_OP_DECRYPT;
> > > > +   int err;
> > > > +   unsigned int num_out = 0, num_in = 0;
> > > > +
> > > > +   /*
> > > > +* Avoid to do DMA from the stack, switch to using
> > > > +* dynamically-allocated for the key
> > > > +*/
> > > > +   uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC);
> > > > +
> > > > +   if (!cipher_key)
> > > > +   return -ENOMEM;
> > > > +
> > > > +   memcpy(cipher_key, key, keylen);
> > >
> > > Are there any rules on handling key material in the kernel?  This buffer
> > > is just kfreed later.  Do you need to zero it out before freeing it?
> > >
> > Good questions. For kernel crypto core, each cipher request should be freed
> > by skcipher_request_free(): zeroize and free request data structure.
> >
> > I need to use kzfree() for key as well. I'll also check other stuffs. 
> > Thanks.
> >
> > > > +
> > > > +   spin_lock(>ctrl_lock);
> > >
> > > The QAT accelerator driver doesn't spin while talking to the device in
> > > virtio_crypto_alg_ablkcipher_init_session().  I didn't find any other
> > > driver examples in the kernel tree, but this function seems like a
> > > weakness in the virtio-crypto device.
> > >
> > The control queues of virtio-net and virtio-console are also be locked
> > Please see:
> >  __send_control_msg() in virtio_console.c and virtio-net's control queue
> > protected by rtnl lock.
> >
> > I didn't want to protect session creations but the virtqueue's operations
> > like what other virtio devices do.
> >
> > > While QEMU is servicing the create session command this vcpu is blocked.
> > > The QEMU global mutex is held so no other vcpu can enter QEMU and the
> > > QMP monitor is also blocked.
> > >
> > > This is a scalability and performance problem.  Can you look at how QAT
> > > avoids this synchronous session setup?
> >
> > For QAT driver, the session creation is synchronous as well because it's a
> > plain software operation which can be completed ASAP.
> 
> I'm mentioning the vmexit and wait for request completion in a spinlock
> because the same type of issue has been a performance bottleneck with
> virtio guest driver in the past.
> 
> If there is a way to avoid spinning then that would be preferred.  It's
> basically a known anti-pattern for virtio guest drivers.
> 
Oh, sorry for my misunderstanding. ;)

> Could you initialize the session on the host side when the first
> asynchronous data is submitted for encryption/decryption instead of
> during init_session()?
> 
I remember I discuss this problem with Alex two/three moths
ago. In some scenarios, its indeed a performance problem, such as each request 
has
different algorithms or keys in HTTP connections. It's performance will be 
better
if we just use one data virtqueue to pass session and data to the backend at 
one time.

But for the batch cipher operations with the same algorithm, the performance is 
poor,
Because we can't do batch operations for those requests. That's the great 
function
of session operations on the control virtqueue. Refer to the our clients 
requirements
and the existing QAT driver, the scenario is even more in NFV.

As your mention here, It's hard to do this IMO, because the backend can't know
the previous session belongs to which requests if we don't pass the session_id
to the backend.

Regards,
-Gonglei
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/1] crypto: add virtio-crypto driver

2016-12-01 Thread Stefan Hajnoczi 
On Thu, Dec 01, 2016 at 02:27:19AM +, Gonglei (Arei) wrote:
> > On Tue, Nov 29, 2016 at 08:48:14PM +0800, Gonglei wrote:
> > > +static int virtio_crypto_alg_ablkcipher_init_session(
> > > + struct virtio_crypto_ablkcipher_ctx *ctx,
> > > + uint32_t alg, const uint8_t *key,
> > > + unsigned int keylen,
> > > + int encrypt)
> > > +{
> > > + struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
> > > + unsigned int tmp;
> > > + struct virtio_crypto *vcrypto = ctx->vcrypto;
> > > + int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT :
> > VIRTIO_CRYPTO_OP_DECRYPT;
> > > + int err;
> > > + unsigned int num_out = 0, num_in = 0;
> > > +
> > > + /*
> > > +  * Avoid to do DMA from the stack, switch to using
> > > +  * dynamically-allocated for the key
> > > +  */
> > > + uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC);
> > > +
> > > + if (!cipher_key)
> > > + return -ENOMEM;
> > > +
> > > + memcpy(cipher_key, key, keylen);
> > 
> > Are there any rules on handling key material in the kernel?  This buffer
> > is just kfreed later.  Do you need to zero it out before freeing it?
> > 
> Good questions. For kernel crypto core, each cipher request should be freed
> by skcipher_request_free(): zeroize and free request data structure.
> 
> I need to use kzfree() for key as well. I'll also check other stuffs. Thanks. 
> 
> > > +
> > > + spin_lock(>ctrl_lock);
> > 
> > The QAT accelerator driver doesn't spin while talking to the device in
> > virtio_crypto_alg_ablkcipher_init_session().  I didn't find any other
> > driver examples in the kernel tree, but this function seems like a
> > weakness in the virtio-crypto device.
> > 
> The control queues of virtio-net and virtio-console are also be locked
> Please see:
>  __send_control_msg() in virtio_console.c and virtio-net's control queue
> protected by rtnl lock.
> 
> I didn't want to protect session creations but the virtqueue's operations
> like what other virtio devices do.
> 
> > While QEMU is servicing the create session command this vcpu is blocked.
> > The QEMU global mutex is held so no other vcpu can enter QEMU and the
> > QMP monitor is also blocked.
> > 
> > This is a scalability and performance problem.  Can you look at how QAT
> > avoids this synchronous session setup?
> 
> For QAT driver, the session creation is synchronous as well because it's a
> plain software operation which can be completed ASAP.

I'm mentioning the vmexit and wait for request completion in a spinlock
because the same type of issue has been a performance bottleneck with
virtio guest driver in the past.

If there is a way to avoid spinning then that would be preferred.  It's
basically a known anti-pattern for virtio guest drivers.

Could you initialize the session on the host side when the first
asynchronous data is submitted for encryption/decryption instead of
during init_session()?

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v4 1/1] crypto: add virtio-crypto driver

2016-12-01 Thread Stefan Hajnoczi 
On Thu, Dec 01, 2016 at 02:27:19AM +, Gonglei (Arei) wrote:
> > On Tue, Nov 29, 2016 at 08:48:14PM +0800, Gonglei wrote:
> > > diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
> > b/drivers/crypto/virtio/virtio_crypto_algs.c
> > > new file mode 100644
> > > index 000..08b077f
> > > --- /dev/null
> > > +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> > > @@ -0,0 +1,518 @@
> > > + /* Algorithms supported by virtio crypto device
> > > +  *
> > > +  * Authors: Gonglei 
> > > +  *
> > > +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > > +  *
> > > +  * This program is free software; you can redistribute it and/or modify
> > > +  * it under the terms of the GNU General Public License as published by
> > > +  * the Free Software Foundation; either version 2 of the License, or
> > > +  * (at your option) any later version.
> > > +  *
> > > +  * This program is distributed in the hope that it will be useful,
> > > +  * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > +  * GNU General Public License for more details.
> > > +  *
> > > +  * You should have received a copy of the GNU General Public License
> > > +  * along with this program; if not, see .
> > > +  */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +#include "virtio_crypto_common.h"
> > > +
> > > +static DEFINE_MUTEX(algs_lock);
> > 
> > Did you run checkpatch.pl?  I think it encourages you to document what
> > the lock protects.
> > 
> Sure. Basically I run checkpatch.py each time. :)
> 
> # ./scripts/checkpatch.pl 0001-crypto-add-virtio-crypto-driver.patch 
> total: 0 errors, 0 warnings, 1873 lines checked
> 
> 0001-crypto-add-virtio-crypto-driver.patch has no obvious style problems and 
> is ready for submission.

Looks like a bug in checkpatch.pl:

# check for spinlock_t definitions without a comment.
if ($line =~ /^.\s*(struct\s+mutex|spinlock_t)\s+\S+;/ ||
$line =~ /^.\s*(DEFINE_MUTEX)\s*\(/) {
my $which = $1;
if (!ctx_has_comment($first_line, $linenr)) {
CHK("UNCOMMENTED_DEFINITION",
"$1 definition without comment\n" . $herecurr);
}
}

Since your mutex definition has the 'static' keyword in front of it
checkpatch.pl misses it!

Andy: Is this checkpatch.pl behavior intentional?

Stefan


signature.asc
Description: PGP signature