Re: crypto: caam from tasklet to threadirq

[Russell King - ARM Linux]

Okay, I've re-tested, using a different way of measuring, because using
openssl speed is impractical for off-loaded engines.  I've decided to
use this way to measure the performance:

dd if=/dev/zero bs=1048576 count=128 | /usr/bin/time openssl dgst -md5

For the threaded IRQs case gives:

0.05user 2.74system 0:05.30elapsed 52%CPU (0avgtext+0avgdata 2400maxresident)k
0.06user 2.52system 0:05.18elapsed 49%CPU (0avgtext+0avgdata 2404maxresident)k
0.12user 2.60system 0:05.61elapsed 48%CPU (0avgtext+0avgdata 2460maxresident)k
=> 5.36s => 25.0MB/s

and the tasklet case:

0.08user 2.53system 0:04.83elapsed 54%CPU (0avgtext+0avgdata 2468maxresident)k
0.09user 2.47system 0:05.16elapsed 49%CPU (0avgtext+0avgdata 2368maxresident)k
0.10user 2.51system 0:04.87elapsed 53%CPU (0avgtext+0avgdata 2460maxresident)k
=> 4.95 => 27.1MB/s

which corresponds to an 8% slowdown for the threaded IRQ case.  So,
tasklets are indeed faster than threaded IRQs.

I guess the reason is that tasklets are much simpler, being able to
run just before we return to userspace without involving scheduler
overheads, but that's speculation.

I've tried to perf it, but...

Samples: 31K of event 'cycles', Event count (approx.): 3552246846
  Overhead  Command  Shared Object Symbol
+   33.22%  kworker/0:1  [kernel.vmlinux]  [k] __do_softirq
+   15.78%  irq/311-2101000  [kernel.vmlinux]  [k] __do_softirq
+7.49%  irqbalance   [kernel.vmlinux]  [k] __do_softirq
+7.26%  openssl  [kernel.vmlinux]  [k] __do_softirq
+5.71%  ksoftirqd/0  [kernel.vmlinux]  [k] __do_softirq
+3.64%  kworker/0:2  [kernel.vmlinux]  [k] __do_softirq
+3.52%  swapper  [kernel.vmlinux]  [k] __do_softirq
+3.14%  kworker/0:1  [kernel.vmlinux]  [k] _raw_spin_unlock_irq

I was going to try to get the threaded IRQ case, but I've ended up with
perf getting buggered because of the iMX6 SMP perf disfunctionality:

[ 3448.810416] irq 24: nobody cared (try booting with the "irqpoll" option)
...
[ 3448.824528] Disabling IRQ #24

caused by FSL's utterly brain-dead idea of routing all the perf
interrupts to single non-CPU local interrupt input, and the refusal of
kernel folk to find an acceptable solution to support this.

So, sorry, I'm not going to bother trying to get any further with this.
If the job was not made harder stupid hardware design and kernel
politics, then I might be more inclined to do deeper investigation, but
right now I'm finding that I'm not interested in trying to jump through
these stupid hoops.

I think I've proven from the above that this patch needs to be reverted
due to the performance regression, and that there _is_ most definitely
a deterimental effect of switching from tasklets to threaded IRQs.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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] hwrng: omap - Only fail if pm_runtime_get_sync returns < 0

[Dave Gerlach]

Currently omap-rng checks the return value of pm_runtime_get_sync and
reports failure if anything is returned, however it should be checking
if ret < 0 as pm_runtime_get_sync return 0 on success but also can return
1 if the device was already active which is not a failure case. Only
values < 0 are actual failures.

