Linux cryptographic layer development
 help / color / mirror / Atom feed
* [RFC PATCH v2 11/32] x86: Unroll string I/O when SEV is active
From: Brijesh Singh @ 2017-03-02 15:14 UTC (permalink / raw)
  To: simon.guinot, linux-efi, brijesh.singh, kvm, rkrcmar, matt,
	linux-pci, linus.walleij, gary.hook, linux-mm, paul.gortmaker,
	hpa, cl, dan.j.williams, aarcange, sfr, andriy.shevchenko,
	herbert, bhe, xemul, joro, x86, peterz, piotr.luc, mingo, msalter,
	ross.zwisler, bp, dyoung, thomas.lendacky, jroedel, keescook,
	arnd, <toshi
In-Reply-To: <148846752022.2349.13667498174822419498.stgit@brijesh-build-machine>

From: Tom Lendacky <thomas.lendacky@amd.com>

Secure Encrypted Virtualization (SEV) does not support string I/O, so
unroll the string I/O operation into a loop operating on one element at
a time.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/include/asm/io.h |   26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 833f7cc..b596114 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -327,14 +327,32 @@ static inline unsigned type in##bwl##_p(int port)			\
 									\
 static inline void outs##bwl(int port, const void *addr, unsigned long count) \
 {									\
-	asm volatile("rep; outs" #bwl					\
-		     : "+S"(addr), "+c"(count) : "d"(port));		\
+	if (sev_active()) {						\
+		unsigned type *value = (unsigned type *)addr;		\
+		while (count) {						\
+			out##bwl(*value, port);				\
+			value++;					\
+			count--;					\
+		}							\
+	} else {							\
+		asm volatile("rep; outs" #bwl				\
+			     : "+S"(addr), "+c"(count) : "d"(port));	\
+	}								\
 }									\
 									\
 static inline void ins##bwl(int port, void *addr, unsigned long count)	\
 {									\
-	asm volatile("rep; ins" #bwl					\
-		     : "+D"(addr), "+c"(count) : "d"(port));		\
+	if (sev_active()) {						\
+		unsigned type *value = (unsigned type *)addr;		\
+		while (count) {						\
+			*value = in##bwl(port);				\
+			value++;					\
+			count--;					\
+		}							\
+	} else {							\
+		asm volatile("rep; ins" #bwl				\
+			     : "+D"(addr), "+c"(count) : "d"(port));	\
+	}								\
 }
 
 BUILDIO(b, b, char)

^ permalink raw reply related

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Gilad Ben-Yossef @ 2017-03-02 14:01 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Binoy Jayan, Rajendra, Herbert Xu, Oded, Mike Snitzer,
	Linux kernel mailing list, Milan Broz, linux-raid, dm-devel,
	Mark Brown, Arnd Bergmann, linux-crypto, Shaohua Li,
	David S. Miller, Alasdair Kergon, Ofir
In-Reply-To: <CAFqZXNugEK0do+BYC061i4CidMVgg8_BZue8gUVa2g7_RYsAEw@mail.gmail.com>

On Wed, Mar 1, 2017 at 3:21 PM, Ondrej Mosnacek <omosnace@redhat.com> wrote:
> 2017-03-01 13:42 GMT+01:00 Gilad Ben-Yossef <gilad@benyossef.com>:
>
> Wouldn't adopting a bulk request API (something like what I tried to
> do here [1]) that allows users to supply multiple messages, each with
> their own IV, fulfill this purpose? That way, we wouldn't need to
> introduce any new modes into Crypto API and the drivers/accelerators
> would only need to provide bulk implementations of common modes
> (xts(aes), cbc(aes), ...) to provide better performance for dm-crypt
> (and possibly other users, too).
>
> I'm not sure how exactly these crypto accelerators work, but wouldn't
> it help if the drivers simply get more messages (in our case sectors)
> in a single call? I wonder, would (efficiently) supporting such a
> scheme require changes in the HW itself or could it be achieved just
> by modifying driver code (let's say specifically for your CryptoCell
> accelerator)?
>
> [1] https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg23007.html
>


>From a general perspective - that is things are expect to be true not
just for CryptoCell but for most HW crypto engines,
you want two things - for the HW engine to be able to burst work for a
long time and than rest for a long time vs. a stop and go scheme
(engine utilization)
and for the average IO transaction to be relatively long (bus utilization)

So, a big cluster size i.e. Milan's proposal) works great - you get both.

Submitting a series of sequential small clusters where the HW can
calculate the IV (e.g. Binoy's proposal) works great if the HW
supports it - you get both.

A batched series of small clusters + IV is less favorable - if your HW
engines has lots of parallel context processing (this is expensive for
HW) you might enjoy engine utilization but the bus utilization will be
low - lots of small transactions.

Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

^ permalink raw reply

* Re: [PATCH v3 2/2] crypto: vmx - Use skcipher for xts fallback
From: Herbert Xu @ 2017-03-02 11:07 UTC (permalink / raw)
  To: Paulo Flabiano Smorigo
  Cc: linux-kernel, paulus, linux-crypto, linuxppc-dev, davem
In-Reply-To: <20170301140000.23033-1-pfsmorigo@linux.vnet.ibm.com>

On Wed, Mar 01, 2017 at 11:00:00AM -0300, Paulo Flabiano Smorigo wrote:
> Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v3 1/2] crypto: vmx - Use skcipher for cbc fallback
From: Herbert Xu @ 2017-03-02 11:07 UTC (permalink / raw)
  To: Paulo Flabiano Smorigo
  Cc: linux-kernel, paulus, linux-crypto, linuxppc-dev, davem
In-Reply-To: <20170301135820.20670-1-pfsmorigo@linux.vnet.ibm.com>

On Wed, Mar 01, 2017 at 10:58:20AM -0300, Paulo Flabiano Smorigo wrote:
> Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com>
> ---

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: Problem with RSA test from testmgr
From: Stephan Müller @ 2017-03-02  6:08 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Corentin Labbe, herbert, linux-crypto, linux-kernel
In-Reply-To: <122aa2a3-e478-1a68-083a-273ceabaf2fb@intel.com>

Am Donnerstag, 2. März 2017, 03:15:13 CET schrieb Tadeusz Struk:

Hi Tadeusz,

> 
> 	memset(ptextp, 0, 256);
> 	memcpy(ptextp + 64 - 8, ptext_ex, plen);

I actually have tested that and it did not return the data the kernel 
implementation would return

Ciao
Stephan

^ permalink raw reply

* Re: Problem with RSA test from testmgr
From: Corentin Labbe @ 2017-03-02  6:21 UTC (permalink / raw)
  To: Stephan Müller; +Cc: herbert, linux-crypto, linux-kernel
In-Reply-To: <45107561.YcGaMWr4ZQ@tauon.atsec.com>

On Wed, Mar 01, 2017 at 04:07:17PM +0100, Stephan Müller wrote:
> Am Mittwoch, 1. März 2017, 13:04:14 CET schrieb Corentin Labbe:
> 
> Hi Corentin,
> 
> > 
> > I got the following:
> > 
> > [    1.086228] alg: akcipher: encrypt test failed. Invalid output
> > [    1.092196] 00000000: 6e 7c 8a 75 e7 30 80 d1 5e ab 9b db a2 cf ed db
> > [    1.098882] 00000010: c9 b2 db 43 bd 9a b9 75 27 f3 73 d9 73 b7 81 8c
> > [    1.105524] 00000020: 49 e8 45 fc 43 44 f5 6d f0 f7 b8 f2 ae 6b ae 49
> > [    1.112090] 00000030: 1b 8e 50 c6 88 4e 99 09 78 14 f2 5d 99 c3 7f f9
> > [    1.118747] alg: akcipher: test 1 failed for rsa-generic, err=-22
> > (Exactly the output of my hardare and openssl test)
> > 
> > So the problem is just that my hardware does not handle non-padded data.
> 
> I guess the best course of action would be to patch the test vector to use the 
> padded data.
> 
> Ciao
> Stephan

I am finishing a patch that made testmgr test both (padded and unpadded).

Regards
Corentin Labbe

^ permalink raw reply

* Re: Problem with RSA test from testmgr
From: Tadeusz Struk @ 2017-03-02  2:15 UTC (permalink / raw)
  To: Corentin Labbe, Stephan Müller; +Cc: herbert, linux-crypto, linux-kernel
In-Reply-To: <20170301120414.GA18217@Red>

