Re: [PATCH] orinoco: Use shash instead of ahash for MIC calculations

2016-12-12 Thread Eric Biggers
On Mon, Dec 12, 2016 at 12:55:55PM -0800, Andy Lutomirski wrote:
> +int orinoco_mic(struct crypto_shash *tfm_michael, u8 *key,
>   u8 *da, u8 *sa, u8 priority,
>   u8 *data, size_t data_len, u8 *mic)
>  {
> - AHASH_REQUEST_ON_STACK(req, tfm_michael);
> - struct scatterlist sg[2];
> + SHASH_DESC_ON_STACK(desc, tfm_michael);
>   u8 hdr[ETH_HLEN + 2]; /* size of header + padding */
>   int err;
>  
> @@ -67,18 +66,27 @@ int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
>   hdr[ETH_ALEN * 2 + 2] = 0;
>   hdr[ETH_ALEN * 2 + 3] = 0;
>  
> - /* Use scatter gather to MIC header and data in one go */
> - sg_init_table(sg, 2);
> - sg_set_buf(&sg[0], hdr, sizeof(hdr));
> - sg_set_buf(&sg[1], data, data_len);
> + desc->tfm = tfm_michael;
> + desc->flags = 0;
>  
> - if (crypto_ahash_setkey(tfm_michael, key, MIC_KEYLEN))
> - return -1;
> + err = crypto_shash_setkey(tfm_michael, key, MIC_KEYLEN);
> + if (err)
> + return err;
> +
> + err = crypto_shash_init(desc);
> + if (err)
> + return err;
> +
> + err = crypto_shash_update(desc, hdr, sizeof(hdr));
> + if (err)
> + return err;
> +
> + err = crypto_shash_update(desc, data, data_len);
> + if (err)
> + return err;
> +
> + err = crypto_shash_final(desc, mic);
> + shash_desc_zero(desc);
>  
> - ahash_request_set_tfm(req, tfm_michael);
> - ahash_request_set_callback(req, 0, NULL, NULL);
> - ahash_request_set_crypt(req, sg, mic, data_len + sizeof(hdr));
> - err = crypto_ahash_digest(req);
> - ahash_request_zero(req);
>   return err;

It's probably a good idea to always do shash_desc_zero(), even when something
above it fails.  Otherwise this looks fine.  Thanks for sending these patches!

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: Make a few drivers depend on !VMAP_STACK

2016-12-12 Thread Herbert Xu
On Mon, Dec 12, 2016 at 12:55:20PM -0800, Andy Lutomirski wrote:
> Eric Biggers found several crypto drivers that point scatterlists at
> the stack.  These drivers should never load on x86, but, for future
> safety, make them depend on !VMAP_STACK.
> 
> No -stable backport should be needed as no released kernel
> configuration should be affected.
> 
> Reported-by: Eric Biggers 
> Signed-off-by: Andy Lutomirski 

Nack.  These drivers are all async and are never used with a
stack request.

Cheers,
-- 
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: Remaining crypto API regressions with CONFIG_VMAP_STACK

2016-12-12 Thread Herbert Xu
On Mon, Dec 12, 2016 at 12:45:18PM -0600, Gary R Hook wrote:
> On 12/12/2016 12:34 PM, Andy Lutomirski wrote:
> 
> <...snip...>
> 
> >
> >I have a patch to make these depend on !VMAP_STACK.
> >
> >>drivers/crypto/ccp/ccp-crypto-aes-cmac.c:105,119,142
> >>drivers/crypto/ccp/ccp-crypto-sha.c:95,109,124
> >>drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
> >>drivers/crypto/ccp/ccp-crypto-aes.c:94
> >
> >According to Herbert, these are fine.  I'm personally less convinced
> >since I'm very confused as to what "async" means in the crypto code,
> >but I'm going to leave these alone.
> 
> I went back through the code, and AFAICT every argument to sg_init_one() in
> the above-cited files is a buffer that is part of the request context. Which
> is allocated by the crypto framework, and therefore will never be on the
> stack.
> Right?

Right.

Cheers,
-- 
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: Remaining crypto API regressions with CONFIG_VMAP_STACK

2016-12-12 Thread Herbert Xu
On Mon, Dec 12, 2016 at 10:34:10AM -0800, Andy Lutomirski wrote:
>
> Here's my status.
> 
> > drivers/crypto/bfin_crc.c:351
> > drivers/crypto/qce/sha.c:299
> > drivers/crypto/sahara.c:973,988
> > drivers/crypto/talitos.c:1910
> > drivers/crypto/qce/sha.c:325
> 
> I have a patch to make these depend on !VMAP_STACK.

Why? They're all marked as ASYNC AFAIK.

> I have a patch to convert this to, drumroll please:
> 
> priv->tx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
>   CRYPTO_ALG_ASYNC);
> 
> Herbert, I'm at a loss as what a "shash" that's "ASYNC" even means.

Having 0 as type and CRYPTO_ALG_ASYNC as mask in general means
that we're requesting a sync algorithm (i.e., ASYNC bit off).

However, it is completely unnecessary for shash as they can never
be async.  So this could be changed to just ("michael_mic", 0, 0).

> > net/ceph/crypto.c:182
> 
> This:
> 
> size_t zero_padding = (0x10 - (src_len & 0x0f));
> 
> is an amazing line of code...
> 
> But this driver uses cbc and wants to do synchronous crypto, and I
> don't think that the crypto API supports real synchronous crypto using
> CBC, so I'm going to let someone else fix this.

It does through skcipher if you allocate with (0, CRYPTO_ALG_ASYNC).

I'll try to fix this.

> > net/rxrpc/rxkad.c:737,1000
> 
> Herbert, can you fix this?

Sure I'll take a look.

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 v6 2/2] crypto: add virtio-crypto driver

2016-12-12 Thread Herbert Xu
On Tue, Dec 13, 2016 at 12:05:19AM +0200, Michael S. Tsirkin wrote:
>
> > Sorry but it's too late for 4.10.  It needed to have been in my
> > tree before the merge window opened to make it for this cycle.
> 
> Objections to me merging this? I'm preparing my tree right now.

Sure feel free to merge it through your tree.

Cheers,
-- 
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 v2 2/2] crypto: mediatek - add DT bindings documentation

2016-12-12 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..47a786e
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
@@ -0,0 +1,32 @@
+MediaTek cryptographic accelerators
+
+Required properties:
+- compatible: Should be "mediatek,eip97-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,eip97-crypto";
+   reg = <0 0x1b24 0 0x2>;
+   interrupts = ,
+,
+,
+,
+;
+   clocks = <&topckgen CLK_TOP_ETHIF_SEL>,
+<ðsys CLK_ETHSYS_CRYPTO>;
+   clock-names = "ethif","cryp";
+   power-domains = <&scpsys 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 v2 1/2] Add crypto driver support for some MediaTek chips

2016-12-12 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 algorithms.
- 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  |  766 +
 drivers/crypto/mediatek/mtk-platform.c |  604 ++
 drivers/crypto/mediatek/mtk-platform.h |  238 ++
 drivers/crypto/mediatek/mtk-regs.h |  194 +
 drivers/crypto/mediatek/mtk-sha.c  | 1437 
 8 files changed, 3259 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..937039d 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 EIP97 Cryptographic Engine driver"