Fixes: 61dc0a446e5d ("hwrng: omap - Fix assumption that runtime_get_sync will 
always succeed")
Signed-off-by: Dave Gerlach 
---
 drivers/char/hw_random/omap-rng.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c 
b/drivers/char/hw_random/omap-rng.c
index 01d4be2c354b..f5c26a5f6875 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -385,7 +385,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 
pm_runtime_enable(>dev);
ret = pm_runtime_get_sync(>dev);
-   if (ret) {
+   if (ret < 0) {
dev_err(>dev, "Failed to runtime_get device: %d\n", ret);
pm_runtime_put_noidle(>dev);
goto err_ioremap;
@@ -443,7 +443,7 @@ static int __maybe_unused omap_rng_resume(struct device 
*dev)
int ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret) {
+   if (ret < 0) {
dev_err(dev, "Failed to runtime_get device: %d\n", ret);
pm_runtime_put_noidle(dev);
return ret;
-- 
2.9.3

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

Re: [PATCH] trusted-keys: skcipher bug info

[Mimi Zohar]

On Tue, 2016-09-20 at 20:35 +0800, Herbert Xu wrote:
> On Tue, Sep 20, 2016 at 08:11:51AM -0400, Mimi Zohar wrote:
> > Hi Herbert,
> > 
> > The initial random iv value, initialized in encrypted_init(), should
> > not be modified.  Commit c3917fd "KEYS: Use skcipher", which replaced
> > the blkcipher with skcipher, modifies the iv in
> > crypto_skcipher_encrypt()/decrypt().
> > 
> > The following example creates an encrypted key, writes the key to a
> > file, and then loads the key from the file.  To illustrate the problem,
> > this patch provides crypto_skcipher_encrypt()/decrypt() with a copy of
> > the iv.  With this change, the resulting test-key and test-key1 keys
> > are the same.
> 
> Sorry, I missed the subtlety.  This patch should fix the problem.

Thanks!

Mimi
> 
> ---8<---
> Subject: KEYS: Fix skcipher IV clobbering
> 
> The IV must not be modified by the skcipher operation so we need
> to duplicate it.
> 
> Fixes: c3917fd9dfbc ("KEYS: Use skcipher")
> Cc: sta...@vger.kernel.org
> Reported-by: Mimi Zohar 
> Signed-off-by: Herbert Xu 
> 
> diff --git a/security/keys/encrypted-keys/encrypted.c 
> b/security/keys/encrypted-keys/encrypted.c
> index 5adbfc3..17a0610 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -478,6 +479,7 @@ static int derived_key_encrypt(struct 
> encrypted_key_payload *epayload,
>   struct crypto_skcipher *tfm;
>   struct skcipher_request *req;
>   unsigned int encrypted_datalen;
> + u8 iv[AES_BLOCK_SIZE];
>   unsigned int padlen;
>   char pad[16];
>   int ret;
> @@ -500,8 +502,8 @@ static int derived_key_encrypt(struct 
> encrypted_key_payload *epayload,
>   sg_init_table(sg_out, 1);
>   sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen);
> 
> - skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen,
> -epayload->iv);
> + memcpy(iv, epayload->iv, sizeof(iv));
> + skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
>   ret = crypto_skcipher_encrypt(req);
>   tfm = crypto_skcipher_reqtfm(req);
>   skcipher_request_free(req);
> @@ -581,6 +583,7 @@ static int derived_key_decrypt(struct 
> encrypted_key_payload *epayload,
>   struct crypto_skcipher *tfm;
>   struct skcipher_request *req;
>   unsigned int encrypted_datalen;
> + u8 iv[AES_BLOCK_SIZE];
>   char pad[16];
>   int ret;
> 
> @@ -599,8 +602,8 @@ static int derived_key_decrypt(struct 
> encrypted_key_payload *epayload,
>  epayload->decrypted_datalen);
>   sg_set_buf(_out[1], pad, sizeof pad);
> 
> - skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen,
> -epayload->iv);
> + memcpy(iv, epayload->iv, sizeof(iv));
> + skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
>   ret = crypto_skcipher_decrypt(req);
>   tfm = crypto_skcipher_reqtfm(req);
>   skcipher_request_free(req);
> 


--
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 v5] KEYS: add SP800-56A KDF support for DH

[Stephan Mueller]

Am Freitag, 19. August 2016, 20:39:09 CEST schrieb Stephan Mueller:

Hi David,

> 
> SP800-56A defines the use of DH with key derivation function based on a
> counter. The input to the KDF is defined as (DH shared secret || other
> information). The value for the "other information" is to be provided by
> the caller.
> 
> The KDF is implemented using the hash support from the kernel crypto API.
> The implementation uses the symmetric hash support as the input to the
> hash operation is usually very small. The caller is allowed to specify
> the hash name that he wants to use to derive the key material allowing
> the use of all supported hashes provided with the kernel crypto API.
> 
> As the KDF implements the proper truncation of the DH shared secret to
> the requested size, this patch fills the caller buffer up to its size.
> 
> The patch is tested with a new test added to the keyutils user space
> code which uses a CAVS test vector testing the compliance with
> SP800-56A.

Is there a decision about this patch set?

Ciao
Stephan
--
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] trusted-keys: skcipher bug info

[Herbert Xu]

On Tue, Sep 20, 2016 at 08:11:51AM -0400, Mimi Zohar wrote:
> Hi Herbert,
> 
> The initial random iv value, initialized in encrypted_init(), should
> not be modified.  Commit c3917fd "KEYS: Use skcipher", which replaced
> the blkcipher with skcipher, modifies the iv in
> crypto_skcipher_encrypt()/decrypt().
> 
> The following example creates an encrypted key, writes the key to a
> file, and then loads the key from the file.  To illustrate the problem,
> this patch provides crypto_skcipher_encrypt()/decrypt() with a copy of
> the iv.  With this change, the resulting test-key and test-key1 keys
> are the same.

Sorry, I missed the subtlety.  This patch should fix the problem.

---8<---
Subject: KEYS: Fix skcipher IV clobbering

The IV must not be modified by the skcipher operation so we need
to duplicate it.

Fixes: c3917fd9dfbc ("KEYS: Use skcipher")
Cc: sta...@vger.kernel.org
Reported-by: Mimi Zohar 
Signed-off-by: Herbert Xu 

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 5adbfc3..17a0610 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -478,6 +479,7 @@ static int derived_key_encrypt(struct encrypted_key_payload 
*epayload,
struct crypto_skcipher *tfm;
struct skcipher_request *req;
unsigned int encrypted_datalen;
+   u8 iv[AES_BLOCK_SIZE];
unsigned int padlen;
char pad[16];
int ret;
@@ -500,8 +502,8 @@ static int derived_key_encrypt(struct encrypted_key_payload 
*epayload,
sg_init_table(sg_out, 1);
sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen);
 
-   skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen,
-  epayload->iv);
+   memcpy(iv, epayload->iv, sizeof(iv));
+   skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
ret = crypto_skcipher_encrypt(req);
tfm = crypto_skcipher_reqtfm(req);
skcipher_request_free(req);
@@ -581,6 +583,7 @@ static int derived_key_decrypt(struct encrypted_key_payload 
*epayload,
struct crypto_skcipher *tfm;
struct skcipher_request *req;
unsigned int encrypted_datalen;
+   u8 iv[AES_BLOCK_SIZE];
char pad[16];
int ret;
 
@@ -599,8 +602,8 @@ static int derived_key_decrypt(struct encrypted_key_payload 
*epayload,
   epayload->decrypted_datalen);
sg_set_buf(_out[1], pad, sizeof pad);
 
-   skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen,
-  epayload->iv);
+   memcpy(iv, epayload->iv, sizeof(iv));
+   skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv);
ret = crypto_skcipher_decrypt(req);
tfm = crypto_skcipher_reqtfm(req);
skcipher_request_free(req);

-- 
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 v7 4/9] crypto: acomp - add support for lzo via scomp

[Giovanni Cabiddu]

Hi Herbert,

apologies for the duplicate. The previous email didn't get delivered to
the ML.

On Tue, Sep 20, 2016 at 05:26:18PM +0800, Herbert Xu wrote:
> Rather than duplicating the scratch buffer handling in every scomp
> algorithm, let's centralize this and put it into scomp.c.
Are you suggesting to hide the scratch buffers from the scomp
implementations and allocate them inside crypto_register_scomp?
Does that mean we have to go back to a scomp_alg with a flat buffer API
and linearize inside scomp?
If we take this direction, how do we support DMA from scomp
implementation? Scratch buffers are allocated using vmalloc.

Thanks,

--
Giovanni
--
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] trusted-keys: skcipher bug info

[Mimi Zohar]

Hi Herbert,

The initial random iv value, initialized in encrypted_init(), should
not be modified.  Commit c3917fd "KEYS: Use skcipher", which replaced
the blkcipher with skcipher, modifies the iv in
crypto_skcipher_encrypt()/decrypt().

The following example creates an encrypted key, writes the key to a
file, and then loads the key from the file.  To illustrate the problem,
this patch provides crypto_skcipher_encrypt()/decrypt() with a copy of
the iv.  With this change, the resulting test-key and test-key1 keys
are the same.

keyctl add user kmk "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u
keyctl pipe `keyctl search @u user kmk` > ~/tmp/kmk
keyctl add encrypted test-key "new user:kmk 64" @u
keyctl pipe `keyctl search @u encrypted test-key` > /tmp/test-key1
keyctl add encrypted test-key1 "load `cat /tmp/test-key1`" @u

keyctl print `keyctl search @u encrypted test-key`
keyctl print `keyctl search @u encrypted test-key1`

Either the underlying problem should be fixed or commit c3917fd 
"KEYS: Use skcipher" should be reverted.

thanks,

Mimi

---
 security/keys/encrypted-keys/encrypted.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 5adbfc3..e94f586 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -480,6 +480,7 @@ static int derived_key_encrypt(struct encrypted_key_payload 
*epayload,
unsigned int encrypted_datalen;
unsigned int padlen;
char pad[16];
+   u8 iv[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -500,9 +501,19 @@ static int derived_key_encrypt(struct 
encrypted_key_payload *epayload,
sg_init_table(sg_out, 1);
sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen);
 
+   memcpy(iv, epayload->iv, 16);   /* iv is modified */
skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen,
-  epayload->iv);
+  iv);
+print_hex_dump(KERN_INFO, "original iv: ", DUMP_PREFIX_NONE, 32, 1,
+  epayload->iv, ivsize, 0);
+print_hex_dump(KERN_INFO, "copied iv: ", DUMP_PREFIX_NONE, 32, 1,
+  iv, ivsize, 0);
ret = crypto_skcipher_encrypt(req);
+print_hex_dump(KERN_INFO, "original iv: ", DUMP_PREFIX_NONE, 32, 1,
+  epayload->iv, ivsize, 0);
+print_hex_dump(KERN_INFO, "modified iv: ", DUMP_PREFIX_NONE, 32, 1,
+  iv, ivsize, 0);
+
tfm = crypto_skcipher_reqtfm(req);
skcipher_request_free(req);
crypto_free_skcipher(tfm);
@@ -582,6 +593,7 @@ static int derived_key_decrypt(struct encrypted_key_payload 
*epayload,
struct skcipher_request *req;
unsigned int encrypted_datalen;
char pad[16];
+   u8 iv[16];
int ret;
 
encrypted_datalen = roundup(epayload->decrypted_datalen, blksize);
@@ -599,8 +611,9 @@ static int derived_key_decrypt(struct encrypted_key_payload 
*epayload,
   epayload->decrypted_datalen);
sg_set_buf(_out[1], pad, sizeof pad);
 
+   memcpy(iv, epayload->iv, 16);   /* iv is modified */
skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen,
-  epayload->iv);
+  iv);
ret = crypto_skcipher_decrypt(req);
tfm = crypto_skcipher_reqtfm(req);
skcipher_request_free(req);
@@ -778,8 +791,11 @@ static int encrypted_init(struct encrypted_key_payload 
*epayload,
 
get_random_bytes(epayload->decrypted_data,
 epayload->decrypted_datalen);
-   } else
+   } else {
ret = encrypted_key_decrypt(epayload, format, hex_encoded_iv);
+print_hex_dump(KERN_INFO, "init iv: ", DUMP_PREFIX_NONE, 32, 1,
+  epayload->iv, ivsize, 0);
+   }
return ret;
 }
 
-- 
2.7.4



--
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 v7 4/9] crypto: acomp - add support for lzo via scomp

[Herbert Xu]

On Tue, Sep 13, 2016 at 01:49:36PM +0100, Giovanni Cabiddu wrote:
>
> + lzo_src_scratches = crypto_scomp_alloc_scratches(LZO_SCRATCH_SIZE);
> + if (!lzo_src_scratches)
> + return -ENOMEM;

Rather than duplicating the scratch buffer handling in every scomp
algorithm, let's centralize this and put it into scomp.c.

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