Hi Corentin,
On 03/01/2017 04:04 AM, Corentin Labbe wrote:
>> I would think the issue is that the OpenSSL BIGNUM lib has some issues: when 
>> calculating m^e mod n, m has to be equal to the key size. The kernel's MPI 
>> code handles the case where m is smaller than the key size.
>>
>> Note, in your code below, ptext is the 8 bytes from ptext_ex plus trailing 
>> zeroes whereas the kernel uses just the 8 bytes.
>>
>> It seems that your implementation has the same issue.
>>
>> What about the following test: change vector->m to be 64 bytes (i.e. 
>> RSA_size(key) in size in testmgr.h and check the output of crypto/rsa.c, 
>> openssl's output with the app below and your RSA hardware.
> I got the following:
> 
> [    1.086228] alg: akcipher: encrypt test failed. Invalid output
> [    1.092196] 00000000: 6e 7c 8a 75 e7 30 80 d1 5e ab 9b db a2 cf ed db
> [    1.098882] 00000010: c9 b2 db 43 bd 9a b9 75 27 f3 73 d9 73 b7 81 8c
> [    1.105524] 00000020: 49 e8 45 fc 43 44 f5 6d f0 f7 b8 f2 ae 6b ae 49
> [    1.112090] 00000030: 1b 8e 50 c6 88 4e 99 09 78 14 f2 5d 99 c3 7f f9
> [    1.118747] alg: akcipher: test 1 failed for rsa-generic, err=-22
> (Exactly the output of my hardare and openssl test)
> 
> So the problem is just that my hardware does not handle non-padded data.

The difference between openssl's RSA_private_decrypt() and the akcipher api
is that openssl only takes only one size, flen, for both src and dst buffers,
so in your test app you need to do something like this:

	memset(ptextp, 0, 256);
	memcpy(ptextp + 64 - 8, ptext_ex, plen);

	key = RSA_new();

	key->n = BN_bin2bn(n, sizeof(n)-1, key->n);
	key->e = BN_bin2bn(e, sizeof(e)-1, key->e);

	num = RSA_public_encrypt(RSA_size(key), ptextp, ctext, key, RSA_NO_PADDING);

The akcipher API has separate sizes for both the src and dst. It is the length of the
scatterlist in the akcipher_request. If a HW can't handle different buffers lengths
then its driver needs to add the padding internally.

Thanks,
-- 
Tadeusz

^ permalink raw reply

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Binoy Jayan @ 2017-03-01 18:04 UTC (permalink / raw)
  To: Milan Broz
  Cc: Rajendra, Herbert Xu, Oded, Mike Snitzer,
	Linux kernel mailing list, Ondrej Mosnacek, linux-raid,
	Gilad Ben-Yossef, dm-devel, Mark Brown, Arnd Bergmann,
	linux-crypto, Shaohua Li, David S. Miller, Alasdair Kergon, Ofir
In-Reply-To: <68f70534-a309-46ba-a84d-8acc1e6620e5@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1616 bytes --]

Hi Milan,

On 1 March 2017 at 02:35, Milan Broz <gmazyland@gmail.com> wrote:

> On 02/22/2017 07:12 AM, Binoy Jayan wrote:
> >
> > I was wondering if this is near to be ready for submission (apart from
> > the testmgr.c
> > changes) or I need to make some changes to make it similar to the IPSec
> offload?
>
> I just tried this and except it registers the IV for every new device
> again, it works...
> (After a while you have many duplicate entries in /proc/crypto.)


It is because the the crypto lookup api sees that the crypto algorithm is in
a LARVAL state and registers a new instance every time by invoking the
".create" callback. I guess it should be solved by adding test data to
testmgr.

Do you have some real performance numbers that proves that such a patch is
> adequate?
>

While waiting to do some implementation of the hw crypto drivers to work
with
dm-crypt, I'll also generate some numbers to compare the performance with
the
original dm-crypt code with the new one with a software implementation in
place.


> I would really like to see the performance issue fixed but I am really not
> sure
> this approach works for everyone. It would be better to avoid repeating
> this exercise later.
> IIRC Ondra's "bulk" mode, despite rejected, shows that there is a potential
> to speedup things even for crypt drivers that do not support own IV
> generators.
>

I think it should work for everyone (even for ciphers not supporting IVs)
if the null IV
mode is used. It should be upto the IV generation template to choose to
generate IV
or just call the underlying (base) template/cipher.

Regards,
Binoy