+   depends on ARM && (ARCH_MEDIATEK || COMPILE_TEST)
+   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
+ EIP97 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..aa915c6
--- /dev/null
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -0,0 +1,766 @@
+/*
+ * Cryptographic API.
+ *
+ * Driver for EIP97 AES acceleration.
+ *
+ * Copyright (c) 2016 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 "mtk-platform.h"
+
+#define AES_QUEUE_SIZE 512
+#define AES_BUF_ORDER  2
+#define AES_BUF_SIZE   ((PAGE_SIZE << AES_BUF_ORDER) \
+   & ~(AES_BLOCK_SIZE - 1))
+
+/* AES command token */
+#define AES_CT_SIZE_ECB2
+#define AES_CT_SIZE_CBC3
+#define AES_CT_CTRL_HDRcpu_to_le32(0x0022)
+#define AES_COMMAND0   cpu_to_le32(0x0500)
+#define AES_COMMAND1   cpu_to_le32(0x2d06)
+#define AES_COMMAND2   cpu_to_le32(0xe4a63806)
+
+/* AES transform information */
+#define AES_TFM_ECBcpu_to_le32(0x0 << 0)
+#define AES_TFM_CBCcpu_to_le32(0x1 << 0)
+#define AES_TFM_DECRYPTcpu_to_le32(0x5 << 0)
+#define AES_TFM_ENCRYPTcpu_to_le32(0x4 << 0)
+#define AES_TFM_SIZE(x)cpu_to_le32((x) << 8)
+#define AES_TFM_128BITScpu_to_le32(0xb << 16)
+#define AES_TFM_192BITScpu_to_le32(0xd << 16)
+#define AES_TFM_256BITScpu_to_le32(0xf << 16)
+#define AES_TFM_FULL_IVcpu_to_le32(0xf << 5)
+
+/* AES flags */
+#define AES_FLAGS_MODE_MSK 0x7
+#define AES_FLAGS_ECB  BIT(0)
+#define AES_FLAGS_CBC  BIT(1)
+#define AES_FLAGS_ENCRYPT  BIT(2)
+#define AES_FLAGS_BUSY BIT(3)
+
+/**
+ * mtk_aes_ct is a set of hardware instructions(command token)

[PATCH v2 0/2] Add MediaTek crypto accelerator driver

2016-12-12 Thread Ryder Lee
Hello,

This adds support for the MediaTek hardware accelerator on
some SoCs.

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

Changes since v2:
- use byteorder conversion macros and type identifiers for descriptors
- revise register definition macros to make it more clear
- revise DT compatiable string

Changes since v1:
- remove EXPORT_SYMBOL
- remove unused PRNG setting
- sort headers in alphabetical order
- add a definition for IRQ unmber
- replace ambiguous definition
- add more annotation and function comment
- add COMPILE_TEST in Kconfig

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  |  766 +++
 drivers/crypto/mediatek/mtk-platform.c |  604 
 drivers/crypto/mediatek/mtk-platform.h |  238 
 drivers/crypto/mediatek/mtk-regs.h |  194 +++
 drivers/crypto/mediatek/mtk-sha.c  | 1437 
 9 files changed, 3291 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: [PATCH v6 2/2] crypto: add virtio-crypto driver

2016-12-12 Thread Gonglei (Arei)
>
> Subject: Re: [PATCH v6 2/2] crypto: add virtio-crypto driver
> 
> On Mon, Dec 12, 2016 at 06:54:07PM +0800, Herbert Xu wrote:
> > On Mon, Dec 12, 2016 at 06:25:12AM +, Gonglei (Arei) wrote:
> > > Hi, Michael & Herbert
> > >
> > > Because the virtio-crypto device emulation had been in QEMU 2.8,
> > > would you please merge the virtio-crypto driver for 4.10 if no other
> > > comments? If so, Miachel pls ack and/or review the patch, then
> > > Herbert will take it (I asked him last week). Thank you!
> > >
> > > Ps: Note on 4.10 merge window timing from Linus
> > >  https://lkml.org/lkml/2016/12/7/506
> > >
> > > Dec 23rd is the deadline for 4.10 merge window.
> >
> > Sorry but it's too late for 4.10.  It needed to have been in my
> > tree before the merge window opened to make it for this cycle.
> >
> > Cheers,
> 
> 
> Objections to me merging this? I'm preparing my tree right now.
> 
That's great if so since 4.11 merge window opens 
at least three months later.

Do you agree with it Herbert? Thanks.

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] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-12 Thread Andy Lutomirski
On Mon, Dec 12, 2016 at 2:28 PM, David Howells  wrote:
> Andy Lutomirski  wrote:
>
>> +static const char zero_pad[16] = {0};
>
> Isn't there a global page of zeros or something that we can share?  Also, you
> shouldn't explicitly initialise it so that it stays in .bss.

This is a double-edged sword.  It seems that omitting the
initialization causes it to go in .bss, which isn't read only.  I have
no idea why initializing make a difference at all -- the IMO sensible
behavior would be to put it in .rodata as NOBITS either way.

But I'll use empty_zero_page.

>
>> - sg_set_buf(&sg_out[1], pad, sizeof pad);
>> + sg_set_buf(&sg_out[1], zero_pad, sizeof zero_pad);
>
> Can you put brackets on the sizeof?

Will do for v2.
--
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] wusbcore: Fix one more crypto-on-the-stack bug

2016-12-12 Thread Andy Lutomirski
On Mon, Dec 12, 2016 at 1:44 PM, Greg KH  wrote:
> On Mon, Dec 12, 2016 at 12:52:45PM -0800, Andy Lutomirski wrote:
>> The driver put a constant buffer of all zeros on the stack and
>> pointed a scatterlist entry at it.  This doesn't work with virtual
>> stacks.  Make the buffer static to fix it.
>>
>> Cc: sta...@vger.kernel.org # 4.9 only
>> Reported-by: Eric Biggers 
>> Signed-off-by: Andy Lutomirski 
>> ---
>>  drivers/usb/wusbcore/crypto.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
>> index 79451f7ef1b7..a7e007a0cd49 100644
>> --- a/drivers/usb/wusbcore/crypto.c
>> +++ b/drivers/usb/wusbcore/crypto.c
>> @@ -216,7 +216,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
>>   struct scatterlist sg[4], sg_dst;
>>   void *dst_buf;
>>   size_t dst_size;
>> - const u8 bzero[16] = { 0 };
>> + static const u8 bzero[16] = { 0 };
>
> Hm, can static memory handle DMA?  That's a requirement of the USB
> stack, does this data later end up being sent down to a USB host
> controller?

I think it doesn't, but I'll switch it to use empty_zero_page instead.

--Andy
--
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 v3] siphash: add cryptographically secure hashtable function

2016-12-12 Thread Jason A. Donenfeld
On Tue, Dec 13, 2016 at 12:01 AM, Andi Kleen  wrote:
> It would be nice if the network code could be converted to use siphash
> for the secure sequence numbers. Right now it pulls in a lot of code
> for bigger secure hashes just for that, which is a problem for tiny
> kernels.

Indeed this would be a great first candidate. There are lots of places
where MD5 (!!) is pulled in for this sort of thing, when SipHash could
be a faster and leaner replacement (and arguably more secure than
rusty MD5).
--
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 v3] siphash: add cryptographically secure hashtable function

2016-12-12 Thread Andi Kleen
> Dozens of languages are already using this internally for their hash
> tables. Some of the BSDs already use this in their kernels. SipHash is
> a widely known high-speed solution to a widely known problem, and it's
> time we catch-up.

It would be nice if the network code could be converted to use siphash
for the secure sequence numbers. Right now it pulls in a lot of code
for bigger secure hashes just for that, which is a problem for tiny
kernels.

-Andi
--
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] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-12 Thread David Howells
Andy Lutomirski  wrote:

> +static const char zero_pad[16] = {0};

Isn't there a global page of zeros or something that we can share?  Also, you
shouldn't explicitly initialise it so that it stays in .bss.

> - sg_set_buf(&sg_out[1], pad, sizeof pad);
> + sg_set_buf(&sg_out[1], zero_pad, sizeof zero_pad);

Can you put brackets on the sizeof?

Thanks,
David
--
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 v3] siphash: add cryptographically secure hashtable function

2016-12-12 Thread Jason A. Donenfeld
SipHash is a 64-bit keyed hash function that is actually a
cryptographically secure PRF, like HMAC. Except SipHash is super fast,
and is meant to be used as a hashtable keyed lookup function.

SipHash isn't just some new trendy hash function. It's been around for a
while, and there really isn't anything that comes remotely close to
being useful in the way SipHash is. With that said, why do we need this?

There are a variety of attacks known as "hashtable poisoning" in which an
attacker forms some data such that the hash of that data will be the
same, and then preceeds to fill up all entries of a hashbucket. This is
a realistic and well-known denial-of-service vector.

Linux developers already seem to be aware that this is an issue, and
various places that use hash tables in, say, a network context, use a
non-cryptographically secure function (usually jhash) and then try to
twiddle with the key on a time basis (or in many cases just do nothing
and hope that nobody notices). While this is an admirable attempt at
solving the problem, it doesn't actually fix it. SipHash fixes it.

(It fixes it in such a sound way that you could even build a stream
cipher out of SipHash that would resist the modern cryptanalysis.)

There are a modicum of places in the kernel that are vulnerable to
hashtable poisoning attacks, either via userspace vectors or network
vectors, and there's not a reliable mechanism inside the kernel at the
moment to fix it. The first step toward fixing these issues is actually
getting a secure primitive into the kernel for developers to use. Then
we can, bit by bit, port things over to it as deemed appropriate.

Dozens of languages are already using this internally for their hash
tables. Some of the BSDs already use this in their kernels. SipHash is
a widely known high-speed solution to a widely known problem, and it's
time we catch-up.

Signed-off-by: Jason A. Donenfeld 
Cc: Jean-Philippe Aumasson 
Cc: Daniel J. Bernstein 
---
Changes from v2->v3:

  - The unaligned helpers are now used for reading from u8* arrays.
  - Linus' trick with load_unaligned_zeropad has been implemented for
64-bit/dcache platforms.
  - Non 64-bit/dcache platforms now use a more optimized duff's device
for shortcutting certain sized left-overs.
  - The Kconfig help text for the test now mentions siphash.
  - The function now returns a native-endian byte sequence inside a
u64, which is more correct. As well, the tests vectors are now
represented as u64 literals, rather than byte sequences.
  - The origin of the test vectors is now inside a comment.


 include/linux/siphash.h | 20 +
 lib/Kconfig.debug   |  6 ++--
 lib/Makefile|  5 ++--
 lib/siphash.c   | 75 +
 lib/test_siphash.c  | 74 
 5 files changed, 175 insertions(+), 5 deletions(-)
 create mode 100644 include/linux/siphash.h
 create mode 100644 lib/siphash.c
 create mode 100644 lib/test_siphash.c

diff --git a/include/linux/siphash.h b/include/linux/siphash.h
new file mode 100644
index ..6623b3090645
--- /dev/null
+++ b/include/linux/siphash.h
@@ -0,0 +1,20 @@
+/* Copyright (C) 2016 Jason A. Donenfeld 
+ *
+ * This file is provided under a dual BSD/GPLv2 license.
+ *
+ * SipHash: a fast short-input PRF
+ * https://131002.net/siphash/
+ */
+
+#ifndef _LINUX_SIPHASH_H
+#define _LINUX_SIPHASH_H
+
+#include 
+
+enum siphash_lengths {
+   SIPHASH24_KEY_LEN = 16
+};
+
+u64 siphash24(const u8 *data, size_t len, const u8 key[SIPHASH24_KEY_LEN]);
+
+#endif /* _LINUX_SIPHASH_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a6c8db1d62f6..2a1797704b41 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1823,9 +1823,9 @@ config TEST_HASH
tristate "Perform selftest on hash functions"
default n
help
- Enable this option to test the kernel's integer ()
- and string () hash functions on boot
- (or module load).
+ Enable this option to test the kernel's integer (),
+ string (), and siphash ()
+ hash functions on boot (or module load).
 
  This is intended to help people writing architecture-specific
  optimized versions.  If unsure, say N.
diff --git a/lib/Makefile b/lib/Makefile
index 50144a3aeebd..71d398b04a74 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -22,7 +22,8 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
 sha1.o chacha20.o md5.o irq_regs.o argv_split.o \
 flex_proportions.o ratelimit.o show_mem.o \
 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
-earlycpio.o seq_buf.o nmi_backtrace.o nodemask.o win_minmax.o
+earlycpio.o seq_buf.o siphash.o \
+nmi_backtrace.o nodemask.o win_minmax.o
 
 lib-$(CONFIG_MMU) += ioremap.o
 lib-$(CONFIG_SMP) += cpumask.o
@@ -44,7 +45,7 @@ obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o
 obj-y += kstrtox.o
 obj-$(CON

Re: [PATCH v6 2/2] crypto: add virtio-crypto driver

2016-12-12 Thread Michael S. Tsirkin
On Mon, Dec 12, 2016 at 06:54:07PM +0800, Herbert Xu wrote:
> On Mon, Dec 12, 2016 at 06:25:12AM +, Gonglei (Arei) wrote:
> > Hi, Michael & Herbert
> > 
> > Because the virtio-crypto device emulation had been in QEMU 2.8,
> > would you please merge the virtio-crypto driver for 4.10 if no other
> > comments? If so, Miachel pls ack and/or review the patch, then
> > Herbert will take it (I asked him last week). Thank you!
> > 
> > Ps: Note on 4.10 merge window timing from Linus
> >  https://lkml.org/lkml/2016/12/7/506
> > 
> > Dec 23rd is the deadline for 4.10 merge window.
> 
> Sorry but it's too late for 4.10.  It needed to have been in my
> tree before the merge window opened to make it for this cycle.
> 
> Cheers,


Objections to me merging this? I'm preparing my tree right now.

> -- 
> 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] siphash: add cryptographically secure hashtable function

2016-12-12 Thread Jason A. Donenfeld
On Mon, Dec 12, 2016 at 10:44 PM, Jason A. Donenfeld  wrote:
> #if defined(CONFIG_DCACHE_WORD_ACCESS) && BITS_PER_LONG == 64
>switch (left) {
>case 0: break;
>case 1: b |= data[0]; break;
>case 2: b |= get_unaligned_le16(data); break;
>case 4: b |= get_unaligned_le32(data); break;
>default:
>b |= le64_to_cpu(load_unaligned_zeropad(data) &
> bytemask_from_count(left));
>break;
>}
> #else
>switch (left) {
>case 7: b |= ((u64)data[6]) << 48;
>case 6: b |= ((u64)data[5]) << 40;
>case 5: b |= ((u64)data[4]) << 32;
>case 4: b |= get_unaligned_le32(data); break;
>case 3: b |= ((u64)data[2]) << 16;
>case 2: b |= get_unaligned_le16(data); break;
>case 1: b |= data[0];
>}
> #endif

As it turns out, perhaps unsurprisingly, the code generation here is
really not nice, resulting in many branches instead of a computed
jump. I'll submit v3 with just a branch-less load_unaligned_zeropad
for the 64-bit/dcache case and the duff's device for the other case.
--
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] wusbcore: Fix one more crypto-on-the-stack bug

2016-12-12 Thread Greg KH
On Mon, Dec 12, 2016 at 12:52:45PM -0800, Andy Lutomirski wrote:
> The driver put a constant buffer of all zeros on the stack and
> pointed a scatterlist entry at it.  This doesn't work with virtual
> stacks.  Make the buffer static to fix it.
> 
> Cc: sta...@vger.kernel.org # 4.9 only
> Reported-by: Eric Biggers 
> Signed-off-by: Andy Lutomirski 
> ---
>  drivers/usb/wusbcore/crypto.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
> index 79451f7ef1b7..a7e007a0cd49 100644
> --- a/drivers/usb/wusbcore/crypto.c
> +++ b/drivers/usb/wusbcore/crypto.c
> @@ -216,7 +216,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
>   struct scatterlist sg[4], sg_dst;
>   void *dst_buf;
>   size_t dst_size;
> - const u8 bzero[16] = { 0 };
> + static const u8 bzero[16] = { 0 };

Hm, can static memory handle DMA?  That's a requirement of the USB
stack, does this data later end up being sent down to a USB host
controller?

thanks,

greg k-h
--
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] siphash: add cryptographically secure hashtable function

2016-12-12 Thread Jason A. Donenfeld
Hi Linus,

> I guess you could try to just remove the "if (left)" test entirely, if
> it is at least partly the mispredict. It should do the right thing
> even with a zero count, and it might schedule the code better. Code
> size _should_ be better with the byte mask model (which won't matter
> in the hot loop example, since it will all be cached, possibly even in
> the uop cache for really tight benchmark loops).

Originally I had just forgotten the `if (left)`, and had the same
sub-par benchmarks. In the v3 revision that I'm working on at the
moment, I'm using your dcache trick for cases 3,5,6,7 and
short-circuiting cases 1,2,4 to just directly access those bytes as
integers. For the 32-bit case, I do something similar, but built
inside of the duff's device. This should give optimal performance for
the most popular use cases, which involve hashing "some stuff" plus a
leftover u16 (port number?) or u32 (ipv4 addr?).

#if defined(CONFIG_DCACHE_WORD_ACCESS) && BITS_PER_LONG == 64
   switch (left) {
   case 0: break;
   case 1: b |= data[0]; break;
   case 2: b |= get_unaligned_le16(data); break;
   case 4: b |= get_unaligned_le32(data); break;
   default:
   b |= le64_to_cpu(load_unaligned_zeropad(data) &
bytemask_from_count(left));
   break;
   }
#else
   switch (left) {
   case 7: b |= ((u64)data[6]) << 48;
   case 6: b |= ((u64)data[5]) << 40;
   case 5: b |= ((u64)data[4]) << 32;
   case 4: b |= get_unaligned_le32(data); break;
   case 3: b |= ((u64)data[2]) << 16;
   case 2: b |= get_unaligned_le16(data); break;
   case 1: b |= data[0];
   }
#endif

It seems like this might be best of all worlds?

Jason
--
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] siphash: add cryptographically secure hashtable function

2016-12-12 Thread Linus Torvalds
On Sun, Dec 11, 2016 at 9:48 PM, Jason A. Donenfeld  wrote:
> I modified the test to hash data of size 0 through 7 repeatedly
> 1 times, and benchmarked that a few times on a Skylake laptop.
> The `load_unaligned_zeropad & bytemask_from_count` version was
> consistently 7% slower.
>
> I then modified it again to simply hash a 4 byte constant repeatedly
> 10 times. The `load_unaligned_zeropad & bytemask_from_count`
> version was around 6% faster. I tried again with a 7 byte constant and
> got more or less a similar result.
>
> Then I tried with a 1 byte constant, and found that the
> `load_unaligned_zeropad & bytemask_from_count` version was slower.
>
> So, it would seem that between the `if (left)` and the `switch
> (left)`, there's the same number of branches.

Interesting.

For the dcache code (which is where that trick comes from), we used to
have a loop (rather than the duff's device thing), and it performed
badly due to the consistently badly predicted branch of the loop. But
I never compared it against the duff's device version.

I guess you could try to just remove the "if (left)" test entirely, if
it is at least partly the mispredict. It should do the right thing
even with a zero count, and it might schedule the code better. Code
size _should_ be better with the byte mask model (which won't matter
in the hot loop example, since it will all be cached, possibly even in
the uop cache for really tight benchmark loops).

 Linus
--
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] siphash: add cryptographically secure hashtable function

2016-12-12 Thread Jason A. Donenfeld
Hey Eric,

Lots of good points; thanks for the review. Responses are inline below.

On Mon, Dec 12, 2016 at 6:42 AM, Eric Biggers  wrote:
> Maybe add to the help text for CONFIG_TEST_HASH that it now tests siphash too?

Good call. Will do.

> This assumes the key and message buffers are aligned to __alignof__(u64).
> Unless that's going to be a clearly documented requirement for callers, you
> should use get_unaligned_le64() instead.  And you can pass a 'u8 *' directly 
> to
> get_unaligned_le64(), no need for a helper function.

I had thought about that briefly, but just sort of figured most people
were passing in aligned variables... but that's a pretty bad
assumption to make especially for 64-bit alignment. I'll switch to
using the get_unaligned functions.

[As a side note, I wonder if crypto/chacha20_generic.c should be using
the unaligned functions instead too, at least for the iv reading...]

> It makes sense for this to return a u64, but that means the cpu_to_le64() is
> wrong, since u64 indicates CPU endianness.  It should just return 'b'.

At first I was very opposed to making this change, since by returning
a value with an explicit byte order, you can cast to u8 and have
uniform indexed byte access across platforms. But of course this
doesn't make any sense, since it's returning a u64, and it makes all
other bitwise operations non-uniform anyway.  I checked with JP
(co-creator of siphash, CC'd) and he confirmed your suspicion that it
was just to make the test vector comparison easier and for some
byte-wise uniformity, but that it's not strictly necessary. So, I've
removed that last cpu_to_le64, and I've also refactored those test
vectors to be written as ULL literals, so that a simple == integer
comparison will work across platforms.

> Can you mention in a comment where the test vectors came from?

Sure, will do.


> If you make the output really be CPU-endian like I'm suggesting then this will
> need to be something like:
>
> if (out != get_unaligned_le64(test_vectors[i])) {
>
> Or else make the test vectors be an array of u64.

Yep, I wound up doing that.

Thanks Eric! Will submit a v3 soon if nobody else has comments.

Jason
--
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] orinoco: Use shash instead of ahash for MIC calculations

2016-12-12 Thread Andy Lutomirski
Eric Biggers pointed out that the orinoco driver pointed scatterlists
at the stack.

Fix it by switching from ahash to shash.  The result should be
simpler, faster, and more correct.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers 
Signed-off-by: Andy Lutomirski 
---

Compile-tested only.

drivers/net/wireless/intersil/orinoco/mic.c | 44 +++--
 drivers/net/wireless/intersil/orinoco/mic.h |  3 +-
 drivers/net/wireless/intersil/orinoco/orinoco.h |  4 +--
 3 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/mic.c 
b/drivers/net/wireless/intersil/orinoco/mic.c
index bc7397d709d3..08bc7822f820 100644
--- a/drivers/net/wireless/intersil/orinoco/mic.c
+++ b/drivers/net/wireless/intersil/orinoco/mic.c
@@ -16,7 +16,7 @@
 //
 int orinoco_mic_init(struct orinoco_private *priv)
 {
-   priv->tx_tfm_mic = crypto_alloc_ahash("michael_mic", 0,
+   priv->tx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
  CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->tx_tfm_mic)) {
printk(KERN_DEBUG "orinoco_mic_init: could not allocate "
@@ -25,7 +25,7 @@ int orinoco_mic_init(struct orinoco_private *priv)
return -ENOMEM;
}
 
-   priv->rx_tfm_mic = crypto_alloc_ahash("michael_mic", 0,
+   priv->rx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
  CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->rx_tfm_mic)) {
printk(KERN_DEBUG "orinoco_mic_init: could not allocate "
@@ -40,17 +40,16 @@ int orinoco_mic_init(struct orinoco_private *priv)
 void orinoco_mic_free(struct orinoco_private *priv)
 {
if (priv->tx_tfm_mic)
-   crypto_free_ahash(priv->tx_tfm_mic);
+   crypto_free_shash(priv->tx_tfm_mic);
if (priv->rx_tfm_mic)
-   crypto_free_ahash(priv->rx_tfm_mic);
+   crypto_free_shash(priv->rx_tfm_mic);
 }
 
-int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
+int orinoco_mic(struct crypto_shash *tfm_michael, u8 *key,
u8 *da, u8 *sa, u8 priority,
u8 *data, size_t data_len, u8 *mic)
 {
-   AHASH_REQUEST_ON_STACK(req, tfm_michael);
-   struct scatterlist sg[2];
+   SHASH_DESC_ON_STACK(desc, tfm_michael);
u8 hdr[ETH_HLEN + 2]; /* size of header + padding */
int err;
 
@@ -67,18 +66,27 @@ int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
hdr[ETH_ALEN * 2 + 2] = 0;
hdr[ETH_ALEN * 2 + 3] = 0;
 
-   /* Use scatter gather to MIC header and data in one go */
-   sg_init_table(sg, 2);
-   sg_set_buf(&sg[0], hdr, sizeof(hdr));
-   sg_set_buf(&sg[1], data, data_len);
+   desc->tfm = tfm_michael;
+   desc->flags = 0;
 
-   if (crypto_ahash_setkey(tfm_michael, key, MIC_KEYLEN))
-   return -1;
+   err = crypto_shash_setkey(tfm_michael, key, MIC_KEYLEN);
+   if (err)
+   return err;
+
+   err = crypto_shash_init(desc);
+   if (err)
+   return err;
+
+   err = crypto_shash_update(desc, hdr, sizeof(hdr));
+   if (err)
+   return err;
+
+   err = crypto_shash_update(desc, data, data_len);
+   if (err)
+   return err;
+
+   err = crypto_shash_final(desc, mic);
+   shash_desc_zero(desc);
 
-   ahash_request_set_tfm(req, tfm_michael);
-   ahash_request_set_callback(req, 0, NULL, NULL);
-   ahash_request_set_crypt(req, sg, mic, data_len + sizeof(hdr));
-   err = crypto_ahash_digest(req);
-   ahash_request_zero(req);
return err;
 }
diff --git a/drivers/net/wireless/intersil/orinoco/mic.h 
b/drivers/net/wireless/intersil/orinoco/mic.h
index ce731d05cc98..e8724e889219 100644
--- a/drivers/net/wireless/intersil/orinoco/mic.h
+++ b/drivers/net/wireless/intersil/orinoco/mic.h
@@ -6,6 +6,7 @@
 #define _ORINOCO_MIC_H_
 
 #include 
+#include 
 
 #define MICHAEL_MIC_LEN 8
 
@@ -15,7 +16,7 @@ struct crypto_ahash;
 
 int orinoco_mic_init(struct orinoco_private *priv);
 void orinoco_mic_free(struct orinoco_private *priv);
-int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
+int orinoco_mic(struct crypto_shash *tfm_michael, u8 *key,
u8 *da, u8 *sa, u8 priority,
u8 *data, size_t data_len, u8 *mic);
 
diff --git a/drivers/net/wireless/intersil/orinoco/orinoco.h 
b/drivers/net/wireless/intersil/orinoco/orinoco.h
index 2f0c84b1c440..5fa1c3e3713f 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco.h
+++ b/drivers/net/wireless/intersil/orinoco/orinoco.h
@@ -152,8 +152,8 @@ struct orinoco_private {
u8 *wpa_ie;
int wpa_ie_len;
 
-   struct crypto_ahash *rx_tfm_mic;
-   struct crypto_ahash *tx_tfm_mic;
+   struct crypto_shash *rx_tfm_mic;
+   struct crypto_shash *tx_tfm_mic;
 
unsigned int w

[PATCH] crypto: Make a few drivers depend on !VMAP_STACK

2016-12-12 Thread Andy Lutomirski
Eric Biggers found several crypto drivers that point scatterlists at
the stack.  These drivers should never load on x86, but, for future
safety, make them depend on !VMAP_STACK.

No -stable backport should be needed as no released kernel
configuration should be affected.

Reported-by: Eric Biggers 
Signed-off-by: Andy Lutomirski 
---
 drivers/crypto/Kconfig | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f2b223..481e67e54ffd 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -245,7 +245,7 @@ config CRYPTO_DEV_TALITOS
select CRYPTO_BLKCIPHER
select CRYPTO_HASH
select HW_RANDOM
-   depends on FSL_SOC
+   depends on FSL_SOC && !VMAP_STACK
help
  Say 'Y' here to use the Freescale Security Engine (SEC)
  to offload cryptographic algorithm computation.
@@ -357,7 +357,7 @@ config CRYPTO_DEV_PICOXCELL
 
 config CRYPTO_DEV_SAHARA
tristate "Support for SAHARA crypto accelerator"
-   depends on ARCH_MXC && OF
+   depends on ARCH_MXC && OF && !VMAP_STACK
select CRYPTO_BLKCIPHER
select CRYPTO_AES
select CRYPTO_ECB
@@ -410,7 +410,7 @@ endif # if CRYPTO_DEV_UX500
 
 config CRYPTO_DEV_BFIN_CRC
tristate "Support for Blackfin CRC hardware"
-   depends on BF60x
+   depends on BF60x && !VMAP_STACK
help
  Newer Blackfin processors have CRC hardware. Select this if you
  want to use the Blackfin CRC module.
@@ -487,7 +487,7 @@ source "drivers/crypto/qat/Kconfig"
 
 config CRYPTO_DEV_QCE
tristate "Qualcomm crypto engine accelerator"
-   depends on (ARCH_QCOM || COMPILE_TEST) && HAS_DMA && HAS_IOMEM
+   depends on (ARCH_QCOM || COMPILE_TEST) && HAS_DMA && HAS_IOMEM && 
!VMAP_STACK
select CRYPTO_AES
select CRYPTO_DES
select CRYPTO_ECB
-- 
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


[PATCH] cifs: Fix smbencrypt() to stop pointing a scatterlist at the stack

2016-12-12 Thread Andy Lutomirski
smbencrypt() points a scatterlist to the stack, which is breaks if
CONFIG_VMAP_STACK=y.

Fix it by switching to crypto_cipher_encrypt_one().  The new code
should be considerably faster as an added benefit.

This code is nearly identical to some code that Eric Biggers
suggested.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers 
Signed-off-by: Andy Lutomirski 
---

Compile-tested only.

fs/cifs/smbencrypt.c | 40 
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/fs/cifs/smbencrypt.c b/fs/cifs/smbencrypt.c
index 699b7868108f..c12bffefa3c9 100644
--- a/fs/cifs/smbencrypt.c
+++ b/fs/cifs/smbencrypt.c
@@ -23,7 +23,7 @@
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 */
 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -69,46 +69,22 @@ str_to_key(unsigned char *str, unsigned char *key)
 static int
 smbhash(unsigned char *out, const unsigned char *in, unsigned char *key)
 {
-   int rc;
unsigned char key2[8];
-   struct crypto_skcipher *tfm_des;
-   struct scatterlist sgin, sgout;
-   struct skcipher_request *req;
+   struct crypto_cipher *tfm_des;
 
str_to_key(key, key2);
 
-   tfm_des = crypto_alloc_skcipher("ecb(des)", 0, CRYPTO_ALG_ASYNC);
+   tfm_des = crypto_alloc_cipher("des", 0, 0);
if (IS_ERR(tfm_des)) {
-   rc = PTR_ERR(tfm_des);
-   cifs_dbg(VFS, "could not allocate des crypto API\n");
-   goto smbhash_err;
-   }
-
-   req = skcipher_request_alloc(tfm_des, GFP_KERNEL);
-   if (!req) {
-   rc = -ENOMEM;
cifs_dbg(VFS, "could not allocate des crypto API\n");
-   goto smbhash_free_skcipher;
+   return PTR_ERR(tfm_des);
}
 
-   crypto_skcipher_setkey(tfm_des, key2, 8);
-
-   sg_init_one(&sgin, in, 8);
-   sg_init_one(&sgout, out, 8);
+   crypto_cipher_setkey(tfm_des, key2, 8);
+   crypto_cipher_encrypt_one(tfm_des, out, in);
+   crypto_free_cipher(tfm_des);
 
-   skcipher_request_set_callback(req, 0, NULL, NULL);
-   skcipher_request_set_crypt(req, &sgin, &sgout, 8, NULL);
-
-   rc = crypto_skcipher_encrypt(req);
-   if (rc)
-   cifs_dbg(VFS, "could not encrypt crypt key rc: %d\n", rc);
-
-   skcipher_request_free(req);
-
-smbhash_free_skcipher:
-   crypto_free_skcipher(tfm_des);
-smbhash_err:
-   return rc;
+   return 0;
 }
 
 static int
-- 
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


[PATCH] wusbcore: Fix one more crypto-on-the-stack bug

2016-12-12 Thread Andy Lutomirski
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it.  This doesn't work with virtual
stacks.  Make the buffer static to fix it.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers 
Signed-off-by: Andy Lutomirski 
---
 drivers/usb/wusbcore/crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
index 79451f7ef1b7..a7e007a0cd49 100644
--- a/drivers/usb/wusbcore/crypto.c
+++ b/drivers/usb/wusbcore/crypto.c
@@ -216,7 +216,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
struct scatterlist sg[4], sg_dst;
void *dst_buf;
size_t dst_size;
-   const u8 bzero[16] = { 0 };
+   static const u8 bzero[16] = { 0 };
u8 iv[crypto_skcipher_ivsize(tfm_cbc)];
size_t zero_padding;
 
-- 
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


[PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-12 Thread Andy Lutomirski
The driver put a constant buffer of all zeros on the stack and
pointed a scatterlist entry at it in two places.  This doesn't work
with virtual stacks.  Use a static 16-byte buffer of zeros instead.

Cc: sta...@vger.kernel.org # 4.9 only
Reported-by: Eric Biggers 
Signed-off-by: Andy Lutomirski 
---
 security/keys/encrypted-keys/encrypted.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 17a06105ccb6..fab2fb864002 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -46,6 +46,7 @@ static const char key_format_default[] = "default";
 static const char key_format_ecryptfs[] = "ecryptfs";
 static unsigned int ivsize;
 static int blksize;
+static const char zero_pad[16] = {0};
 
 #define KEY_TRUSTED_PREFIX_LEN (sizeof (KEY_TRUSTED_PREFIX) - 1)
 #define KEY_USER_PREFIX_LEN (sizeof (KEY_USER_PREFIX) - 1)
@@ -481,7 +482,6 @@ static int derived_key_encrypt(struct encrypted_key_payload 
*epayload,
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
unsigned int padlen;
-   char pad[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -493,11 +493,10 @@ static int derived_key_encrypt(struct 
encrypted_key_payload *epayload,
goto out;
dump_decrypted_data(epayload);
 
-   memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 2);
sg_set_buf(&sg_in[0], epayload->decrypted_data,
   epayload->decrypted_datalen);
-   sg_set_buf(&sg_in[1], pad, padlen);
+   sg_set_buf(&sg_in[1], zero_pad, padlen);
 
sg_init_table(sg_out, 1);
sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen);
@@ -584,7 +583,6 @@ static int derived_key_decrypt(struct encrypted_key_payload 
*epayload,
struct skcipher_request *req;
unsigned int encrypted_datalen;
u8 iv[AES_BLOCK_SIZE];
-   char pad[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -594,13 +592,12 @@ static int derived_key_decrypt(struct 
encrypted_key_payload *epayload,
goto out;
dump_encrypted_data(epayload, encrypted_datalen);
 
-   memset(pad, 0, sizeof pad);
sg_init_table(sg_in, 1);
sg_init_table(sg_out, 2);
sg_set_buf(sg_in, epayload->encrypted_data, encrypted_datalen);
sg_set_buf(&sg_out[0], epayload->decrypted_data,
   epayload->decrypted_datalen);
-   sg_set_buf(&sg_out[1], pad, sizeof pad);
+   sg_set_buf(&sg_out[1], zero_pad, sizeof zero_pad);
 
memcpy(iv, epayload->iv, sizeof(iv));
skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
-- 
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: Remaining crypto API regressions with CONFIG_VMAP_STACK

2016-12-12 Thread Gary R Hook

On 12/12/2016 12:34 PM, Andy Lutomirski wrote:

<...snip...>



I have a patch to make these depend on !VMAP_STACK.


drivers/crypto/ccp/ccp-crypto-aes-cmac.c:105,119,142
drivers/crypto/ccp/ccp-crypto-sha.c:95,109,124
drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
drivers/crypto/ccp/ccp-crypto-aes.c:94


According to Herbert, these are fine.  I'm personally less convinced
since I'm very confused as to what "async" means in the crypto code,
but I'm going to leave these alone.


I went back through the code, and AFAICT every argument to sg_init_one() in
the above-cited files is a buffer that is part of the request context. Which
is allocated by the crypto framework, and therefore will never be on the 
stack.

Right?

I don't (as yet) see a need for any patch to these. Someone correct me 
if I'm

missing something.

<...snip...>

--
This is my day job. Follow me at:
IG/Twitter/Facebook: @grhookphoto
IG/Twitter/Facebook: @grhphotographer
--
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: Remaining crypto API regressions with CONFIG_VMAP_STACK

2016-12-12 Thread Andy Lutomirski
On Fri, Dec 9, 2016 at 3:08 PM, Eric Biggers  wrote:
> In the 4.9 kernel, virtually-mapped stacks will be supported and enabled by
> default on x86_64.  This has been exposing a number of problems in which
> on-stack buffers are being passed into the crypto API, which to support crypto
> accelerators operates on 'struct page' rather than on virtual memory.

Here's my status.

> drivers/crypto/bfin_crc.c:351
> drivers/crypto/qce/sha.c:299
> drivers/crypto/sahara.c:973,988
> drivers/crypto/talitos.c:1910
> drivers/crypto/qce/sha.c:325

I have a patch to make these depend on !VMAP_STACK.

> drivers/crypto/ccp/ccp-crypto-aes-cmac.c:105,119,142
> drivers/crypto/ccp/ccp-crypto-sha.c:95,109,124
> drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
> drivers/crypto/ccp/ccp-crypto-aes.c:94

According to Herbert, these are fine.  I'm personally less convinced
since I'm very confused as to what "async" means in the crypto code,
but I'm going to leave these alone.

>
> And these other places do crypto operations on buffers clearly on the stack:
>
> drivers/usb/wusbcore/crypto.c:264
> security/keys/encrypted-keys/encrypted.c:500

I have a patch.

> drivers/net/wireless/intersil/orinoco/mic.c:72

I have a patch to convert this to, drumroll please:

priv->tx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
  CRYPTO_ALG_ASYNC);

Herbert, I'm at a loss as what a "shash" that's "ASYNC" even means.

> net/ceph/crypto.c:182

This:

size_t zero_padding = (0x10 - (src_len & 0x0f));

is an amazing line of code...

But this driver uses cbc and wants to do synchronous crypto, and I
don't think that the crypto API supports real synchronous crypto using
CBC, so I'm going to let someone else fix this.

> net/rxrpc/rxkad.c:737,1000

Herbert, can you fix this?

> fs/cifs/smbencrypt.c:96

I have a patch.


My pile is here:

https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/log/?h=crypto

I'll send out the patches soon.
--
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: arm64/aes: reimplement bit-sliced ARM/NEON implementation for arm64

2016-12-12 Thread Ard Biesheuvel
This is a reimplementation of the NEON version of the bit-sliced AES
algorithm. This code is heavily based on Andy Polyakov's OpenSSL version
for ARM, which is also available in the kernel. This is an alternative for
the existing NEON implementation for arm64 authored by me, which suffers
from poor performance due to its reliance on the pathologically slow four
register variant of the tbl/tbx NEON instruction.

This version is about ~30% (*) faster than the generic C code, but only in
cases where the input can be 8x interleaved (this is a fundamental property
of bit slicing). For this reason, only the chaining modes ECB, XTS and CTR
are implemented. (The significance of ECB is that it could potentially be
used by other chaining modes)

* Measured on Cortex-A57. Note that this is still an order of magnitude
  slower than the implementations that use the dedicated AES instructions
  introduced in ARMv8, but those are part of an optional extension, and so
  it is good to have a fallback.

Signed-off-by: Ard Biesheuvel 
---
 arch/arm64/crypto/Kconfig   |   6 +
 arch/arm64/crypto/Makefile  |   3 +
 arch/arm64/crypto/aes-neonbs-core.S | 905 
 arch/arm64/crypto/aes-neonbs-glue.c | 300 
 4 files changed, 1214 insertions(+)
 create mode 100644 arch/arm64/crypto/aes-neonbs-core.S
 create mode 100644 arch/arm64/crypto/aes-neonbs-glue.c

diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index 450a85df041a..cd0e7a6146b7 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -72,4 +72,10 @@ config CRYPTO_CRC32_ARM64
depends on ARM64
select CRYPTO_HASH
 
+config CRYPTO_AES_NEON_BS
+   tristate "AES in ECB/CBC/CTR/XTS modes using bit-sliced NEON algorithm"
+   depends on KERNEL_MODE_NEON
+   select CRYPTO_BLKCIPHER
+   select CRYPTO_AES
+
 endif
diff --git a/arch/arm64/crypto/Makefile b/arch/arm64/crypto/Makefile
index aad7b744..11d20714ec48 100644
--- a/arch/arm64/crypto/Makefile
+++ b/arch/arm64/crypto/Makefile
@@ -41,6 +41,9 @@ sha256-arm64-y := sha256-glue.o sha256-core.o
 obj-$(CONFIG_CRYPTO_SHA512_ARM64) += sha512-arm64.o
 sha512-arm64-y := sha512-glue.o sha512-core.o
 
+obj-$(CONFIG_CRYPTO_AES_NEON_BS) += aes-neon-bs.o
+aes-neon-bs-y := aes-neonbs-core.o aes-neonbs-glue.o
+
 AFLAGS_aes-ce.o:= -DINTERLEAVE=4
 AFLAGS_aes-neon.o  := -DINTERLEAVE=4
 
diff --git a/arch/arm64/crypto/aes-neonbs-core.S 
b/arch/arm64/crypto/aes-neonbs-core.S
new file mode 100644
index ..d027c276cc75
--- /dev/null
+++ b/arch/arm64/crypto/aes-neonbs-core.S
@@ -0,0 +1,905 @@
+/*
+ * Bit sliced AES using NEON instructions
+ *
+ * Copyright (C) 2016 Linaro Ltd 
+ *
+ * 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.
+ */
+
+/*
+ * The algorithm implemented here is described in detail by the paper
+ * 'Faster and Timing-Attack Resistant AES-GCM' by Emilia Kaesper and
+ * Peter Schwabe (https://eprint.iacr.org/2009/129.pdf)
+ *
+ * This implementation is based primarily on the OpenSSL implementation
+ * for 32-bit ARM written by Andy Polyakov 
+ */
+
+#include 
+#include 
+
+   .text
+
+   rounds  .reqx11
+   bskey   .reqx12
+
+   .macro  in_bs_ch, b0, b1, b2, b3, b4, b5, b6, b7
+   eor \b2, \b2, \b1
+   eor \b5, \b5, \b6
+   eor \b3, \b3, \b0
+   eor \b6, \b6, \b2
+   eor \b5, \b5, \b0
+   eor \b6, \b6, \b3
+   eor \b3, \b3, \b7
+   eor \b7, \b7, \b5
+   eor \b3, \b3, \b4
+   eor \b4, \b4, \b5
+   eor \b2, \b2, \b7
+   eor \b3, \b3, \b1
+   eor \b1, \b1, \b5
+   .endm
+
+   .macro  out_bs_ch, b0, b1, b2, b3, b4, b5, b6, b7
+   eor \b0, \b0, \b6
+   eor \b1, \b1, \b4
+   eor \b4, \b4, \b6
+   eor \b2, \b2, \b0
+   eor \b6, \b6, \b1
+   eor \b1, \b1, \b5
+   eor \b5, \b5, \b3
+   eor \b3, \b3, \b7
+   eor \b7, \b7, \b5
+   eor \b2, \b2, \b5
+   eor \b4, \b4, \b7
+   .endm
+
+   .macro  inv_in_bs_ch, b6, b1, b2, b4, b7, b0, b3, b5
+   eor \b1, \b1, \b7
+   eor \b4, \b4, \b7
+   eor \b7, \b7, \b5
+   eor \b1, \b1, \b3
+   eor \b2, \b2, \b5
+   eor \b3, \b3, \b7
+   eor \b6, \b6, \b1
+   eor \b2, \b2, \b0
+   eor \b5, \b5, \b3
+   eor \b4, \b4, \b6
+   eor \b0, \b0, \b6
+   eor \b1, \b1, \b4
+   .endm
+

Re: [PATCH 1/1] crypto: asymmetric_keys: set error code on failure

2016-12-12 Thread David Howells
Pan Bian  wrote:

>   outlen = crypto_akcipher_maxsize(tfm);
>   output = kmalloc(outlen, GFP_KERNEL);
> - if (!output)
> + if (!output) {
> + ret = -ENOMEM;
>   goto error_free_req;
> + }

This is preferred:

+   ret = -ENOMEM;
outlen = crypto_akcipher_maxsize(tfm);
output = kmalloc(outlen, GFP_KERNEL);
if (!output)
goto error_free_req;

I'll alter your patch.

David
--
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


[RFC PATCH 1/3] crypto: zip - Add ThunderX ZIP driver core

2016-12-12 Thread Jan Glauber
From: Mahipal Challa 

Add a driver for the ZIP engine found on Cavium ThunderX SOCs.
The ZIP engine supports hardware accelerated compression and
decompression. It includes 2 independent ZIP cores and supports:

- DEFLATE compression and decompression (RFC 1951)
- LZS compression and decompression (RFC 2395 and ANSI X3.241-1994)
- ADLER32 and CRC32 checksums for ZLIB (RFC 1950) and GZIP (RFC 1952)

The ZIP engine is presented as a PCI device. It supports DMA and
scatter-gather.

Signed-off-by: Mahipal Challa 
Signed-off-by: Vishnu Nair 
Signed-off-by: Jan Glauber 
---
 drivers/crypto/Kconfig |7 +
 drivers/crypto/Makefile|1 +
 drivers/crypto/cavium/Makefile |4 +
 drivers/crypto/cavium/zip/Makefile |8 +
 drivers/crypto/cavium/zip/common.h |  258 +++
 drivers/crypto/cavium/zip/zip_crypto.h |   61 ++
 drivers/crypto/cavium/zip/zip_device.c |  208 +
 drivers/crypto/cavium/zip/zip_device.h |  138 
 drivers/crypto/cavium/zip/zip_main.c   |  500 
 drivers/crypto/cavium/zip/zip_main.h   |  126 +++
 drivers/crypto/cavium/zip/zip_mem.c|  120 +++
 drivers/crypto/cavium/zip/zip_mem.h|   78 ++
 drivers/crypto/cavium/zip/zip_regs.h   | 1326 
 13 files changed, 2835 insertions(+)
 create mode 100644 drivers/crypto/cavium/Makefile
 create mode 100644 drivers/crypto/cavium/zip/Makefile
 create mode 100644 drivers/crypto/cavium/zip/common.h
 create mode 100644 drivers/crypto/cavium/zip/zip_crypto.h
 create mode 100644 drivers/crypto/cavium/zip/zip_device.c
 create mode 100644 drivers/crypto/cavium/zip/zip_device.h
 create mode 100644 drivers/crypto/cavium/zip/zip_main.c
 create mode 100644 drivers/crypto/cavium/zip/zip_main.h
 create mode 100644 drivers/crypto/cavium/zip/zip_mem.c
 create mode 100644 drivers/crypto/cavium/zip/zip_mem.h
 create mode 100644 drivers/crypto/cavium/zip/zip_regs.h

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f..da48d93 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -485,6 +485,13 @@ config CRYPTO_DEV_MXS_DCP
 
 source "drivers/crypto/qat/Kconfig"
 
+config CRYPTO_DEV_CAVIUM_ZIP
+   tristate "Cavium ZIP driver"
+   depends on PCI && 64BIT && (ARM64 || COMPILE_TEST)
+   ---help---
+ Select this option if you want to enable compression/decompression
+ acceleration on Cavium's ARM based SoCs
+
 config CRYPTO_DEV_QCE
tristate "Qualcomm crypto engine accelerator"
depends on (ARCH_QCOM || COMPILE_TEST) && HAS_DMA && HAS_IOMEM
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index ad7250f..3d152d4 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_CRYPTO_DEV_MXC_SCC) += mxc-scc.o
 obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
 obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
 obj-$(CONFIG_CRYPTO_DEV_QAT) += qat/
+obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += cavium/
 obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
 obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
 obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
diff --git a/drivers/crypto/cavium/Makefile b/drivers/crypto/cavium/Makefile
new file mode 100644
index 000..641268b
--- /dev/null
+++ b/drivers/crypto/cavium/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for Cavium crypto device drivers
+#
+obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += zip/
diff --git a/drivers/crypto/cavium/zip/Makefile 
b/drivers/crypto/cavium/zip/Makefile
new file mode 100644
index 000..2c07508
--- /dev/null
+++ b/drivers/crypto/cavium/zip/Makefile
@@ -0,0 +1,8 @@
+#
+# Makefile for Cavium's ZIP Driver.
+#
+
+obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += thunderx_zip.o
+thunderx_zip-y := zip_main.o\
+  zip_device.o  \
+  zip_mem.o
diff --git a/drivers/crypto/cavium/zip/common.h 
b/drivers/crypto/cavium/zip/common.h
new file mode 100644
index 000..f0694f4
--- /dev/null
+++ b/drivers/crypto/cavium/zip/common.h
@@ -0,0 +1,258 @@
+/***license start
+ * Copyright (c) 2003-2016 Cavium, Inc.
+ * All rights reserved.
+ *
+ * License: one of 'Cavium License' or 'GNU General Public License Version 2'
+ *
+ * This file is provided under the terms of the Cavium License (see below)
+ * or under the terms of GNU General Public License, Version 2, as
+ * published by the Free Software Foundation. When using or redistributing
+ * this file, you may do so under either license.
+ *
+ * Cavium License:  Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ *
+ *  * Redistributions in binary form must reproduce the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer in the documentation and/or other

[RFC PATCH 2/3] crypto: zip - Wire-up Compression / decompression HW offload

2016-12-12 Thread Jan Glauber
From: Mahipal Challa 

This contains changes for adding compression/decompression h/w offload
functionality for both DEFLATE and LZS.

Signed-off-by: Mahipal Challa 
Signed-off-by: Vishnu Nair 
Signed-off-by: Jan Glauber 
---
 drivers/crypto/cavium/zip/Makefile  |   5 +-
 drivers/crypto/cavium/zip/zip_crypto.c  | 243 
 drivers/crypto/cavium/zip/zip_crypto.h  |   6 +
 drivers/crypto/cavium/zip/zip_deflate.c | 190 +
 drivers/crypto/cavium/zip/zip_deflate.h |  62 
 drivers/crypto/cavium/zip/zip_device.c  |   1 +
 drivers/crypto/cavium/zip/zip_inflate.c | 211 +++
 drivers/crypto/cavium/zip/zip_inflate.h |  62 
 drivers/crypto/cavium/zip/zip_main.c|  29 
 9 files changed, 779 insertions(+), 30 deletions(-)
 create mode 100644 drivers/crypto/cavium/zip/zip_crypto.c
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.h
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.h

diff --git a/drivers/crypto/cavium/zip/Makefile 
b/drivers/crypto/cavium/zip/Makefile
index 2c07508..b2f3baaf 100644
--- a/drivers/crypto/cavium/zip/Makefile
+++ b/drivers/crypto/cavium/zip/Makefile
@@ -5,4 +5,7 @@
 obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += thunderx_zip.o
 thunderx_zip-y := zip_main.o\
   zip_device.o  \
-  zip_mem.o
+  zip_crypto.o  \
+  zip_mem.o \
+  zip_deflate.o \
+  zip_inflate.o
diff --git a/drivers/crypto/cavium/zip/zip_crypto.c 
b/drivers/crypto/cavium/zip/zip_crypto.c
new file mode 100644
index 000..888e18b
--- /dev/null
+++ b/drivers/crypto/cavium/zip/zip_crypto.c
@@ -0,0 +1,243 @@
+/***license start
+ * Copyright (c) 2003-2016 Cavium, Inc.
+ * All rights reserved.
+ *
+ * License: one of 'Cavium License' or 'GNU General Public License Version 2'
+ *
+ * This file is provided under the terms of the Cavium License (see below)
+ * or under the terms of GNU General Public License, Version 2, as
+ * published by the Free Software Foundation. When using or redistributing
+ * this file, you may do so under either license.
+ *
+ * Cavium License:  Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ *
+ *  * Redistributions in binary form must reproduce the above
+ *copyright notice, this list of conditions and the following
+ *disclaimer in the documentation and/or other materials provided
+ *with the distribution.
+ *
+ *  * Neither the name of Cavium Inc. nor the names of its contributors may be
+ *used to endorse or promote products derived from this software without
+ *specific prior written permission.
+ *
+ * This Software, including technical data, may be subject to U.S. export
+ * control laws, including the U.S. Export Administration Act and its
+ * associated regulations, and may be subject to export or import
+ * regulations in other countries.
+ *
+ * TO THE MAXIMUM EXTENT PERMITTED BY LAW, THE SOFTWARE IS PROVIDED "AS IS"
+ * AND WITH ALL FAULTS AND CAVIUM INC. MAKES NO PROMISES, REPRESENTATIONS
+ * OR WARRANTIES, EITHER EXPRESS, IMPLIED, STATUTORY, OR OTHERWISE, WITH
+ * RESPECT TO THE SOFTWARE, INCLUDING ITS CONDITION, ITS CONFORMITY TO ANY
+ * REPRESENTATION OR DESCRIPTION, OR THE EXISTENCE OF ANY LATENT OR PATENT
+ * DEFECTS, AND CAVIUM SPECIFICALLY DISCLAIMS ALL IMPLIED (IF ANY)
+ * WARRANTIES OF TITLE, MERCHANTABILITY, NONINFRINGEMENT, FITNESS FOR A
+ * PARTICULAR PURPOSE, LACK OF VIRUSES, ACCURACY OR COMPLETENESS, QUIET
+ * ENJOYMENT, QUIET POSSESSION OR CORRESPONDENCE TO DESCRIPTION. THE
+ * ENTIRE  RISK ARISING OUT OF USE OR PERFORMANCE OF THE SOFTWARE LIES
+ * WITH YOU.
+ ***license end**/
+
+#include "zip_crypto.h"
+
+static void zip_static_init_zip_ops(struct zip_operation *zip_ops,
+   int lzs_flag)
+{
+   zip_ops->flush= ZIP_FLUSH_FINISH;
+
+   /* equivalent to level 6 of opensource zlib */
+   zip_ops->speed  = 1;
+
+   if (!lzs_flag) {
+   zip_ops->ccode  = 0; /* Auto Huffman */
+   zip_ops->lzs_flag   = 0;
+   zip_ops->format = ZLIB_FORMAT;
+   } else {
+   zip_ops->ccode  = 3; /* LZS Encoding */
+   zip_ops->lzs_flag   = 1;
+   zip_ops->format = LZS_FORMAT;
+   }
+   zip_ops->begin_file   = 1;
+   zip_ops->history_len  = 0;
+   zip_ops->end_file = 1;
+   zip_ops->compcode = 0;
+

[RFC PATCH 3/3] crypto: zip - Add Compression/decompression statistics

2016-12-12 Thread Jan Glauber
From: Mahipal Challa 

Add statistics for compression/decompression hardware offload
under debugfs.

Signed-off-by: Mahipal Challa 
Signed-off-by: Vishnu Nair 
Signed-off-by: Jan Glauber 
---
 drivers/crypto/cavium/zip/zip_deflate.c |  10 ++
 drivers/crypto/cavium/zip/zip_inflate.c |  12 ++
 drivers/crypto/cavium/zip/zip_main.c| 227 
 drivers/crypto/cavium/zip/zip_main.h|  15 +++
 4 files changed, 264 insertions(+)

diff --git a/drivers/crypto/cavium/zip/zip_deflate.c 
b/drivers/crypto/cavium/zip/zip_deflate.c
index 913cc25..11052d8 100644
--- a/drivers/crypto/cavium/zip/zip_deflate.c
+++ b/drivers/crypto/cavium/zip/zip_deflate.c
@@ -122,12 +122,19 @@ int zip_deflate(struct zip_operation *zip_ops, struct 
zip_state *s,
/* Prepares zip command based on the input parameters */
prepare_zip_command(zip_ops, s, zip_cmd);
 
+   atomic64_add(zip_ops->input_len, &zip_dev->stats.comp_in_bytes);
/* Loads zip command into command queues and rings door bell */
queue = zip_load_instr(zip_cmd, zip_dev);
 
+   /* Stats update for compression requests submitted */
+   atomic64_inc(&zip_dev->stats.comp_req_submit);
+
while (!result_ptr->s.compcode)
continue;
 
+   /* Stats update for compression requests completed */
+   atomic64_inc(&zip_dev->stats.comp_req_complete);
+
zip_ops->compcode = result_ptr->s.compcode;
switch (zip_ops->compcode) {
case ZIP_NOTDONE:
@@ -175,6 +182,9 @@ int zip_deflate(struct zip_operation *zip_ops, struct 
zip_state *s,
zip_err("Unknown Format:%d\n", zip_ops->format);
}
 
+   atomic64_add(result_ptr->s.totalbyteswritten,
+&zip_dev->stats.comp_out_bytes);
+
/* Update output_len */
if (zip_ops->output_len < result_ptr->s.totalbyteswritten) {
/* Dynamic stop && strm->output_len < zipconstants[onfsize] */
diff --git a/drivers/crypto/cavium/zip/zip_inflate.c 
b/drivers/crypto/cavium/zip/zip_inflate.c
index 849c4c85..44503d8 100644
--- a/drivers/crypto/cavium/zip/zip_inflate.c
+++ b/drivers/crypto/cavium/zip/zip_inflate.c
@@ -135,12 +135,20 @@ int zip_inflate(struct zip_operation *zip_ops, struct 
zip_state *s,
/* Prepare inflate zip command */
prepare_inflate_zcmd(zip_ops, s, zip_cmd);
 
+   atomic64_add(zip_ops->input_len, &zip_dev->stats.decomp_in_bytes);
+
/* Load inflate command to zip queue and ring the doorbell */
queue = zip_load_instr(zip_cmd, zip_dev);
 
+   /* Decompression requests submitted stats update */
+   atomic64_inc(&zip_dev->stats.decomp_req_submit);
+
while (!result_ptr->s.compcode)
continue;
 
+   /* Decompression requests completed stats update */
+   atomic64_inc(&zip_dev->stats.decomp_req_complete);
+
zip_ops->compcode = result_ptr->s.compcode;
switch (zip_ops->compcode) {
case ZIP_NOTDONE:
@@ -157,6 +165,7 @@ int zip_inflate(struct zip_operation *zip_ops, struct 
zip_state *s,
 
default:
zip_dbg("Instruction failed. Code = %d\n", zip_ops->compcode);
+   atomic64_inc(&zip_dev->stats.decomp_bad_reqs);
zip_update_cmd_bufs(zip_dev, queue);
return ZIP_ERROR;
}
@@ -169,6 +178,9 @@ int zip_inflate(struct zip_operation *zip_ops, struct 
zip_state *s,
 
zip_ops->csum = result_ptr->s.adler32;
 
+   atomic64_add(result_ptr->s.totalbyteswritten,
+&zip_dev->stats.decomp_out_bytes);
+
if (zip_ops->output_len < result_ptr->s.totalbyteswritten) {
zip_err("output_len (%d) < total bytes written (%d)\n",
zip_ops->output_len, result_ptr->s.totalbyteswritten);
diff --git a/drivers/crypto/cavium/zip/zip_main.c 
b/drivers/crypto/cavium/zip/zip_main.c
index ae3395f..56631bf 100644
--- a/drivers/crypto/cavium/zip/zip_main.c
+++ b/drivers/crypto/cavium/zip/zip_main.c
@@ -427,6 +427,228 @@ static void zip_unregister_compression_device(void)
crypto_unregister_alg(&zip_comp_lzs);
 }
 
+/*
+ * debugfs functions
+ */
+#ifdef CONFIG_DEBUG_FS
+#include 
+
+/* Displays ZIP device statistics */
+static int zip_show_stats(struct seq_file *s, void *unused)
+{
+   u64 val = 0ull;
+   u64 avg_chunk = 0ull, avg_cr = 0ull;
+   u32 q = 0;
+
+   int index  = 0;
+   struct zip_device *zip;
+   struct zip_stats  *st;
+
+   for (index = 0; index < MAX_ZIP_DEVICES; index++) {
+   if (zip_dev[index]) {
+   zip = zip_dev[index];
+   st  = &zip->stats;
+
+   /* Get all the pending requests */
+   for (q = 0; q < ZIP_NUM_QUEUES; q++) {
+   val = zip_reg_read((zip->reg_base +
+   ZIP_DBG_COREX_STA(q)));
+   val = (val >> 32);
+

[RFC PATCH 0/3] Cavium ThunderX ZIP driver

2016-12-12 Thread Jan Glauber
Hi Herbert,

this series adds support for hardware accelerated compression & decompression
as found on ThunderX (arm64) SOCs. I've been reviewing this driver internally
for some time and would like to get feedback on the RFC to see if this goes
into the right direction and to see if there are any concerns.

We've discussed switching to the new acomp algorithm but for the time being
decided against acomp because our test cases are not yet supported with it.

To test the ZIP driver we've used ZSWAP and IPComp.

Performance numbers from ZSWAP look promising.
The "average time" for compressing a 4KB page:

Compression Software:  128 usec
Compression HW deflate  :   16 usec
Compression HW LZS  :   10 usec

Decompression Software  : 20 usec
Decompression HW deflate: 7 usec
Decompression HW LZS: 5 usec

Patches are on top of 4.9.

Feedback welcome!
Jan 

-

Mahipal Challa (3):
  crypto: zip - Add ThunderX ZIP driver core
  crypto: zip - Wire-up Compression / decompression HW offload
  crypto: zip - Add Compression/decompression statistics

 drivers/crypto/Kconfig  |7 +
 drivers/crypto/Makefile |1 +
 drivers/crypto/cavium/Makefile  |4 +
 drivers/crypto/cavium/zip/Makefile  |   11 +
 drivers/crypto/cavium/zip/common.h  |  258 ++
 drivers/crypto/cavium/zip/zip_crypto.c  |  243 ++
 drivers/crypto/cavium/zip/zip_crypto.h  |   67 ++
 drivers/crypto/cavium/zip/zip_deflate.c |  200 +
 drivers/crypto/cavium/zip/zip_deflate.h |   62 ++
 drivers/crypto/cavium/zip/zip_device.c  |  209 +
 drivers/crypto/cavium/zip/zip_device.h  |  138 
 drivers/crypto/cavium/zip/zip_inflate.c |  223 ++
 drivers/crypto/cavium/zip/zip_inflate.h |   62 ++
 drivers/crypto/cavium/zip/zip_main.c|  698 
 drivers/crypto/cavium/zip/zip_main.h|  141 
 drivers/crypto/cavium/zip/zip_mem.c |  120 +++
 drivers/crypto/cavium/zip/zip_mem.h |   78 ++
 drivers/crypto/cavium/zip/zip_regs.h| 1326 +++
 18 files changed, 3848 insertions(+)
 create mode 100644 drivers/crypto/cavium/Makefile
 create mode 100644 drivers/crypto/cavium/zip/Makefile
 create mode 100644 drivers/crypto/cavium/zip/common.h
 create mode 100644 drivers/crypto/cavium/zip/zip_crypto.c
 create mode 100644 drivers/crypto/cavium/zip/zip_crypto.h
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.h
 create mode 100644 drivers/crypto/cavium/zip/zip_device.c
 create mode 100644 drivers/crypto/cavium/zip/zip_device.h
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.h
 create mode 100644 drivers/crypto/cavium/zip/zip_main.c
 create mode 100644 drivers/crypto/cavium/zip/zip_main.h
 create mode 100644 drivers/crypto/cavium/zip/zip_mem.c
 create mode 100644 drivers/crypto/cavium/zip/zip_mem.h
 create mode 100644 drivers/crypto/cavium/zip/zip_regs.h

-- 
2.9.0.rc0.21.g322

--
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 v6 2/2] crypto: add virtio-crypto driver

2016-12-12 Thread Herbert Xu
On Mon, Dec 12, 2016 at 06:25:12AM +, Gonglei (Arei) wrote:
> Hi, Michael & Herbert
> 
> Because the virtio-crypto device emulation had been in QEMU 2.8,
> would you please merge the virtio-crypto driver for 4.10 if no other
> comments? If so, Miachel pls ack and/or review the patch, then
> Herbert will take it (I asked him last week). Thank you!
> 
> Ps: Note on 4.10 merge window timing from Linus
>  https://lkml.org/lkml/2016/12/7/506
> 
> Dec 23rd is the deadline for 4.10 merge window.

Sorry but it's too late for 4.10.  It needed to have been in my
tree before the merge window opened to make it for this cycle.

Cheers,
-- 
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