[-- Attachment #1.2: Type: text/html, Size: 2501 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Milan Broz @ 2017-03-01 15:38 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Binoy Jayan, Oded, Ofir, Herbert Xu, David S. Miller,
	linux-crypto, Mark Brown, Arnd Bergmann,
	Linux kernel mailing list, Alasdair Kergon, Mike Snitzer,
	dm-devel, Shaohua Li, linux-raid, Rajendra, Ondrej Mosnacek
In-Reply-To: <2aef6e54-805f-e09b-ae66-c198f8c05335@gmail.com>


On 03/01/2017 02:04 PM, Milan Broz wrote:
> On 03/01/2017 01:42 PM, Gilad Ben-Yossef wrote:
> ...
> 
>> I can certainly understand if you don't wont to take the patch until
>> we have results with
>> dm-crypt itself but the difference between 8 separate invocation of
>> the engine for 512
>> bytes of XTS and a single invocation for 4KB are pretty big.
> 
> Yes, I know it. But the same can be achieved if we just implement
> 4k sector encryption in dmcrypt. It is incompatible with LUKS1
> (but next LUKS version will support it) but I think this is not
> a problem for now.
> 
> If the underlying device supports atomic write of 4k sectors, then
> there should not be a problem.
> 
> This is one of the speed-up I would like to compare with the IV approach,
> because everyone should benefit from 4k sectors in the end.
> And no crypto API changes are needed here.
> 
> (I have an old patch for this, so I will try to revive it.)

If anyone interested, simple experimental patch for larger sector size
(up to the page size) for dmcrypt is in this branch:

http://git.kernel.org/cgit/linux/kernel/git/mbroz/linux.git/log/?h=dm-crypt-4k-sector

It would be nice to check what performance gain could be provided
by this simple approach.

Milan

^ permalink raw reply

* Re: Problem with RSA test from testmgr
From: Stephan Müller @ 2017-03-01 15:07 UTC (permalink / raw)
  To: Corentin Labbe; +Cc: herbert, linux-crypto, linux-kernel
In-Reply-To: <20170301120414.GA18217@Red>

Am Mittwoch, 1. März 2017, 13:04:14 CET schrieb Corentin Labbe:

Hi Corentin,

> 
> I got the following:
> 
> [    1.086228] alg: akcipher: encrypt test failed. Invalid output
> [    1.092196] 00000000: 6e 7c 8a 75 e7 30 80 d1 5e ab 9b db a2 cf ed db
> [    1.098882] 00000010: c9 b2 db 43 bd 9a b9 75 27 f3 73 d9 73 b7 81 8c
> [    1.105524] 00000020: 49 e8 45 fc 43 44 f5 6d f0 f7 b8 f2 ae 6b ae 49
> [    1.112090] 00000030: 1b 8e 50 c6 88 4e 99 09 78 14 f2 5d 99 c3 7f f9
> [    1.118747] alg: akcipher: test 1 failed for rsa-generic, err=-22
> (Exactly the output of my hardare and openssl test)
> 
> So the problem is just that my hardware does not handle non-padded data.

I guess the best course of action would be to patch the test vector to use the 
padded data.

Ciao
Stephan

^ permalink raw reply

* [PATCH v3 2/2] crypto: vmx - Use skcipher for xts fallback
From: Paulo Flabiano Smorigo @ 2017-03-01 14:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: herbert, paulus, linux-crypto, Paulo Flabiano Smorigo,
	linuxppc-dev, davem
In-Reply-To: <20170224142734.16188-1-pfsmorigo@linux.vnet.ibm.com>

Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com>
---
 drivers/crypto/vmx/aes_xts.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
index 24353ec3..6adc929 100644
--- a/drivers/crypto/vmx/aes_xts.c
+++ b/drivers/crypto/vmx/aes_xts.c
@@ -28,11 +28,12 @@
 #include <crypto/aes.h>
 #include <crypto/scatterwalk.h>
 #include <crypto/xts.h>
+#include <crypto/skcipher.h>
 
 #include "aesp8-ppc.h"
 
 struct p8_aes_xts_ctx {
-	struct crypto_blkcipher *fallback;
+	struct crypto_skcipher *fallback;
 	struct aes_key enc_key;
 	struct aes_key dec_key;
 	struct aes_key tweak_key;
@@ -41,7 +42,7 @@ struct p8_aes_xts_ctx {
 static int p8_aes_xts_init(struct crypto_tfm *tfm)
 {
 	const char *alg;
-	struct crypto_blkcipher *fallback;
+	struct crypto_skcipher *fallback;
 	struct p8_aes_xts_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	if (!(alg = crypto_tfm_alg_name(tfm))) {
@@ -49,8 +50,8 @@ static int p8_aes_xts_init(struct crypto_tfm *tfm)
 		return -ENOENT;
 	}
 
-	fallback =
-		crypto_alloc_blkcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
+	fallback = crypto_alloc_skcipher(alg, 0,
+			CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
 	if (IS_ERR(fallback)) {
 		printk(KERN_ERR
 			"Failed to allocate transformation for '%s': %ld\n",
@@ -58,11 +59,11 @@ static int p8_aes_xts_init(struct crypto_tfm *tfm)
 		return PTR_ERR(fallback);
 	}
 	printk(KERN_INFO "Using '%s' as fallback implementation.\n",
-		crypto_tfm_alg_driver_name((struct crypto_tfm *) fallback));
+		crypto_skcipher_driver_name(fallback));
 
-	crypto_blkcipher_set_flags(
+	crypto_skcipher_set_flags(
 		fallback,
-		crypto_blkcipher_get_flags((struct crypto_blkcipher *)tfm));
+		crypto_skcipher_get_flags((struct crypto_skcipher *)tfm));
 	ctx->fallback = fallback;
 
 	return 0;
@@ -73,7 +74,7 @@ static void p8_aes_xts_exit(struct crypto_tfm *tfm)
 	struct p8_aes_xts_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	if (ctx->fallback) {
-		crypto_free_blkcipher(ctx->fallback);
+		crypto_free_skcipher(ctx->fallback);
 		ctx->fallback = NULL;
 	}
 }
@@ -98,7 +99,7 @@ static int p8_aes_xts_setkey(struct crypto_tfm *tfm, const u8 *key,
 	pagefault_enable();
 	preempt_enable();
 
-	ret += crypto_blkcipher_setkey(ctx->fallback, key, keylen);
+	ret += crypto_skcipher_setkey(ctx->fallback, key, keylen);
 	return ret;
 }
 
@@ -113,15 +114,14 @@ static int p8_aes_xts_crypt(struct blkcipher_desc *desc,
 	struct blkcipher_walk walk;
 	struct p8_aes_xts_ctx *ctx =
 		crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
-	struct blkcipher_desc fallback_desc = {
-		.tfm = ctx->fallback,
-		.info = desc->info,
-		.flags = desc->flags
-	};
 
 	if (in_interrupt()) {
-		ret = enc ? crypto_blkcipher_encrypt(&fallback_desc, dst, src, nbytes) :
-                            crypto_blkcipher_decrypt(&fallback_desc, dst, src, nbytes);
+		SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
+		skcipher_request_set_tfm(req, ctx->fallback);
+		skcipher_request_set_callback(req, desc->flags, NULL, NULL);
+		skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
+		ret = enc? crypto_skcipher_encrypt(req) : crypto_skcipher_decrypt(req);
+		skcipher_request_zero(req);
 	} else {
 		preempt_disable();
 		pagefault_disable();
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 1/2] crypto: vmx - Use skcipher for cbc fallback
From: Paulo Flabiano Smorigo @ 2017-03-01 13:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: benh, paulus, mpe, herbert, davem, linux-crypto, linuxppc-dev,
	Paulo Flabiano Smorigo
In-Reply-To: <20170226192107.GA7129@gallifrey>

Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com>
---
 drivers/crypto/vmx/aes_cbc.c | 47 ++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
index 94ad5c0..72a26eb 100644
--- a/drivers/crypto/vmx/aes_cbc.c
+++ b/drivers/crypto/vmx/aes_cbc.c
@@ -27,11 +27,12 @@
 #include <asm/switch_to.h>
 #include <crypto/aes.h>
 #include <crypto/scatterwalk.h>
+#include <crypto/skcipher.h>
 
 #include "aesp8-ppc.h"
 
 struct p8_aes_cbc_ctx {
-	struct crypto_blkcipher *fallback;
+	struct crypto_skcipher *fallback;
 	struct aes_key enc_key;
 	struct aes_key dec_key;
 };
@@ -39,7 +40,7 @@ struct p8_aes_cbc_ctx {
 static int p8_aes_cbc_init(struct crypto_tfm *tfm)
 {
 	const char *alg;
-	struct crypto_blkcipher *fallback;
+	struct crypto_skcipher *fallback;
 	struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	if (!(alg = crypto_tfm_alg_name(tfm))) {
@@ -47,8 +48,9 @@ static int p8_aes_cbc_init(struct crypto_tfm *tfm)
 		return -ENOENT;
 	}
 
-	fallback =
-	    crypto_alloc_blkcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
+	fallback = crypto_alloc_skcipher(alg, 0,
+			CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+
 	if (IS_ERR(fallback)) {
 		printk(KERN_ERR
 		       "Failed to allocate transformation for '%s': %ld\n",
@@ -56,11 +58,12 @@ static int p8_aes_cbc_init(struct crypto_tfm *tfm)
 		return PTR_ERR(fallback);
 	}
 	printk(KERN_INFO "Using '%s' as fallback implementation.\n",
-	       crypto_tfm_alg_driver_name((struct crypto_tfm *) fallback));
+		crypto_skcipher_driver_name(fallback));
+
 
-	crypto_blkcipher_set_flags(
+	crypto_skcipher_set_flags(
 		fallback,
-		crypto_blkcipher_get_flags((struct crypto_blkcipher *)tfm));
+		crypto_skcipher_get_flags((struct crypto_skcipher *)tfm));
 	ctx->fallback = fallback;
 
 	return 0;
@@ -71,7 +74,7 @@ static void p8_aes_cbc_exit(struct crypto_tfm *tfm)
 	struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
 
 	if (ctx->fallback) {
-		crypto_free_blkcipher(ctx->fallback);
+		crypto_free_skcipher(ctx->fallback);
 		ctx->fallback = NULL;
 	}
 }
@@ -91,7 +94,7 @@ static int p8_aes_cbc_setkey(struct crypto_tfm *tfm, const u8 *key,
 	pagefault_enable();
 	preempt_enable();
 
-	ret += crypto_blkcipher_setkey(ctx->fallback, key, keylen);
+	ret += crypto_skcipher_setkey(ctx->fallback, key, keylen);
 	return ret;
 }
 
@@ -103,15 +106,14 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc *desc,
 	struct blkcipher_walk walk;
 	struct p8_aes_cbc_ctx *ctx =
 		crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
-	struct blkcipher_desc fallback_desc = {
-		.tfm = ctx->fallback,
-		.info = desc->info,
-		.flags = desc->flags
-	};
 
 	if (in_interrupt()) {
-		ret = crypto_blkcipher_encrypt(&fallback_desc, dst, src,
-					       nbytes);
+		SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
+		skcipher_request_set_tfm(req, ctx->fallback);
+		skcipher_request_set_callback(req, desc->flags, NULL, NULL);
+		skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
+		ret = crypto_skcipher_encrypt(req);
+		skcipher_request_zero(req);
 	} else {
 		preempt_disable();
 		pagefault_disable();
@@ -144,15 +146,14 @@ static int p8_aes_cbc_decrypt(struct blkcipher_desc *desc,
 	struct blkcipher_walk walk;
 	struct p8_aes_cbc_ctx *ctx =
 		crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
-	struct blkcipher_desc fallback_desc = {
-		.tfm = ctx->fallback,
-		.info = desc->info,
-		.flags = desc->flags
-	};
 
 	if (in_interrupt()) {
-		ret = crypto_blkcipher_decrypt(&fallback_desc, dst, src,
-					       nbytes);
+		SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
+		skcipher_request_set_tfm(req, ctx->fallback);
+		skcipher_request_set_callback(req, desc->flags, NULL, NULL);
+		skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
+		ret = crypto_skcipher_decrypt(req);
+		skcipher_request_zero(req);
 	} else {
 		preempt_disable();
 		pagefault_disable();
-- 
2.9.3

^ permalink raw reply related

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Ondrej Mosnacek @ 2017-03-01 13:21 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Milan Broz, Binoy Jayan, Oded, Ofir, Herbert Xu, David S. Miller,
	linux-crypto, Mark Brown, Arnd Bergmann,
	Linux kernel mailing list, Alasdair Kergon, Mike Snitzer,
	dm-devel, Shaohua Li, linux-raid, Rajendra
In-Reply-To: <CAOtvUMePYT7OJDKL7i0y3y6Jvqsxyx6Je6g9aGKVq6PLZ_-Z8w@mail.gmail.com>

2017-03-01 13:42 GMT+01:00 Gilad Ben-Yossef <gilad@benyossef.com>:
> It really is an observation about overhead of context switches between
> dm-crypt and
> whatever/wherever you handle crypto - be it an off CPU hardware engine
> or a bunch
> of parallel kernel threads running on other cores. You really want to
> burst as much as
> possible.

[...]

>> For XTS you need just simple linear IV. No problem with that, implementation
>> in crypto API and hw is trivial.
>>
>> But for compatible IV (that provides compatibility with loopAES and very old TrueCrypt),
>> these should be never ever implemented again anywhere.
>
>>
>> Specifically "tcw" is broken, insecure and provided here just to help people to migrate
>> from old Truecrypt containers. Even Truecrypt followers removed it from the codebase.
>> (It is basically combination of IV and slight modification of CBC mode. All
>> recent version switched to XTS and plain IV.)
>>
>> So building abstraction over something known to be broken and that is now intentionally
>> isolated inside dmcrypt is, in my opinion, really not a good idea.
>>
>
> I don't think anyone is interested in these modes. How do you support
> XTS and essiv in
> a generic way without supporting this broken modes is not something
> I'm clear on though.

Wouldn't adopting a bulk request API (something like what I tried to
do here [1]) that allows users to supply multiple messages, each with
their own IV, fulfill this purpose? That way, we wouldn't need to
introduce any new modes into Crypto API and the drivers/accelerators
would only need to provide bulk implementations of common modes
(xts(aes), cbc(aes), ...) to provide better performance for dm-crypt
(and possibly other users, too).

I'm not sure how exactly these crypto accelerators work, but wouldn't
it help if the drivers simply get more messages (in our case sectors)
in a single call? I wonder, would (efficiently) supporting such a
scheme require changes in the HW itself or could it be achieved just
by modifying driver code (let's say specifically for your CryptoCell
accelerator)?

Thanks,
Ondrej

[1] https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg23007.html

>
> Thanks,
> Gilad
>
>
>
> --
> Gilad Ben-Yossef
> Chief Coffee Drinker
>
> "If you take a class in large-scale robotics, can you end up in a
> situation where the homework eats your dog?"
>  -- Jean-Baptiste Queru

^ permalink raw reply

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Milan Broz @ 2017-03-01 13:04 UTC (permalink / raw)
  To: Gilad Ben-Yossef, Milan Broz
  Cc: Binoy Jayan, Rajendra, Herbert Xu, Oded, Mike Snitzer,
	Linux kernel mailing list, Ondrej Mosnacek, linux-raid, dm-devel,
	Mark Brown, Arnd Bergmann, linux-crypto, Shaohua Li,
	David S. Miller, Alasdair Kergon, Ofir
In-Reply-To: <CAOtvUMePYT7OJDKL7i0y3y6Jvqsxyx6Je6g9aGKVq6PLZ_-Z8w@mail.gmail.com>

On 03/01/2017 01:42 PM, Gilad Ben-Yossef wrote:
...

> I can certainly understand if you don't wont to take the patch until
> we have results with
> dm-crypt itself but the difference between 8 separate invocation of
> the engine for 512
> bytes of XTS and a single invocation for 4KB are pretty big.

Yes, I know it. But the same can be achieved if we just implement
4k sector encryption in dmcrypt. It is incompatible with LUKS1
(but next LUKS version will support it) but I think this is not
a problem for now.

If the underlying device supports atomic write of 4k sectors, then
there should not be a problem.

This is one of the speed-up I would like to compare with the IV approach,
because everyone should benefit from 4k sectors in the end.
And no crypto API changes are needed here.

(I have an old patch for this, so I will try to revive it.)

Milan

^ permalink raw reply

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Gilad Ben-Yossef @ 2017-03-01 12:42 UTC (permalink / raw)
  To: Milan Broz
  Cc: Binoy Jayan, Rajendra, Herbert Xu, Oded, Mike Snitzer,
	Linux kernel mailing list, Ondrej Mosnacek, linux-raid, dm-devel,
	Mark Brown, Arnd Bergmann, linux-crypto, Shaohua Li,
	David S. Miller, Alasdair Kergon, Ofir
In-Reply-To: <b563eb97-82ba-69d2-c4c5-66bc716a7507@gmail.com>

On Wed, Mar 1, 2017 at 11:29 AM, Milan Broz <gmazyland@gmail.com> wrote:
>
> On 03/01/2017 09:30 AM, Gilad Ben-Yossef wrote:
> > On Tue, Feb 28, 2017 at 11:05 PM, Milan Broz <gmazyland@gmail.com> wrote:
> >>
> >> On 02/22/2017 07:12 AM, Binoy Jayan wrote:
> >>>
> >>> I was wondering if this is near to be ready for submission (apart from
> >>> the testmgr.c
> >>> changes) or I need to make some changes to make it similar to the IPSec offload?
> >>
> >> I just tried this and except it registers the IV for every new device again, it works...
> >> (After a while you have many duplicate entries in /proc/crypto.)
> >>
> >> But I would like to see some summary why such a big patch is needed in the first place.
> >> (During an internal discussions seems that people are already lost in mails and
> >> patches here, so Ondra promised me to send some summary mail soon here.)
> >>
> >> IIRC the first initial problem was dmcrypt performance on some embedded
> >> crypto processors that are not able to cope with small crypto requests effectively.
> >>
> >>
> >> Do you have some real performance numbers that proves that such a patch is adequate?
> >>
> >> I would really like to see the performance issue fixed but I am really not sure
> >> this approach works for everyone. It would be better to avoid repeating this exercise later.
> >> IIRC Ondra's "bulk" mode, despite rejected, shows that there is a potential
> >> to speedup things even for crypt drivers that do not support own IV generators.
> >>
> >
> > AFAIK the problem that we are trying to solve is that if the IV is
> > generated outside the crypto API
> > domain than you are forced to have an invocation of the crypto API per
> > each block because you
> > need to provide the IV for each block.
> >
> > By putting the IV generation responsibility in the Crypto API we open
> > the way to do a single invocation
> > of the crypto API for a whole sequence of blocks.
>
> Sure, but this is theory. Does it really work on some hw already?
> Do you have performance measurements or comparison?

I'm working on up streaming a driver for Arm  CryptoCell that supports
this and working
offline to get Binoy a board to test this with. Alas, shipping crypto
HW has its fair share
of regulatory challenges... :-)

I can certainly understand if you don't wont to take the patch until
we have results with
dm-crypt itself but the difference between 8 separate invocation of
the engine for 512
bytes of XTS and a single invocation for 4KB are pretty big.

>From what I know of HW engines I'd be surprised if this is in any way
unique to CryptoCell.

> > For software implementation of XTS this doesn't matter much - but for
> > hardware based XTS providers
>
> It is not only embedded crypto, we have some more reports in the past
> that 512B sectors are not ideal even for other systems.
> (IIRC it was also with AES-NI that represents really big group of users).

I never said anything about embedded :-)

It really is an observation about overhead of context switches between
dm-crypt and
whatever/wherever you handle crypto - be it an off CPU hardware engine
or a bunch
of parallel kernel threads running on other cores. You really want to
burst as much as
possible.


>
> > This lead some vendors to ship hacked up versions of dm-crypt to match
> > the specific crypto hardware
> > they were using, or so I've heard at least - didn't see the code myself.
>
> I saw few version of that. There was a very hacky way to provide request-based dmcrypt
> (see old "Introduce the request handling for dm-crypt" thread on dm-devel).
> This is not the acceptable way but definitely it points to the same problem.
>
> > I believe Binoy is trying to address this in a generic upstream worthy
> > way instead.
>
> IIRC the problem is performance, if we can solve it by some generic way,
> good, but for now it seems to be a big change and just hope it helps later...
>

I see what you're saying. We need number to back this up.

> > Anyway, you are only supposed to see s difference when using a
> > hardware based XTS provider algo
> > that supports IV generation.
> >
> >> I like the patch is now contained inside dmcrypt, but it still exposes IVs that
> >> are designed just for old, insecure, compatibility-only containers.
> >>
> >> I really do not think every compatible crap must be accessible through crypto API.
> >> (I wrote the dmcrypt lrw and tcw compatibility IVs and I would never do that this way
> >> if I know it is accessible outside of dmcrypt internals...)
> >> Even the ESSIV is something that was born to fix predictive IVs (CBC watermarking
> >> attacks) for disk encryption only, no reason to expose it outside of disk encryption.
> >>
> >
> > The point is that you have more than one implementation of these
> > "compatible crap" - the
> > software implementation that you wrote and potentially multiple
> > hardware implementations
> > and putting this in the crypto API domain is the way to abstract this
> > so you use the one
> > that works best of your platform.
>
> For XTS you need just simple linear IV. No problem with that, implementation
> in crypto API and hw is trivial.
>
> But for compatible IV (that provides compatibility with loopAES and very old TrueCrypt),
> these should be never ever implemented again anywhere.

>
> Specifically "tcw" is broken, insecure and provided here just to help people to migrate
> from old Truecrypt containers. Even Truecrypt followers removed it from the codebase.
> (It is basically combination of IV and slight modification of CBC mode. All
> recent version switched to XTS and plain IV.)
>
> So building abstraction over something known to be broken and that is now intentionally
> isolated inside dmcrypt is, in my opinion, really not a good idea.
>

I don't think anyone is interested in these modes. How do you support
XTS and essiv in
a generic way without supporting this broken modes is not something
I'm clear on though.

>
> But please do get me wrong,  I do not want to block any improvement.
>
> But it seems to me that this thread focused on creating nice crypto API interface
> for FDE IVs instead of demonstration that the proposed solution really solves
> the performance issue.
> And not only for your hw driver, maybe other systems could benefit from the better
> processing of small requests as well.
>

Of course, the benefits at large needs to outweigh the cost. But I
don't think functioning
better when working on large bursts is in any way special to specific HW.

Indeed, I wonder if we can show a benefit for just cryptd use case.
I'll look into that.

Thanks,
Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

^ permalink raw reply

* Re: Problem with RSA test from testmgr
From: Corentin Labbe @ 2017-03-01 12:04 UTC (permalink / raw)
  To: Stephan Müller; +Cc: herbert, linux-crypto, linux-kernel
In-Reply-To: <9482066.IEBbAWS9B8@positron.chronox.de>

On Tue, Feb 28, 2017 at 11:35:04PM +0100, Stephan Müller wrote:
> Am Dienstag, 28. Februar 2017, 17:45:53 CET schrieb Corentin Labbe:
> 
> Hi Corentin,
> 
> > On Tue, Feb 28, 2017 at 05:08:35PM +0100, Stephan Müller wrote:
> > > Am Dienstag, 28. Februar 2017, 16:59:53 CET schrieb Corentin Labbe:
> > > 
> > > Hi Corentin,
> > > 
> > > > hello
> > > > 
> > > > I work on the sun8i-ce crypto accelerator and I have some problem with
> > > > the
> > > > RSA part.
> > > > 
> > > > The RSA register fail at the first RSA test (encrypt 512bit) with this
> > > > output: [ 8480.146843] alg: akcipher: encrypt test failed. Invalid
> > > > output
> > > > [ 8480.146871] 00000000: 6e 7c 8a 75 e7 30 80 d1 5e ab 9b db a2 cf ed db
> > > > [ 8480.146897] 00000010: c9 b2 db 43 bd 9a b9 75 27 f3 73 d9 73 b7 81 8c
> > > > [ 8480.146921] 00000020: 49 e8 45 fc 43 44 f5 6d f0 f7 b8 f2 ae 6b ae 49
> > > > [ 8480.146946] 00000030: 1b 8e 50 c6 88 4e 99 09 78 14 f2 5d 99 c3 7f f9
> > > > [ 8480.146995] alg: akcipher: test 1 failed for rsa-sun8i-ce, err=-22
> > > > 
> > > > But with the same parameters (msg, n, e) openssl give me exactly this
> > > > output.
> > > > 
> > > > So what I miss for made it work ?
> > > > In which format testmgr expect the output data ?
> > > 
> > > The output should be simply the binary string from the modular
> > > exponentiation operation.
> > > 
> > > What I am wondering is: the output logged above is not found in the
> > > expected values of testmgr.h. Which input data or test vectors do you
> > > use?
> > > 
> > > Ciao
> > > Stephan
> > 
> > I use the first test from rsa_tv_template in crypto/testmgr.h
> > The test fail on the encrypt operation.
> > 
> > I have put below the openssl program that give me the same output than my
> > hardware accelerator with the same parameters.
> 
> I would think the issue is that the OpenSSL BIGNUM lib has some issues: when 
> calculating m^e mod n, m has to be equal to the key size. The kernel's MPI 
> code handles the case where m is smaller than the key size.
> 
> Note, in your code below, ptext is the 8 bytes from ptext_ex plus trailing 
> zeroes whereas the kernel uses just the 8 bytes.
> 
> It seems that your implementation has the same issue.
> 
> What about the following test: change vector->m to be 64 bytes (i.e. 
> RSA_size(key) in size in testmgr.h and check the output of crypto/rsa.c, 
> openssl's output with the app below and your RSA hardware.

I got the following:

[    1.086228] alg: akcipher: encrypt test failed. Invalid output
[    1.092196] 00000000: 6e 7c 8a 75 e7 30 80 d1 5e ab 9b db a2 cf ed db
[    1.098882] 00000010: c9 b2 db 43 bd 9a b9 75 27 f3 73 d9 73 b7 81 8c
[    1.105524] 00000020: 49 e8 45 fc 43 44 f5 6d f0 f7 b8 f2 ae 6b ae 49
[    1.112090] 00000030: 1b 8e 50 c6 88 4e 99 09 78 14 f2 5d 99 c3 7f f9
[    1.118747] alg: akcipher: test 1 failed for rsa-generic, err=-22
(Exactly the output of my hardare and openssl test)

So the problem is just that my hardware does not handle non-padded data.

Thanks
Regards
Corentin Labbe

^ permalink raw reply

* Re: [PATCH 1/2] crypto: arm/crc32 - fix build error with outdated binutils
From: Herbert Xu @ 2017-03-01 11:52 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-arm-kernel, linux-crypto, arnd, jonathanh
In-Reply-To: <1488292617-27317-1-git-send-email-ard.biesheuvel@linaro.org>

On Tue, Feb 28, 2017 at 02:36:56PM +0000, Ard Biesheuvel wrote:
> Annotate a vmov instruction with an explicit element size of 32 bits.
> This is inferred by recent toolchains, but apparently, older versions
> need some help figuring this out.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] crypto: Pad aes_ccm_enc_tv_template vector
From: Herbert Xu @ 2017-03-01 11:52 UTC (permalink / raw)
  To: Laura Abbott; +Cc: David S. Miller, linux-crypto, linux-kernel, Ard Biesheuvel
In-Reply-To: <1488319645-29129-1-git-send-email-labbott@redhat.com>

On Tue, Feb 28, 2017 at 02:07:25PM -0800, Laura Abbott wrote:
> Running with KASAN and crypto tests currently gives
> 
>  BUG: KASAN: global-out-of-bounds in __test_aead+0x9d9/0x2200 at addr ffffffff8212fca0
>  Read of size 16 by task cryptomgr_test/1107
>  Address belongs to variable 0xffffffff8212fca0
>  CPU: 0 PID: 1107 Comm: cryptomgr_test Not tainted 4.10.0+ #45
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.1-1.fc24 04/01/2014
>  Call Trace:
>   dump_stack+0x63/0x8a
>   kasan_report.part.1+0x4a7/0x4e0
>   ? __test_aead+0x9d9/0x2200
>   ? crypto_ccm_init_crypt+0x218/0x3c0 [ccm]
>   kasan_report+0x20/0x30
>   check_memory_region+0x13c/0x1a0
>   memcpy+0x23/0x50
>   __test_aead+0x9d9/0x2200
>   ? kasan_unpoison_shadow+0x35/0x50
>   ? alg_test_akcipher+0xf0/0xf0
>   ? crypto_skcipher_init_tfm+0x2e3/0x310
>   ? crypto_spawn_tfm2+0x37/0x60
>   ? crypto_ccm_init_tfm+0xa9/0xd0 [ccm]
>   ? crypto_aead_init_tfm+0x7b/0x90
>   ? crypto_alloc_tfm+0xc4/0x190
>   test_aead+0x28/0xc0
>   alg_test_aead+0x54/0xd0
>   alg_test+0x1eb/0x3d0
>   ? alg_find_test+0x90/0x90
>   ? __sched_text_start+0x8/0x8
>   ? __wake_up_common+0x70/0xb0
>   cryptomgr_test+0x4d/0x60
>   kthread+0x173/0x1c0
>   ? crypto_acomp_scomp_free_ctx+0x60/0x60
>   ? kthread_create_on_node+0xa0/0xa0
>   ret_from_fork+0x2c/0x40
>  Memory state around the buggy address:
>   ffffffff8212fb80: 00 00 00 00 01 fa fa fa fa fa fa fa 00 00 00 00
>   ffffffff8212fc00: 00 01 fa fa fa fa fa fa 00 00 00 00 01 fa fa fa
>  >ffffffff8212fc80: fa fa fa fa 00 05 fa fa fa fa fa fa 00 00 00 00
>                                    ^
>   ffffffff8212fd00: 01 fa fa fa fa fa fa fa 00 00 00 00 01 fa fa fa
>   ffffffff8212fd80: fa fa fa fa 00 00 00 00 00 05 fa fa fa fa fa fa
> 
> This always happens on the same IV which is less than 16 bytes.
> 
> Per Ard,
> 
> "CCM IVs are 16 bytes, but due to the way they are constructed
> internally, the final couple of bytes of input IV are dont-cares.
> 
> Apparently, we do read all 16 bytes, which triggers the KASAN errors."
> 
> Fix this by padding the IV with null bytes to be at least 16 bytes.
> 
> Fixes: 0bc5a6c5c79a ("crypto: testmgr - Disable rfc4309 test and convert
> test vectors")
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Laura Abbott <labbott@redhat.com>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 2/2] crypto: arm - add build time test for CRC instruction support
From: Herbert Xu @ 2017-03-01 11:52 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: arnd, linux-crypto, linux-arm-kernel, jonathanh
In-Reply-To: <1488292617-27317-2-git-send-email-ard.biesheuvel@linaro.org>

On Tue, Feb 28, 2017 at 02:36:57PM +0000, Ard Biesheuvel wrote:
> The accelerated CRC32 module for ARM may use either the scalar CRC32
> instructions, the NEON 64x64 to 128 bit polynomial multiplication
> (vmull.p64) instruction, or both, depending on what the current CPU
> supports.
> 
> However, this also requires support in binutils, and as it turns out,
> versions of binutils exist that support the vmull.p64 instruction but
> not the crc32 instructions.
> 
> So refactor the Makefile logic so that this module only gets built if
> binutils has support for both.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Milan Broz @ 2017-03-01  9:29 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Binoy Jayan, Rajendra, Herbert Xu, Oded, Mike Snitzer,
	Linux kernel mailing list, Ondrej Mosnacek, linux-raid, dm-devel,
	Mark Brown, Arnd Bergmann, linux-crypto, Shaohua Li,
	David S. Miller, Alasdair Kergon, Ofir
In-Reply-To: <CAOtvUMf3c0fUjABmbfMrbs0W4gyhKVZyc_tZH3rQg65zF18YMg@mail.gmail.com>

On 03/01/2017 09:30 AM, Gilad Ben-Yossef wrote:
> On Tue, Feb 28, 2017 at 11:05 PM, Milan Broz <gmazyland@gmail.com> wrote:
>>
>> On 02/22/2017 07:12 AM, Binoy Jayan wrote:
>>>
>>> I was wondering if this is near to be ready for submission (apart from
>>> the testmgr.c
>>> changes) or I need to make some changes to make it similar to the IPSec offload?
>>
>> I just tried this and except it registers the IV for every new device again, it works...
>> (After a while you have many duplicate entries in /proc/crypto.)
>>
>> But I would like to see some summary why such a big patch is needed in the first place.
>> (During an internal discussions seems that people are already lost in mails and
>> patches here, so Ondra promised me to send some summary mail soon here.)
>>
>> IIRC the first initial problem was dmcrypt performance on some embedded
>> crypto processors that are not able to cope with small crypto requests effectively.
>>
>>
>> Do you have some real performance numbers that proves that such a patch is adequate?
>>
>> I would really like to see the performance issue fixed but I am really not sure
>> this approach works for everyone. It would be better to avoid repeating this exercise later.
>> IIRC Ondra's "bulk" mode, despite rejected, shows that there is a potential
>> to speedup things even for crypt drivers that do not support own IV generators.
>>
> 
> AFAIK the problem that we are trying to solve is that if the IV is
> generated outside the crypto API
> domain than you are forced to have an invocation of the crypto API per
> each block because you
> need to provide the IV for each block.
> 
> By putting the IV generation responsibility in the Crypto API we open
> the way to do a single invocation
> of the crypto API for a whole sequence of blocks.

Sure, but this is theory. Does it really work on some hw already?
Do you have performance measurements or comparison?

> For software implementation of XTS this doesn't matter much - but for
> hardware based XTS providers

It is not only embedded crypto, we have some more reports in the past
that 512B sectors are not ideal even for other systems.
(IIRC it was also with AES-NI that represents really big group of users).

> This lead some vendors to ship hacked up versions of dm-crypt to match
> the specific crypto hardware
> they were using, or so I've heard at least - didn't see the code myself.

I saw few version of that. There was a very hacky way to provide request-based dmcrypt
(see old "Introduce the request handling for dm-crypt" thread on dm-devel).
This is not the acceptable way but definitely it points to the same problem.

> I believe Binoy is trying to address this in a generic upstream worthy
> way instead.

IIRC the problem is performance, if we can solve it by some generic way,
good, but for now it seems to be a big change and just hope it helps later...

> Anyway, you are only supposed to see s difference when using a
> hardware based XTS provider algo
> that supports IV generation.
> 
>> I like the patch is now contained inside dmcrypt, but it still exposes IVs that
>> are designed just for old, insecure, compatibility-only containers.
>>
>> I really do not think every compatible crap must be accessible through crypto API.
>> (I wrote the dmcrypt lrw and tcw compatibility IVs and I would never do that this way
>> if I know it is accessible outside of dmcrypt internals...)
>> Even the ESSIV is something that was born to fix predictive IVs (CBC watermarking
>> attacks) for disk encryption only, no reason to expose it outside of disk encryption.
>>
> 
> The point is that you have more than one implementation of these
> "compatible crap" - the
> software implementation that you wrote and potentially multiple
> hardware implementations
> and putting this in the crypto API domain is the way to abstract this
> so you use the one
> that works best of your platform.

For XTS you need just simple linear IV. No problem with that, implementation
in crypto API and hw is trivial.

But for compatible IV (that provides compatibility with loopAES and very old TrueCrypt),
these should be never ever implemented again anywhere.

Specifically "tcw" is broken, insecure and provided here just to help people to migrate
from old Truecrypt containers. Even Truecrypt followers removed it from the codebase.
(It is basically combination of IV and slight modification of CBC mode. All
recent version switched to XTS and plain IV.)

So building abstraction over something known to be broken and that is now intentionally
isolated inside dmcrypt is, in my opinion, really not a good idea.


But please do get me wrong,  I do not want to block any improvement.

But it seems to me that this thread focused on creating nice crypto API interface
for FDE IVs instead of demonstration that the proposed solution really solves
the performance issue.
And not only for your hw driver, maybe other systems could benefit from the better
processing of small requests as well.

Milan

^ permalink raw reply

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Gilad Ben-Yossef @ 2017-03-01  8:30 UTC (permalink / raw)
  To: Milan Broz
  Cc: Binoy Jayan, Rajendra, Herbert Xu, Oded, Mike Snitzer,
	Linux kernel mailing list, Ondrej Mosnacek, linux-raid, dm-devel,
	Mark Brown, Arnd Bergmann, linux-crypto, Shaohua Li,
	David S. Miller, Alasdair Kergon, Ofir
In-Reply-To: <68f70534-a309-46ba-a84d-8acc1e6620e5@gmail.com>

On Tue, Feb 28, 2017 at 11:05 PM, Milan Broz <gmazyland@gmail.com> wrote:
>
> On 02/22/2017 07:12 AM, Binoy Jayan wrote:
> >
> > I was wondering if this is near to be ready for submission (apart from
> > the testmgr.c
> > changes) or I need to make some changes to make it similar to the IPSec offload?
>
> I just tried this and except it registers the IV for every new device again, it works...
> (After a while you have many duplicate entries in /proc/crypto.)
>
> But I would like to see some summary why such a big patch is needed in the first place.
> (During an internal discussions seems that people are already lost in mails and
> patches here, so Ondra promised me to send some summary mail soon here.)
>
> IIRC the first initial problem was dmcrypt performance on some embedded
> crypto processors that are not able to cope with small crypto requests effectively.
>
>
> Do you have some real performance numbers that proves that such a patch is adequate?
>
> I would really like to see the performance issue fixed but I am really not sure
> this approach works for everyone. It would be better to avoid repeating this exercise later.
> IIRC Ondra's "bulk" mode, despite rejected, shows that there is a potential
> to speedup things even for crypt drivers that do not support own IV generators.
>

AFAIK the problem that we are trying to solve is that if the IV is
generated outside the crypto API
domain than you are forced to have an invocation of the crypto API per
each block because you
need to provide the IV for each block.

By putting the IV generation responsibility in the Crypto API we open
the way to do a single invocation
of the crypto API for a whole sequence of blocks.

For software implementation of XTS this doesn't matter much - but for
hardware based XTS providers
that can do the IV generation themselves it's the difference between a
sequence of small invocation,
with all the overhead that that entails  and a single big invocatio -
and this can be big.

This lead some vendors to ship hacked up versions of dm-crypt to match
the specific crypto hardware
they were using, or so I've heard at least - didn't see the code myself.

I believe Binoy is trying to address this in a generic upstream worthy
way instead.

Anyway, you are only supposed to see s difference when using a
hardware based XTS provider algo
that supports IV generation.

> I like the patch is now contained inside dmcrypt, but it still exposes IVs that
> are designed just for old, insecure, compatibility-only containers.
>
> I really do not think every compatible crap must be accessible through crypto API.
> (I wrote the dmcrypt lrw and tcw compatibility IVs and I would never do that this way
> if I know it is accessible outside of dmcrypt internals...)
> Even the ESSIV is something that was born to fix predictive IVs (CBC watermarking
> attacks) for disk encryption only, no reason to expose it outside of disk encryption.
>

The point is that you have more than one implementation of these
"compatible crap" - the
software implementation that you wrote and potentially multiple
hardware implementations
and putting this in the crypto API domain is the way to abstract this
so you use the one
that works best of your platform.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

^ permalink raw reply

* Re: Problem with RSA test from testmgr
From: Stephan Müller @ 2017-02-28 22:35 UTC (permalink / raw)
  To: Corentin Labbe; +Cc: herbert, linux-crypto, linux-kernel
In-Reply-To: <20170228164553.GA2155@Red>

Am Dienstag, 28. Februar 2017, 17:45:53 CET schrieb Corentin Labbe:

Hi Corentin,

> On Tue, Feb 28, 2017 at 05:08:35PM +0100, Stephan Müller wrote:
> > Am Dienstag, 28. Februar 2017, 16:59:53 CET schrieb Corentin Labbe:
> > 
> > Hi Corentin,
> > 
> > > hello
> > > 
> > > I work on the sun8i-ce crypto accelerator and I have some problem with
> > > the
> > > RSA part.
> > > 
> > > The RSA register fail at the first RSA test (encrypt 512bit) with this
> > > output: [ 8480.146843] alg: akcipher: encrypt test failed. Invalid
> > > output
> > > [ 8480.146871] 00000000: 6e 7c 8a 75 e7 30 80 d1 5e ab 9b db a2 cf ed db
> > > [ 8480.146897] 00000010: c9 b2 db 43 bd 9a b9 75 27 f3 73 d9 73 b7 81 8c
> > > [ 8480.146921] 00000020: 49 e8 45 fc 43 44 f5 6d f0 f7 b8 f2 ae 6b ae 49
> > > [ 8480.146946] 00000030: 1b 8e 50 c6 88 4e 99 09 78 14 f2 5d 99 c3 7f f9
> > > [ 8480.146995] alg: akcipher: test 1 failed for rsa-sun8i-ce, err=-22
> > > 
> > > But with the same parameters (msg, n, e) openssl give me exactly this
> > > output.
> > > 
> > > So what I miss for made it work ?
> > > In which format testmgr expect the output data ?
> > 
> > The output should be simply the binary string from the modular
> > exponentiation operation.
> > 
> > What I am wondering is: the output logged above is not found in the
> > expected values of testmgr.h. Which input data or test vectors do you
> > use?
> > 
> > Ciao
> > Stephan
> 
> I use the first test from rsa_tv_template in crypto/testmgr.h
> The test fail on the encrypt operation.
> 
> I have put below the openssl program that give me the same output than my
> hardware accelerator with the same parameters.

I would think the issue is that the OpenSSL BIGNUM lib has some issues: when 
calculating m^e mod n, m has to be equal to the key size. The kernel's MPI 
code handles the case where m is smaller than the key size.

Note, in your code below, ptext is the 8 bytes from ptext_ex plus trailing 
zeroes whereas the kernel uses just the 8 bytes.

It seems that your implementation has the same issue.

What about the following test: change vector->m to be 64 bytes (i.e. 
RSA_size(key) in size in testmgr.h and check the output of crypto/rsa.c, 
openssl's output with the app below and your RSA hardware.
> 
> Regards
> 
> 
> #include <stdio.h>
> #include <string.h>
> #include <openssl/crypto.h>
> #include <openssl/err.h>
> #include <openssl/bn.h>
> #include <openssl/rsa.h>
> 
> static const unsigned char n[] =
> "\x00\xAA\x36\xAB\xCE\x88\xAC\xFD\xFF\x55\x52\x3C\x7F\xC4\x52\x3F"
> "\x90\xEF\xA0\x0D\xF3\x77\x4A\x25\x9F\x2E\x62\xB4\xC5\xD9\x9C\xB5"
> "\xAD\xB3\x00\xA0\x28\x5E\x53\x01\x93\x0E\x0C\x70\xFB\x68\x76\x93"
> "\x9C\xE6\x16\xCE\x62\x4A\x11\xE0\x08\x6D\x34\x1E\xBC\xAC\xA0\xA1"
> "\xF5";
> static const unsigned char e[] = "\x11";
> 
> int main(int argc, char *argv[])
> {
> 	static unsigned char ptext_ex[] = "\x54\x85\x9b\x34\x2c\x49\xea\x2a";
> 	RSA *key;
> 	int num, i;
> 	int plen = sizeof(ptext_ex) - 1;
> 	unsigned char *ctext = malloc(256);
> 	unsigned char *ptext = malloc(256);
> 	unsigned char *ptextp = malloc(256);
> 
> 	CRYPTO_malloc_debug_init();
> 	CRYPTO_dbg_set_options(V_CRYPTO_MDEBUG_ALL);
>         CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
> 
> 	memset(ptextp, 0, 256);
> 	memcpy(ptextp, ptext_ex, plen);
> 
> 	key = RSA_new();
> 
> 	key->n = BN_bin2bn(n, sizeof(n)-1, key->n);
> 	key->e = BN_bin2bn(e, sizeof(e)-1, key->e);
> 
> 	num = RSA_public_encrypt(RSA_size(key), ptextp, ctext, key,
> RSA_NO_PADDING);
> 
> 	printf("Result %d plen=%d\n", num, plen);
> 	for (i = 0; i < num; i++)
> 		printf("%02x ", ctext[i]);
> 	printf("\n");
> 	return 0;
> }


Ciao
Stephan

^ permalink raw reply

* [PATCH] crypto: Pad aes_ccm_enc_tv_template vector
From: Laura Abbott @ 2017-02-28 22:07 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Laura Abbott, linux-crypto, linux-kernel, Ard Biesheuvel

Running with KASAN and crypto tests currently gives

 BUG: KASAN: global-out-of-bounds in __test_aead+0x9d9/0x2200 at addr ffffffff8212fca0
 Read of size 16 by task cryptomgr_test/1107
 Address belongs to variable 0xffffffff8212fca0
 CPU: 0 PID: 1107 Comm: cryptomgr_test Not tainted 4.10.0+ #45
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.1-1.fc24 04/01/2014
 Call Trace:
  dump_stack+0x63/0x8a
  kasan_report.part.1+0x4a7/0x4e0
  ? __test_aead+0x9d9/0x2200
  ? crypto_ccm_init_crypt+0x218/0x3c0 [ccm]
  kasan_report+0x20/0x30
  check_memory_region+0x13c/0x1a0
  memcpy+0x23/0x50
  __test_aead+0x9d9/0x2200
  ? kasan_unpoison_shadow+0x35/0x50
  ? alg_test_akcipher+0xf0/0xf0
  ? crypto_skcipher_init_tfm+0x2e3/0x310
  ? crypto_spawn_tfm2+0x37/0x60
  ? crypto_ccm_init_tfm+0xa9/0xd0 [ccm]
  ? crypto_aead_init_tfm+0x7b/0x90
  ? crypto_alloc_tfm+0xc4/0x190
  test_aead+0x28/0xc0
  alg_test_aead+0x54/0xd0
  alg_test+0x1eb/0x3d0
  ? alg_find_test+0x90/0x90
  ? __sched_text_start+0x8/0x8
  ? __wake_up_common+0x70/0xb0
  cryptomgr_test+0x4d/0x60
  kthread+0x173/0x1c0
  ? crypto_acomp_scomp_free_ctx+0x60/0x60
  ? kthread_create_on_node+0xa0/0xa0
  ret_from_fork+0x2c/0x40
 Memory state around the buggy address:
  ffffffff8212fb80: 00 00 00 00 01 fa fa fa fa fa fa fa 00 00 00 00
  ffffffff8212fc00: 00 01 fa fa fa fa fa fa 00 00 00 00 01 fa fa fa
 >ffffffff8212fc80: fa fa fa fa 00 05 fa fa fa fa fa fa 00 00 00 00
                                   ^
  ffffffff8212fd00: 01 fa fa fa fa fa fa fa 00 00 00 00 01 fa fa fa
  ffffffff8212fd80: fa fa fa fa 00 00 00 00 00 05 fa fa fa fa fa fa

This always happens on the same IV which is less than 16 bytes.

Per Ard,

"CCM IVs are 16 bytes, but due to the way they are constructed
internally, the final couple of bytes of input IV are dont-cares.

Apparently, we do read all 16 bytes, which triggers the KASAN errors."

Fix this by padding the IV with null bytes to be at least 16 bytes.

Fixes: 0bc5a6c5c79a ("crypto: testmgr - Disable rfc4309 test and convert
test vectors")
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 crypto/testmgr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 006ecc4..03f4731 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -22691,7 +22691,7 @@ static struct aead_testvec aes_ccm_enc_tv_template[] = {
 			  "\x09\x75\x9a\x9b\x3c\x9b\x27\x39",
 		.klen	= 32,
 		.iv	= "\x03\xf9\xd9\x4e\x63\xb5\x3d\x9d"
-			  "\x43\xf6\x1e\x50",
+			  "\x43\xf6\x1e\x50\0\0\0\0",
 		.assoc	= "\x57\xf5\x6b\x8b\x57\x5c\x3d\x3b"
 			  "\x13\x02\x01\x0c\x83\x4c\x96\x35"
 			  "\x8e\xd6\x39\xcf\x7d\x14\x9b\x94"
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Milan Broz @ 2017-02-28 21:05 UTC (permalink / raw)
  To: Binoy Jayan
  Cc: Rajendra, Herbert Xu, Oded, Mike Snitzer,
	Linux kernel mailing list, Ondrej Mosnacek, linux-raid,
	Gilad Ben-Yossef, dm-devel, Mark Brown, Arnd Bergmann,
	linux-crypto, Shaohua Li, David S. Miller, Alasdair Kergon, Ofir
In-Reply-To: <CAHv-k_8rgArzV2uQ64h1ZTNRpssX1jMf=RwuPoBmsjQ0FhCsWA@mail.gmail.com>

On 02/22/2017 07:12 AM, Binoy Jayan wrote:
> 
> I was wondering if this is near to be ready for submission (apart from
> the testmgr.c
> changes) or I need to make some changes to make it similar to the IPSec offload?

I just tried this and except it registers the IV for every new device again, it works...
(After a while you have many duplicate entries in /proc/crypto.)

But I would like to see some summary why such a big patch is needed in the first place.
(During an internal discussions seems that people are already lost in mails and
patches here, so Ondra promised me to send some summary mail soon here.)

IIRC the first initial problem was dmcrypt performance on some embedded
crypto processors that are not able to cope with small crypto requests effectively.

Do you have some real performance numbers that proves that such a patch is adequate?

I would really like to see the performance issue fixed but I am really not sure
this approach works for everyone. It would be better to avoid repeating this exercise later.
IIRC Ondra's "bulk" mode, despite rejected, shows that there is a potential
to speedup things even for crypt drivers that do not support own IV generators.

I like the patch is now contained inside dmcrypt, but it still exposes IVs that
are designed just for old, insecure, compatibility-only containers.

I really do not think every compatible crap must be accessible through crypto API.
(I wrote the dmcrypt lrw and tcw compatibility IVs and I would never do that this way
if I know it is accessible outside of dmcrypt internals...)
Even the ESSIV is something that was born to fix predictive IVs (CBC watermarking
attacks) for disk encryption only, no reason to expose it outside of disk encryption.

Milan

^ permalink raw reply

* Re: KASAN errors after 21c8e72037fb ("crypto: testmgr - use calculated count for number of test vectors")
From: Ard Biesheuvel @ 2017-02-28 20:37 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Herbert Xu, linux-crypto@vger.kernel.org,
	Linux Kernel Mailing List
In-Reply-To: <d4b417e1-f34b-f706-8806-4dd3814d1f1f@redhat.com>

On 28 February 2017 at 20:17, Laura Abbott <labbott@redhat.com> wrote:
> Hi,
>
> While attempting to debug something else, I saw KASAN errors
> from the crypto test with ccm:
>
> [   28.231615] ==================================================================
> [   28.232007] BUG: KASAN: global-out-of-bounds in __test_aead+0x9d9/0x2200 at addr ffffffff8212fca0
> [   28.232007] Read of size 16 by task cryptomgr_test/1107
> [   28.232007] Address belongs to variable 0xffffffff8212fca0
> [   28.232007] CPU: 0 PID: 1107 Comm: cryptomgr_test Not tainted 4.10.0+ #45
> [   28.232007] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.1-1.fc24 04/01/2014
> [   28.232007] Call Trace:
> [   28.232007]  dump_stack+0x63/0x8a
> [   28.232007]  kasan_report.part.1+0x4a7/0x4e0
> [   28.232007]  ? __test_aead+0x9d9/0x2200
> [   28.232007]  ? crypto_ccm_init_crypt+0x218/0x3c0 [ccm]
> [   28.232007]  kasan_report+0x20/0x30
> [   28.232007]  check_memory_region+0x13c/0x1a0
> [   28.232007]  memcpy+0x23/0x50
> [   28.232007]  __test_aead+0x9d9/0x2200
> [   28.232007]  ? kasan_unpoison_shadow+0x35/0x50
> [   28.232007]  ? alg_test_akcipher+0xf0/0xf0
> [   28.232007]  ? crypto_skcipher_init_tfm+0x2e3/0x310
> [   28.232007]  ? crypto_spawn_tfm2+0x37/0x60
> [   28.232007]  ? crypto_ccm_init_tfm+0xa9/0xd0 [ccm]
> [   28.232007]  ? crypto_aead_init_tfm+0x7b/0x90
> [   28.232007]  ? crypto_alloc_tfm+0xc4/0x190
> [   28.232007]  test_aead+0x28/0xc0
> [   28.232007]  alg_test_aead+0x54/0xd0
> [   28.232007]  alg_test+0x1eb/0x3d0
> [   28.232007]  ? alg_find_test+0x90/0x90
> [   28.232007]  ? __sched_text_start+0x8/0x8
> [   28.232007]  ? __wake_up_common+0x70/0xb0
> [   28.232007]  cryptomgr_test+0x4d/0x60
> [   28.232007]  kthread+0x173/0x1c0
> [   28.232007]  ? crypto_acomp_scomp_free_ctx+0x60/0x60
> [   28.232007]  ? kthread_create_on_node+0xa0/0xa0
> [   28.232007]  ret_from_fork+0x2c/0x40
> [   28.232007] Memory state around the buggy address:
> [   28.232007]  ffffffff8212fb80: 00 00 00 00 01 fa fa fa fa fa fa fa 00 00 00 00
> [   28.232007]  ffffffff8212fc00: 00 01 fa fa fa fa fa fa 00 00 00 00 01 fa fa fa
> [   28.232007] >ffffffff8212fc80: fa fa fa fa 00 05 fa fa fa fa fa fa 00 00 00 00
> [   28.232007]                                   ^
> [   28.232007]  ffffffff8212fd00: 01 fa fa fa fa fa fa fa 00 00 00 00 01 fa fa fa
> [   28.232007]  ffffffff8212fd80: fa fa fa fa 00 00 00 00 00 05 fa fa fa fa fa fa
> [   28.232007] ==================================================================
>
> This seems to be caused by 21c8e72037fb ("crypto: testmgr -
> use calculated count for number of test vectors") but I think
> it's because it  now exposes this test vector which always causes the failure.
>
> {
>                 .key    = "\x7c\xc8\x18\x3b\x8d\x99\xe0\x7c"
>                           "\x45\x41\xb8\xbd\x5c\xa7\xc2\x32"
>                           "\x8a\xb8\x02\x59\xa4\xfe\xa9\x2c"
>                           "\x09\x75\x9a\x9b\x3c\x9b\x27\x39",
>                 .klen   = 32,
>                 .iv     = "\x03\xf9\xd9\x4e\x63\xb5\x3d\x9d"
>                           "\x43\xf6\x1e\x50",
>                 .assoc  = "\x57\xf5\x6b\x8b\x57\x5c\x3d\x3b"
>                           "\x13\x02\x01\x0c\x83\x4c\x96\x35"
>                           "\x8e\xd6\x39\xcf\x7d\x14\x9b\x94"
>                           "\xb0\x39\x36\xe6\x8f\x57\xe0\x13",
>                 .alen   = 32,
>                 .input  = "\x3b\x6c\x29\x36\xb6\xef\x07\xa6"
>                           "\x83\x72\x07\x4f\xcf\xfa\x66\x89"
>                           "\x5f\xca\xb1\xba\xd5\x8f\x2c\x27"
>                           "\x30\xdb\x75\x09\x93\xd4\x65\xe4",
>                 .ilen   = 32,
>                 .result = "\xb0\x88\x5a\x33\xaa\xe5\xc7\x1d"
>                           "\x85\x23\xc7\xc6\x2f\xf4\x1e\x3d"
>                           "\xcc\x63\x44\x25\x07\x78\x4f\x9e"
>                           "\x96\xb8\x88\xeb\xbc\x48\x1f\x06"
>                           "\x39\xaf\x39\xac\xd8\x4a\x80\x39"
>                           "\x7b\x72\x8a\xf7",
>                 .rlen   = 44,
>         },
>
>
> If I pad iv with extra NULL bytes the KASAN error goes away.
>
> Thoughts?
>

CCM IVs are 16 bytes, but due to the way they are constructed
internally, the final couple of bytes of input IV are dont-cares.

Apparently, we do read all 16 bytes, which triggers the KASAN errors.

So adding any kind of padding bytes to pad to length 16 should fix the
issue, and so your proposed fix is correct.

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox