Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: crypto: tcrypt - Do not bail on EINPROGRESS in multibuffer hash test
From: Megha Dey @ 2016-06-30 17:36 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Krzysztof Kozlowski, David S. Miller, linux-crypto, linux-kernel,
	Bartlomiej Zolnierkiewicz, Fenghua Yu, Tim Chen
In-Reply-To: <20160630030013.GA535@gondor.apana.org.au>

On Thu, 2016-06-30 at 11:00 +0800, Herbert Xu wrote:
> On Wed, Jun 29, 2016 at 10:45:56AM -0700, Megha Dey wrote:
> > I tested the latest cryptodev tree on my haswell machine and this is
> > what I see:
> > [   40.402834] modprobe tcrypt mode=422
> > [   40.403105] testing speed of multibuffer sha1 (sha1_mb)
> > [   40.403108] test  0 (   16 byte blocks,   16 bytes per update,   1
> > updates):  32271 cycles/operation,  252 cycles/byte
> > [   40.403118] At least one hashing failed ret=-115
> > [   43.218712] modprobe tcrypt mode=423
> > [   43.218712] testing speed of multibuffer sha256 (sha256_mb)
> > [   43.218715] test  0 (   16 byte blocks,   16 bytes per update,   1
> > updates): 106965 cycles/operation,  835 cycles/byte
> > [   43.218747] At least one hashing failed ret=-115
> > [   45.346657] modprobe tcrypt mode=424
> > [   45.346657] testing speed of multibuffer sha512 (sha512_mb)
> > [   45.346660] test  0 (   16 byte blocks,   16 bytes per update,   1
> > updates):  43179 cycles/operation,  337 cycles/byte
> > [   45.346673] At least one hashing failed ret=-115
> > 
> > Don't think this is expected, is it?
> 
> No that's not right.  Does this patch fix it?
> 
yes, it fixes it. This is my output:

testing speed of multibuffer sha1 (sha1_mb)
[   31.342538] test  0 (   16 byte blocks,   16 bytes per update,   1
updates):
31614 cycles/operation,  246 cycles/byte
[   31.342549] test  2 (   64 byte blocks,   64 bytes per update,   1
updates):
10893 cycles/operation,   21 cycles/byte
[   31.342553] test  5 (  256 byte blocks,  256 bytes per update,   1
updates):
11043 cycles/operation,    5 cycles/byte
[   31.342557] test  8 ( 1024 byte blocks, 1024 bytes per update,   1
updates):
17355 cycles/operation,    2 cycles/byte
[   31.342562] test 12 ( 2048 byte blocks, 2048 bytes per update,   1
updates):
26328 cycles/operation,    1 cycles/byte
[   31.342570] test 16 ( 4096 byte blocks, 4096 bytes per update,   1
updates):
43833 cycles/operation,    1 cycles/byte
[   31.342584] test 21 ( 8192 byte blocks, 8192 bytes per update,   1
updates):
81141 cycles/operation,    1 cycles/byte
[   44.700088]
[   44.700088] testing speed of multibuffer sha256 (sha256_mb)
[   44.700091] test  0 (   16 byte blocks,   16 bytes per update,   1
updates):
116874 cycles/operation,  913 cycles/byte
[   44.700126] test  2 (   64 byte blocks,   64 bytes per update,   1
updates):
60894 cycles/operation,  118 cycles/byte
[   44.700144] test  5 (  256 byte blocks,  256 bytes per update,   1
updates):
15366 cycles/operation,    7 cycles/byte
[   44.700149] test  8 ( 1024 byte blocks, 1024 bytes per update,   1
updates):
29130 cycles/operation,    3 cycles/byte
[   44.700158] test 12 ( 2048 byte blocks, 2048 bytes per update,   1
updates):
48561 cycles/operation,    2 cycles/byte
[   44.700173] test 16 ( 4096 byte blocks, 4096 bytes per update,   1
updates):
119697 cycles/operation,    3 cycles/byte
[   44.700207] test 21 ( 8192 byte blocks, 8192 bytes per update,   1
updates):
164163 cycles/operation,    2 cycles/byte
[   66.309608]
[   66.309608] testing speed of multibuffer sha512 (sha512_mb)
[   66.309623] test  0 (   16 byte blocks,   16 bytes per update,   1
updates):
301050 cycles/operation, 2351 cycles/byte
[   66.309717] test  2 (   64 byte blocks,   64 bytes per update,   1
updates):
115416 cycles/operation,  225 cycles/byte
[   66.309757] test  5 (  256 byte blocks,  256 bytes per update,   1
updates):
175068 cycles/operation,   85 cycles/byte
[   66.309814] test  8 ( 1024 byte blocks, 1024 bytes per update,   1
updates):
354834 cycles/operation,   43 cycles/byte
[   66.309920] test 12 ( 2048 byte blocks, 2048 bytes per update,   1
updates):
599814 cycles/operation,   36 cycles/byte
[   66.310096] test 16 ( 4096 byte blocks, 4096 bytes per update,   1
updates):
585666 cycles/operation,   17 cycles/byte
[   66.310263] test 21 ( 8192 byte blocks, 8192 bytes per update,   1
updates):
1057770 cycles/operation,   16 cycles/byte

> Thanks!
> 
> ---8<---
> The multibuffer hash speed test is incorrectly bailing because
> of an EINPROGRESS return value.  This patch fixes it by setting
> ret to zero if it is equal to -EINPROGRESS.
> 
> Reported-by: Megha Dey <megha.dey@linux.intel.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
> index 8a91dc3..202cfa1 100644
> --- a/crypto/tcrypt.c
> +++ b/crypto/tcrypt.c
> @@ -655,8 +486,10 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
>  
>  		for (k = 0; k < 8; k++) {
>  			ret = crypto_ahash_digest(data[k].req);
> -			if (ret == -EINPROGRESS)
> +			if (ret == -EINPROGRESS) {
> +				ret = 0;
>  				continue;
> +			}
>  
>  			if (ret)
>  				break;

^ permalink raw reply

* [PATCH] crypto: qat - Switch to new rsa_helper functions
From: Salvatore Benedetto @ 2016-06-30 16:59 UTC (permalink / raw)
  To: herbert; +Cc: salvatore.benedetto, linux-crypto

Drop all asn1 related code in qat and use the new rsa_helper
functions rsa_parse_[pub|priv]_key for parsing the key

Signed-off-by: Salvatore Benedetto <salvatore.benedetto@intel.com>
---
 crypto/testmgr.c                                  |  1 +
 drivers/crypto/qat/Kconfig                        |  2 +-
 drivers/crypto/qat/qat_common/Makefile            | 10 -----
 drivers/crypto/qat/qat_common/qat_asym_algs.c     | 49 +++++++++--------------
 drivers/crypto/qat/qat_common/qat_rsaprivkey.asn1 | 11 -----
 drivers/crypto/qat/qat_common/qat_rsapubkey.asn1  |  4 --
 6 files changed, 22 insertions(+), 55 deletions(-)
 delete mode 100644 drivers/crypto/qat/qat_common/qat_rsaprivkey.asn1
 delete mode 100644 drivers/crypto/qat/qat_common/qat_rsapubkey.asn1

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 537fdc3..b52a81c 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1941,6 +1941,7 @@ static int do_test_rsa(struct crypto_akcipher *tfm,
 	if (err)
 		goto free_req;
 
+	err = -ENOMEM;
 	out_len_max = crypto_akcipher_maxsize(tfm);
 	outbuf_enc = kzalloc(out_len_max, GFP_KERNEL);
 	if (!outbuf_enc)
diff --git a/drivers/crypto/qat/Kconfig b/drivers/crypto/qat/Kconfig
index 85b44e5..571d04d 100644
--- a/drivers/crypto/qat/Kconfig
+++ b/drivers/crypto/qat/Kconfig
@@ -5,11 +5,11 @@ config CRYPTO_DEV_QAT
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_AKCIPHER
 	select CRYPTO_HMAC
+	select CRYPTO_RSA
 	select CRYPTO_SHA1
 	select CRYPTO_SHA256
 	select CRYPTO_SHA512
 	select FW_LOADER
-	select ASN1
 
 config CRYPTO_DEV_QAT_DH895xCC
 	tristate "Support for Intel(R) DH895xCC"
diff --git a/drivers/crypto/qat/qat_common/Makefile b/drivers/crypto/qat/qat_common/Makefile
index 6d74b91..92fb6ff 100644
--- a/drivers/crypto/qat/qat_common/Makefile
+++ b/drivers/crypto/qat/qat_common/Makefile
@@ -1,11 +1,3 @@
-$(obj)/qat_rsapubkey-asn1.o: $(obj)/qat_rsapubkey-asn1.c \
-			     $(obj)/qat_rsapubkey-asn1.h
-$(obj)/qat_rsaprivkey-asn1.o: $(obj)/qat_rsaprivkey-asn1.c \
-			      $(obj)/qat_rsaprivkey-asn1.h
-
-clean-files += qat_rsapubkey-asn1.c qat_rsapubkey-asn1.h
-clean-files += qat_rsaprivkey-asn1.c qat_rsaprivkey-asn1.h
-
 obj-$(CONFIG_CRYPTO_DEV_QAT) += intel_qat.o
 intel_qat-objs := adf_cfg.o \
 	adf_isr.o \
@@ -19,8 +11,6 @@ intel_qat-objs := adf_cfg.o \
 	adf_hw_arbiter.o \
 	qat_crypto.o \
 	qat_algs.o \
-	qat_rsapubkey-asn1.o \
-	qat_rsaprivkey-asn1.o \
 	qat_asym_algs.o \
 	qat_uclo.o \
 	qat_hal.o
diff --git a/drivers/crypto/qat/qat_common/qat_asym_algs.c b/drivers/crypto/qat/qat_common/qat_asym_algs.c
index 05f49d4..04b0ef8 100644
--- a/drivers/crypto/qat/qat_common/qat_asym_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_asym_algs.c
@@ -52,8 +52,6 @@
 #include <linux/dma-mapping.h>
 #include <linux/fips.h>
 #include <crypto/scatterwalk.h>
-#include "qat_rsapubkey-asn1.h"
-#include "qat_rsaprivkey-asn1.h"
 #include "icp_qat_fw_pke.h"
 #include "adf_accel_devices.h"
 #include "adf_transport.h"
@@ -502,10 +500,8 @@ unmap_src:
 	return ret;
 }
 
-int qat_rsa_get_n(void *context, size_t hdrlen, unsigned char tag,
-		  const void *value, size_t vlen)
+int qat_rsa_set_n(struct qat_rsa_ctx *ctx, const char *value, size_t vlen)
 {
-	struct qat_rsa_ctx *ctx = context;
 	struct qat_crypto_instance *inst = ctx->inst;
 	struct device *dev = &GET_DEV(inst->accel_dev);
 	const char *ptr = value;
@@ -518,11 +514,6 @@ int qat_rsa_get_n(void *context, size_t hdrlen, unsigned char tag,
 
 	ctx->key_sz = vlen;
 	ret = -EINVAL;
-	/* In FIPS mode only allow key size 2K & 3K */
-	if (fips_enabled && (ctx->key_sz != 256 && ctx->key_sz != 384)) {
-		pr_err("QAT: RSA: key size not allowed in FIPS mode\n");
-		goto err;
-	}
 	/* invalid key size provided */
 	if (!qat_rsa_enc_fn_id(ctx->key_sz))
 		goto err;
@@ -540,10 +531,8 @@ err:
 	return ret;
 }
 
-int qat_rsa_get_e(void *context, size_t hdrlen, unsigned char tag,
-		  const void *value, size_t vlen)
+int qat_rsa_set_e(struct qat_rsa_ctx *ctx, const char *value, size_t vlen)
 {
-	struct qat_rsa_ctx *ctx = context;
 	struct qat_crypto_instance *inst = ctx->inst;
 	struct device *dev = &GET_DEV(inst->accel_dev);
 	const char *ptr = value;
@@ -559,18 +548,15 @@ int qat_rsa_get_e(void *context, size_t hdrlen, unsigned char tag,
 	}
 
 	ctx->e = dma_zalloc_coherent(dev, ctx->key_sz, &ctx->dma_e, GFP_KERNEL);
-	if (!ctx->e) {
-		ctx->e = NULL;
+	if (!ctx->e)
 		return -ENOMEM;
-	}
+
 	memcpy(ctx->e + (ctx->key_sz - vlen), ptr, vlen);
 	return 0;
 }
 
-int qat_rsa_get_d(void *context, size_t hdrlen, unsigned char tag,
-		  const void *value, size_t vlen)
+int qat_rsa_set_d(struct qat_rsa_ctx *ctx, const char *value, size_t vlen)
 {
-	struct qat_rsa_ctx *ctx = context;
 	struct qat_crypto_instance *inst = ctx->inst;
 	struct device *dev = &GET_DEV(inst->accel_dev);
 	const char *ptr = value;
@@ -585,12 +571,6 @@ int qat_rsa_get_d(void *context, size_t hdrlen, unsigned char tag,
 	if (!ctx->key_sz || !vlen || vlen > ctx->key_sz)
 		goto err;
 
-	/* In FIPS mode only allow key size 2K & 3K */
-	if (fips_enabled && (vlen != 256 && vlen != 384)) {
-		pr_err("QAT: RSA: key size not allowed in FIPS mode\n");
-		goto err;
-	}
-
 	ret = -ENOMEM;
 	ctx->d = dma_zalloc_coherent(dev, ctx->key_sz, &ctx->dma_d, GFP_KERNEL);
 	if (!ctx->d)
@@ -608,6 +588,7 @@ static int qat_rsa_setkey(struct crypto_akcipher *tfm, const void *key,
 {
 	struct qat_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct device *dev = &GET_DEV(ctx->inst->accel_dev);
+	struct rsa_key rsa_key;
 	int ret;
 
 	/* Free the old key if any */
@@ -625,13 +606,23 @@ static int qat_rsa_setkey(struct crypto_akcipher *tfm, const void *key,
 	ctx->d = NULL;
 
 	if (private)
-		ret = asn1_ber_decoder(&qat_rsaprivkey_decoder, ctx, key,
-				       keylen);
+		ret = rsa_parse_priv_key(&rsa_key, key, keylen);
 	else
-		ret = asn1_ber_decoder(&qat_rsapubkey_decoder, ctx, key,
-				       keylen);
+		ret = rsa_parse_pub_key(&rsa_key, key, keylen);
+	if (ret < 0)
+		goto free;
+
+	ret = qat_rsa_set_n(ctx, rsa_key.n, rsa_key.n_sz);
 	if (ret < 0)
 		goto free;
+	ret = qat_rsa_set_e(ctx, rsa_key.e, rsa_key.e_sz);
+	if (ret < 0)
+		goto free;
+	if (private) {
+		ret = qat_rsa_set_d(ctx, rsa_key.d, rsa_key.d_sz);
+		if (ret < 0)
+			goto free;
+	}
 
 	if (!ctx->n || !ctx->e) {
 		/* invalid key provided */
diff --git a/drivers/crypto/qat/qat_common/qat_rsaprivkey.asn1 b/drivers/crypto/qat/qat_common/qat_rsaprivkey.asn1
deleted file mode 100644
index f0066ad..0000000
--- a/drivers/crypto/qat/qat_common/qat_rsaprivkey.asn1
+++ /dev/null
@@ -1,11 +0,0 @@
-RsaPrivKey ::= SEQUENCE {
-	version		INTEGER,
-	n		INTEGER ({ qat_rsa_get_n }),
-	e		INTEGER ({ qat_rsa_get_e }),
-	d		INTEGER ({ qat_rsa_get_d }),
-	prime1		INTEGER,
-	prime2		INTEGER,
-	exponent1	INTEGER,
-	exponent2	INTEGER,
-	coefficient	INTEGER
-}
diff --git a/drivers/crypto/qat/qat_common/qat_rsapubkey.asn1 b/drivers/crypto/qat/qat_common/qat_rsapubkey.asn1
deleted file mode 100644
index bd667b3..0000000
--- a/drivers/crypto/qat/qat_common/qat_rsapubkey.asn1
+++ /dev/null
@@ -1,4 +0,0 @@
-RsaPubKey ::= SEQUENCE {
-	n INTEGER ({ qat_rsa_get_n }),
-	e INTEGER ({ qat_rsa_get_e })
-}
-- 
2.7.4

^ permalink raw reply related

* kexec_file_load signature verification failure
From: Dave Young @ 2016-06-30 14:05 UTC (permalink / raw)
  To: David Howells; +Cc: linux-crypto, linux-kernel, kexec, pjones

Hi, David

kexec_file_load fails with a Bad Message error with recent kernel:

kexec_file_load failed: Bad message

Debug messages:

[ 3463.794391] PEFILE: checksum @ da
[ 3463.794395] PEFILE: header size = 200
[ 3463.794397] PEFILE: cert = 898 @4a5370 [97 08 00 00 00 02 02 00 30 82
08 8b 06 09 2a 86 48 86 f7 0d 01 07 02 a0 82 08 7c 30 82 08 78 02 01 01
31 0f 30 0d 06 09 60 86 48 01 65 03 04 02 01 05 00 30 5c 06 0a 2b 06 01
04 01 82 37 02 01]
[ 3463.794399] PEFILE: sig wrapper = { 897, 200, 2 }
[ 3463.794404] PKCS7: Got data
[ 3463.794407] X.509: PubKey Algo: 9
[ 3463.794411] X.509: Extension: 53
[ 3463.794412] X.509: Extension: 50
[ 3463.794413] X.509: Extension: 49
[ 3463.794414] X.509: subjkeyid 9d5c9a70fe6578e1ba171ba2ab9f3449d6688559
[ 3463.794415] X.509: Extension: 56
[ 3463.794417] X.509: x509_note_tbs_certificate(,4,04,8,780)!
[ 3463.794418] X.509: Signature type: 9 size 513
[ 3463.794419] X.509: AKID: keyid:
9d5c9a70fe6578e1ba171ba2ab9f3449d6688559
[ 3463.794420] X.509: authkeyid 9d5c9a70fe6578e1ba171ba2ab9f3449d6688559
[ 3463.795236] PKCS7: Got cert 1 for dyoung kernel test key
[ 3463.795238] PKCS7: - fingerprint
00d4a0934d161cab68311f301d06035504030c1664796f756e67206b65726e656c2074657374206b6579
[ 3463.795950] PKCS7: verify dyoung kernel test key: 00d4a0934d161cab68
[ 3463.795951] PKCS7: - issuer dyoung kernel test key
[ 3463.795952] PKCS7: - authkeyid.skid
9d5c9a70fe6578e1ba171ba2ab9f3449d6688559
[ 3463.795952] PKCS7: - self-signed
[ 3463.795954] Look up:
"ex:00d4a0934d161cab68311f301d06035504030c1664796f756e67206b65726e656c2074657374206b6579"
[ 3463.796642] PEFILE: Digest: 0 []
[ 3463.796644] PEFILE: Digest size mismatch (20 != 0)

Bisect results:

Bisecting: 0 revisions left to test after this (roughly 0 steps)
[e68503bd6836ba765dc8e0ee77ea675fedc07e41] KEYS: Generalise
system_verify_data() to provide access to internal content

I can not revert this comment only to test, seems there are a lot of
dependencies.

Could you please help on this issue?

Thanks
Dave

^ permalink raw reply

* Re: [patch] crypto: sha256-mb - cleanup a || vs | typo
From: Dan Carpenter @ 2016-06-30 12:33 UTC (permalink / raw)
  To: walter harms
  Cc: Joe Perches, H. Peter Anvin, Herbert Xu, David S. Miller,
	Thomas Gleixner, Ingo Molnar, x86, Tim Chen, Megha Dey,
	Wang, Rui Y, Denys Vlasenko, Xiaodong Liu, linux-crypto,
	linux-kernel, kernel-janitors
In-Reply-To: <57750665.7000703@bfs.de>

The difference between | and || is that || has ordering constraints.
It's from the C standard, and not the compiler version.

regards,
dan carpenter


^ permalink raw reply

* Re: [patch] crypto: sha256-mb - cleanup a || vs | typo
From: walter harms @ 2016-06-30 11:45 UTC (permalink / raw)
  To: Joe Perches
  Cc: Dan Carpenter, H. Peter Anvin, Herbert Xu, David S. Miller,
	Thomas Gleixner, Ingo Molnar, x86, Tim Chen, Megha Dey,
	Wang, Rui Y, Denys Vlasenko, Xiaodong Liu, linux-crypto,
	linux-kernel, kernel-janitors
In-Reply-To: <1467285386.24287.143.camel@perches.com>



Am 30.06.2016 13:16, schrieb Joe Perches:
> On Thu, 2016-06-30 at 10:50 +0300, Dan Carpenter wrote:
>> On Wed, Jun 29, 2016 at 10:05:53AM -0700, H. Peter Anvin wrote:
>>> On 06/29/16 07:42, Dan Carpenter wrote:
>>>>>> and | behave basically the same here but || is intended.  It causes a
>>>> static checker warning to mix up bitwise and logical operations.
>>>>
>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>>
>>>> diff --git a/arch/x86/crypto/sha256-mb/sha256_mb.c b/arch/x86/crypto/sha256-mb/sha256_mb.c
> []
>>>> @@ -299,7 +299,7 @@ static struct sha256_hash_ctx *sha256_ctx_mgr_submit(struct sha256_ctx_mgr *mgr,
>>>>  	 * Or if the user's buffer contains less than a whole block,
>>>>  	 * append as much as possible to the extra block.
>>>>  	 */
>>>> -	if ((ctx->partial_block_buffer_length) | (len < SHA256_BLOCK_SIZE)) {
>>>> +	if ((ctx->partial_block_buffer_length) || (len < SHA256_BLOCK_SIZE)) {
>>>>  		/* Compute how many bytes to copy from user buffer into
>>>>  		 * extra block
>>>>  		 */
>>>>
>>> As far as I know the | was an intentional optimization, so you may way
>>> to look at the generated code.
>> I know how the rules work.  I just thought it looked more like a typo
>> than an optimization.  It's normally a typo.  It's hard to tell the
>> intent.
> 
> The compiler could potentially emit the same code when
> optimizing but at least gcc 5.3 doesn't.
> 
> It's probably useful to add a comment for the specific intent
> here rather than change a potentially useful static checker.
> 

perhaps we can agree not to play tricks with a compiler.
Everything may be true for a certain version of CC but the next compiler is different.

just my 2 cents,
 wh

^ permalink raw reply

* Re: [patch] crypto: sha256-mb - cleanup a || vs | typo
From: Joe Perches @ 2016-06-30 11:16 UTC (permalink / raw)
  To: Dan Carpenter, H. Peter Anvin
  Cc: Herbert Xu, David S. Miller, Thomas Gleixner, Ingo Molnar, x86,
	Tim Chen, Megha Dey, Wang, Rui Y, Denys Vlasenko, Xiaodong Liu,
	linux-crypto, linux-kernel, kernel-janitors
In-Reply-To: <20160630075056.GR32247@mwanda>

On Thu, 2016-06-30 at 10:50 +0300, Dan Carpenter wrote:
> On Wed, Jun 29, 2016 at 10:05:53AM -0700, H. Peter Anvin wrote:
> > On 06/29/16 07:42, Dan Carpenter wrote:
> > > > > and | behave basically the same here but || is intended.  It causes a
> > > static checker warning to mix up bitwise and logical operations.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > 
> > > diff --git a/arch/x86/crypto/sha256-mb/sha256_mb.c b/arch/x86/crypto/sha256-mb/sha256_mb.c
[]
> > > @@ -299,7 +299,7 @@ static struct sha256_hash_ctx *sha256_ctx_mgr_submit(struct sha256_ctx_mgr *mgr,
> > >  	 * Or if the user's buffer contains less than a whole block,
> > >  	 * append as much as possible to the extra block.
> > >  	 */
> > > -	if ((ctx->partial_block_buffer_length) | (len < SHA256_BLOCK_SIZE)) {
> > > +	if ((ctx->partial_block_buffer_length) || (len < SHA256_BLOCK_SIZE)) {
> > >  		/* Compute how many bytes to copy from user buffer into
> > >  		 * extra block
> > >  		 */
> > > 
> > As far as I know the | was an intentional optimization, so you may way
> > to look at the generated code.
> I know how the rules work.  I just thought it looked more like a typo
> than an optimization.  It's normally a typo.  It's hard to tell the
> intent.

The compiler could potentially emit the same code when
optimizing but at least gcc 5.3 doesn't.

It's probably useful to add a comment for the specific intent
here rather than change a potentially useful static checker.

^ permalink raw reply

* [PATCH] crypto: qat - make qat_asym_algs.o depend on asn1 headers
From: Jan Stancek @ 2016-06-30 10:23 UTC (permalink / raw)
  To: tadeusz.struk, herbert; +Cc: qat-linux, linux-crypto, linux-kernel, jstancek

Parallel build can sporadically fail because asn1 headers may
not be built yet by the time qat_asym_algs.o is compiled:
  drivers/crypto/qat/qat_common/qat_asym_algs.c:55:32: fatal error: qat_rsapubkey-asn1.h: No such file or directory
   #include "qat_rsapubkey-asn1.h"

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Cc: Tadeusz Struk <tadeusz.struk@intel.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
 drivers/crypto/qat/qat_common/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/qat/qat_common/Makefile b/drivers/crypto/qat/qat_common/Makefile
index 6d74b91f2152..5fc3dbb9ada0 100644
--- a/drivers/crypto/qat/qat_common/Makefile
+++ b/drivers/crypto/qat/qat_common/Makefile
@@ -2,6 +2,7 @@ $(obj)/qat_rsapubkey-asn1.o: $(obj)/qat_rsapubkey-asn1.c \
 			     $(obj)/qat_rsapubkey-asn1.h
 $(obj)/qat_rsaprivkey-asn1.o: $(obj)/qat_rsaprivkey-asn1.c \
 			      $(obj)/qat_rsaprivkey-asn1.h
+$(obj)/qat_asym_algs.o: $(obj)/qat_rsapubkey-asn1.h $(obj)/qat_rsaprivkey-asn1.h
 
 clean-files += qat_rsapubkey-asn1.c qat_rsapubkey-asn1.h
 clean-files += qat_rsaprivkey-asn1.c qat_rsaprivkey-asn1.h
-- 
1.8.3.1

^ permalink raw reply related

* Re: [patch] crypto: sha256-mb - cleanup a || vs | typo
From: Dan Carpenter @ 2016-06-30  7:50 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Herbert Xu, David S. Miller, Thomas Gleixner, Ingo Molnar, x86,
	Tim Chen, Megha Dey, Wang, Rui Y, Denys Vlasenko, Xiaodong Liu,
	linux-crypto, linux-kernel, kernel-janitors
In-Reply-To: <8538242a-eab7-127e-e47e-26027fee4f6d@zytor.com>

On Wed, Jun 29, 2016 at 10:05:53AM -0700, H. Peter Anvin wrote:
> On 06/29/16 07:42, Dan Carpenter wrote:
> > || and | behave basically the same here but || is intended.  It causes a
> > static checker warning to mix up bitwise and logical operations.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/arch/x86/crypto/sha256-mb/sha256_mb.c b/arch/x86/crypto/sha256-mb/sha256_mb.c
> > index c9d5dcc..4ec895a 100644
> > --- a/arch/x86/crypto/sha256-mb/sha256_mb.c
> > +++ b/arch/x86/crypto/sha256-mb/sha256_mb.c
> > @@ -299,7 +299,7 @@ static struct sha256_hash_ctx *sha256_ctx_mgr_submit(struct sha256_ctx_mgr *mgr,
> >  	 * Or if the user's buffer contains less than a whole block,
> >  	 * append as much as possible to the extra block.
> >  	 */
> > -	if ((ctx->partial_block_buffer_length) | (len < SHA256_BLOCK_SIZE)) {
> > +	if ((ctx->partial_block_buffer_length) || (len < SHA256_BLOCK_SIZE)) {
> >  		/* Compute how many bytes to copy from user buffer into
> >  		 * extra block
> >  		 */
> > 
> 
> As far as I know the | was an intentional optimization, so you may way
> to look at the generated code.

I know how the rules work.  I just thought it looked more like a typo
than an optimization.  It's normally a typo.  It's hard to tell the
intent.

I think I'll modify my static checker to ignore these since the typo is
harmless.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH] crypto: omap-sham - increase cra_proirity to 400
From: Tero Kristo @ 2016-06-30  7:24 UTC (permalink / raw)
  To: Bin Liu, linux-crypto, linux-kernel; +Cc: Herbert Xu, David S. Miller
In-Reply-To: <1467216696-8014-1-git-send-email-b-liu@ti.com>

On 29/06/16 19:11, Bin Liu wrote:
> Some software alg has cra_priority as higher as 300, so increase
> omap-sham priority to 400 to ensure it is on top of any software alg.

You could mention the case where this is causing issues, namely the 
arm-neon-sha implementations which currently have priority of 150...300.

-Tero

>
> Signed-off-by: Bin Liu <b-liu@ti.com>
> ---
>   drivers/crypto/omap-sham.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
> index 63464e8..aa7be252 100644
> --- a/drivers/crypto/omap-sham.c
> +++ b/drivers/crypto/omap-sham.c
> @@ -1328,7 +1328,7 @@ static struct ahash_alg algs_sha1_md5[] = {
>   	.halg.base	= {
>   		.cra_name		= "sha1",
>   		.cra_driver_name	= "omap-sha1",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_KERN_DRIVER_ONLY |
>   						CRYPTO_ALG_ASYNC |
> @@ -1351,7 +1351,7 @@ static struct ahash_alg algs_sha1_md5[] = {
>   	.halg.base	= {
>   		.cra_name		= "md5",
>   		.cra_driver_name	= "omap-md5",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_KERN_DRIVER_ONLY |
>   						CRYPTO_ALG_ASYNC |
> @@ -1375,7 +1375,7 @@ static struct ahash_alg algs_sha1_md5[] = {
>   	.halg.base	= {
>   		.cra_name		= "hmac(sha1)",
>   		.cra_driver_name	= "omap-hmac-sha1",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_KERN_DRIVER_ONLY |
>   						CRYPTO_ALG_ASYNC |
> @@ -1400,7 +1400,7 @@ static struct ahash_alg algs_sha1_md5[] = {
>   	.halg.base	= {
>   		.cra_name		= "hmac(md5)",
>   		.cra_driver_name	= "omap-hmac-md5",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_KERN_DRIVER_ONLY |
>   						CRYPTO_ALG_ASYNC |
> @@ -1428,7 +1428,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
>   	.halg.base	= {
>   		.cra_name		= "sha224",
>   		.cra_driver_name	= "omap-sha224",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
> @@ -1450,7 +1450,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
>   	.halg.base	= {
>   		.cra_name		= "sha256",
>   		.cra_driver_name	= "omap-sha256",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
> @@ -1473,7 +1473,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
>   	.halg.base	= {
>   		.cra_name		= "hmac(sha224)",
>   		.cra_driver_name	= "omap-hmac-sha224",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
> @@ -1497,7 +1497,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
>   	.halg.base	= {
>   		.cra_name		= "hmac(sha256)",
>   		.cra_driver_name	= "omap-hmac-sha256",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
> @@ -1523,7 +1523,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
>   	.halg.base	= {
>   		.cra_name		= "sha384",
>   		.cra_driver_name	= "omap-sha384",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
> @@ -1545,7 +1545,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
>   	.halg.base	= {
>   		.cra_name		= "sha512",
>   		.cra_driver_name	= "omap-sha512",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
> @@ -1568,7 +1568,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
>   	.halg.base	= {
>   		.cra_name		= "hmac(sha384)",
>   		.cra_driver_name	= "omap-hmac-sha384",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
> @@ -1592,7 +1592,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
>   	.halg.base	= {
>   		.cra_name		= "hmac(sha512)",
>   		.cra_driver_name	= "omap-hmac-sha512",
> -		.cra_priority		= 100,
> +		.cra_priority		= 400,
>   		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
>   						CRYPTO_ALG_ASYNC |
>   						CRYPTO_ALG_NEED_FALLBACK,
>

^ permalink raw reply

* crypto: tcrypt - Do not bail on EINPROGRESS in multibuffer hash test
From: Herbert Xu @ 2016-06-30  3:00 UTC (permalink / raw)
  To: Megha Dey
  Cc: Krzysztof Kozlowski, David S. Miller, linux-crypto, linux-kernel,
	Bartlomiej Zolnierkiewicz, Fenghua Yu, Tim Chen
In-Reply-To: <1467222356.4247.4.camel@megha-Z97X-UD7-TH>

On Wed, Jun 29, 2016 at 10:45:56AM -0700, Megha Dey wrote:
> I tested the latest cryptodev tree on my haswell machine and this is
> what I see:
> [   40.402834] modprobe tcrypt mode=422
> [   40.403105] testing speed of multibuffer sha1 (sha1_mb)
> [   40.403108] test  0 (   16 byte blocks,   16 bytes per update,   1
> updates):  32271 cycles/operation,  252 cycles/byte
> [   40.403118] At least one hashing failed ret=-115
> [   43.218712] modprobe tcrypt mode=423
> [   43.218712] testing speed of multibuffer sha256 (sha256_mb)
> [   43.218715] test  0 (   16 byte blocks,   16 bytes per update,   1
> updates): 106965 cycles/operation,  835 cycles/byte
> [   43.218747] At least one hashing failed ret=-115
> [   45.346657] modprobe tcrypt mode=424
> [   45.346657] testing speed of multibuffer sha512 (sha512_mb)
> [   45.346660] test  0 (   16 byte blocks,   16 bytes per update,   1
> updates):  43179 cycles/operation,  337 cycles/byte
> [   45.346673] At least one hashing failed ret=-115
> 
> Don't think this is expected, is it?

No that's not right.  Does this patch fix it?

Thanks!

---8<---
The multibuffer hash speed test is incorrectly bailing because
of an EINPROGRESS return value.  This patch fixes it by setting
ret to zero if it is equal to -EINPROGRESS.

Reported-by: Megha Dey <megha.dey@linux.intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 8a91dc3..202cfa1 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -655,8 +486,10 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 
 		for (k = 0; k < 8; k++) {
 			ret = crypto_ahash_digest(data[k].req);
-			if (ret == -EINPROGRESS)
+			if (ret == -EINPROGRESS) {
+				ret = 0;
 				continue;
+			}
 
 			if (ret)
 				break;
-- 
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 related

* Re: [PATCH v8 6/6] crypto: AF_ALG - add support for key_id
From: Mat Martineau @ 2016-06-29 18:43 UTC (permalink / raw)
  To: Tadeusz Struk
  Cc: dhowells, herbert, smueller, linux-api, marcel,
	mathew.j.martineau, linux-kernel, keyrings, linux-crypto, dwmw2,
	davem
In-Reply-To: <146672255872.23101.10938182451423661314.stgit@tstruk-mobl1.ra.intel.com>


Tadeusz,

On Thu, 23 Jun 2016, Tadeusz Struk wrote:

> This patch adds support for asymmetric key type to AF_ALG.
> It will work as follows: A new PF_ALG socket options are
> added on top of existing ALG_SET_KEY and ALG_SET_PUBKEY, namely
> ALG_SET_KEY_ID and ALG_SET_PUBKEY_ID for setting public and
> private keys respectively. When these new options will be used
> the user, instead of providing the key material, will provide a
> key id and the key itself will be obtained from kernel keyring
> subsystem. The user will use the standard tools (keyctl tool
> or the keyctl syscall) for key instantiation and to obtain the
> key id. The key id can also be obtained by reading the
> /proc/keys file.
>
> When a key corresponding to the given keyid is found, it is stored
> in the socket context and subsequent crypto operation invoked by the
> user will use the new asymmetric accessor functions instead of akcipher
> api. The asymmetric subtype can internally use akcipher api or
> invoke operations defined by a given subtype, depending on the
> key type.
>
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
> ---
> crypto/af_alg.c             |   10 ++
> crypto/algif_akcipher.c     |  212 ++++++++++++++++++++++++++++++++++++++++++-
> include/crypto/if_alg.h     |    1
> include/uapi/linux/if_alg.h |    2
> 4 files changed, 220 insertions(+), 5 deletions(-)
>
> diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.c
> index 2b8d37e..106f715 100644
> --- a/crypto/algif_akcipher.c
> +++ b/crypto/algif_akcipher.c
> +static int asym_key_verify(const struct key *key, struct akcipher_request *req)
> +{
> +	struct public_key_signature sig;
> +	char *src = NULL, *in, digest[20];
> +	int ret;
> +
> +	if (!sg_is_last(req->src)) {
> +		src = kmalloc(req->src_len, GFP_KERNEL);
> +		if (!src)
> +			return -ENOMEM;
> +		scatterwalk_map_and_copy(src, req->src, 0, req->src_len, 0);
> +		in = src;
> +	} else {
> +		in = sg_virt(req->src);
> +	}
> +	sig.pkey_algo = "rsa";
> +	sig.encoding = "pkcs1";
> +	/* Need to find a way to pass the hash param */

Comment still needed?

> +	sig.hash_algo = "sha1";
> +	sig.digest_size = sizeof(digest);
> +	sig.digest = digest;
> +	sig.s_size = req->src_len;
> +	sig.s = src;
> +	ret = verify_signature(key, &sig);
> +	if (!ret) {
> +		req->dst_len = sizeof(digest);

I think you fixed the BUG_ON() problem but there's still an issue with the 
handling of the digest. Check the use of sig->digest in 
public_key_verify_signature(), it's an input not an output. Right now it 
looks like 20 uninitialized bytes are compared with the computed digest 
within verify_signature, and then the unintialized bytes are copied to 
req->dst here.

With some modifications to public_key_verify_signature you could get the 
digest you need, but I'm not sure if verification with a hardware key 
(like a key in a TPM) can or can not provide the digest needed. Maybe this 
is why the verify_signature hook in struct asymmetric_key_subtype is 
optional.

> +		scatterwalk_map_and_copy(digest, req->dst, 0, req->dst_len, 1);
> +	}
> +	kfree(src);
> +	return ret;
> +}
> +

--
Mat Martineau
Intel OTC

^ permalink raw reply

* Re: [PATCH v2] crypto: tcrypt - Fix memory leaks/crashes in multibuffer hash speed test
From: Megha Dey @ 2016-06-29 17:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Herbert Xu
  Cc: David S. Miller, linux-crypto, linux-kernel,
	Bartlomiej Zolnierkiewicz, Fenghua Yu, Tim Chen
In-Reply-To: <57738697.1050401@samsung.com>

I tested the latest cryptodev tree on my haswell machine and this is
what I see:
[   40.402834] modprobe tcrypt mode=422
[   40.403105] testing speed of multibuffer sha1 (sha1_mb)
[   40.403108] test  0 (   16 byte blocks,   16 bytes per update,   1
updates):  32271 cycles/operation,  252 cycles/byte
[   40.403118] At least one hashing failed ret=-115
[   43.218712] modprobe tcrypt mode=423
[   43.218712] testing speed of multibuffer sha256 (sha256_mb)
[   43.218715] test  0 (   16 byte blocks,   16 bytes per update,   1
updates): 106965 cycles/operation,  835 cycles/byte
[   43.218747] At least one hashing failed ret=-115
[   45.346657] modprobe tcrypt mode=424
[   45.346657] testing speed of multibuffer sha512 (sha512_mb)
[   45.346660] test  0 (   16 byte blocks,   16 bytes per update,   1
updates):  43179 cycles/operation,  337 cycles/byte
[   45.346673] At least one hashing failed ret=-115

Don't think this is expected, is it?

This is the patch which might have an issue?
72259deb3a9f2c07d18d71d7c9356754e7d88369

Thanks,
Megha

On Wed, 2016-06-29 at 10:28 +0200, Krzysztof Kozlowski wrote:
> On 06/29/2016 10:19 AM, Herbert Xu wrote:
> > On Wed, Jun 29, 2016 at 10:16:10AM +0200, Krzysztof Kozlowski wrote:
> >>
> >> Seems to work fine except:
> >> 1. The updates are always 1.
> > 
> > Yes the test function only does digest so it's always one update.
> > 
> >> 2. For bigger blocks it reports always 1 or 3 cycles per byte:
> > 
> > Yes the average cycles per-byte should reach an asymptotic value.
> 
> Then:
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply

* Re: [patch] crypto: sha256-mb - cleanup a || vs | typo
From: H. Peter Anvin @ 2016-06-29 17:05 UTC (permalink / raw)
  To: Dan Carpenter, Herbert Xu
  Cc: David S. Miller, Thomas Gleixner, Ingo Molnar, x86, Tim Chen,
	Megha Dey, Wang, Rui Y, Denys Vlasenko, Xiaodong Liu,
	linux-crypto, linux-kernel, kernel-janitors
In-Reply-To: <20160629144242.GE22818@mwanda>

On 06/29/16 07:42, Dan Carpenter wrote:
> || and | behave basically the same here but || is intended.  It causes a
> static checker warning to mix up bitwise and logical operations.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/arch/x86/crypto/sha256-mb/sha256_mb.c b/arch/x86/crypto/sha256-mb/sha256_mb.c
> index c9d5dcc..4ec895a 100644
> --- a/arch/x86/crypto/sha256-mb/sha256_mb.c
> +++ b/arch/x86/crypto/sha256-mb/sha256_mb.c
> @@ -299,7 +299,7 @@ static struct sha256_hash_ctx *sha256_ctx_mgr_submit(struct sha256_ctx_mgr *mgr,
>  	 * Or if the user's buffer contains less than a whole block,
>  	 * append as much as possible to the extra block.
>  	 */
> -	if ((ctx->partial_block_buffer_length) | (len < SHA256_BLOCK_SIZE)) {
> +	if ((ctx->partial_block_buffer_length) || (len < SHA256_BLOCK_SIZE)) {
>  		/* Compute how many bytes to copy from user buffer into
>  		 * extra block
>  		 */
> 

As far as I know the | was an intentional optimization, so you may way
to look at the generated code.

	-hpa


^ permalink raw reply

* [PATCH] crypto: omap-sham - increase cra_proirity to 400
From: Bin Liu @ 2016-06-29 16:11 UTC (permalink / raw)
  To: linux-crypto, linux-kernel; +Cc: Herbert Xu, David S. Miller, Tero Kristo

Some software alg has cra_priority as higher as 300, so increase
omap-sham priority to 400 to ensure it is on top of any software alg.

Signed-off-by: Bin Liu <b-liu@ti.com>
---
 drivers/crypto/omap-sham.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 63464e8..aa7be252 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -1328,7 +1328,7 @@ static struct ahash_alg algs_sha1_md5[] = {
 	.halg.base	= {
 		.cra_name		= "sha1",
 		.cra_driver_name	= "omap-sha1",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_KERN_DRIVER_ONLY |
 						CRYPTO_ALG_ASYNC |
@@ -1351,7 +1351,7 @@ static struct ahash_alg algs_sha1_md5[] = {
 	.halg.base	= {
 		.cra_name		= "md5",
 		.cra_driver_name	= "omap-md5",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_KERN_DRIVER_ONLY |
 						CRYPTO_ALG_ASYNC |
@@ -1375,7 +1375,7 @@ static struct ahash_alg algs_sha1_md5[] = {
 	.halg.base	= {
 		.cra_name		= "hmac(sha1)",
 		.cra_driver_name	= "omap-hmac-sha1",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_KERN_DRIVER_ONLY |
 						CRYPTO_ALG_ASYNC |
@@ -1400,7 +1400,7 @@ static struct ahash_alg algs_sha1_md5[] = {
 	.halg.base	= {
 		.cra_name		= "hmac(md5)",
 		.cra_driver_name	= "omap-hmac-md5",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_KERN_DRIVER_ONLY |
 						CRYPTO_ALG_ASYNC |
@@ -1428,7 +1428,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
 	.halg.base	= {
 		.cra_name		= "sha224",
 		.cra_driver_name	= "omap-sha224",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
@@ -1450,7 +1450,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
 	.halg.base	= {
 		.cra_name		= "sha256",
 		.cra_driver_name	= "omap-sha256",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
@@ -1473,7 +1473,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
 	.halg.base	= {
 		.cra_name		= "hmac(sha224)",
 		.cra_driver_name	= "omap-hmac-sha224",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
@@ -1497,7 +1497,7 @@ static struct ahash_alg algs_sha224_sha256[] = {
 	.halg.base	= {
 		.cra_name		= "hmac(sha256)",
 		.cra_driver_name	= "omap-hmac-sha256",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
@@ -1523,7 +1523,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
 	.halg.base	= {
 		.cra_name		= "sha384",
 		.cra_driver_name	= "omap-sha384",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
@@ -1545,7 +1545,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
 	.halg.base	= {
 		.cra_name		= "sha512",
 		.cra_driver_name	= "omap-sha512",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
@@ -1568,7 +1568,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
 	.halg.base	= {
 		.cra_name		= "hmac(sha384)",
 		.cra_driver_name	= "omap-hmac-sha384",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
@@ -1592,7 +1592,7 @@ static struct ahash_alg algs_sha384_sha512[] = {
 	.halg.base	= {
 		.cra_name		= "hmac(sha512)",
 		.cra_driver_name	= "omap-hmac-sha512",
-		.cra_priority		= 100,
+		.cra_priority		= 400,
 		.cra_flags		= CRYPTO_ALG_TYPE_AHASH |
 						CRYPTO_ALG_ASYNC |
 						CRYPTO_ALG_NEED_FALLBACK,
-- 
1.9.1

^ permalink raw reply related

* [patch V4 30/31] crypto: use parity_long in sahara.c
From: zengzhaoxiu @ 2016-06-29 15:18 UTC (permalink / raw)
  To: linux-kernel; +Cc: herbert, davem, linux-crypto, Zhaoxiu Zeng
In-Reply-To: <1462958683-26142-1-git-send-email-zengzhaoxiu@163.com>

From: Zhaoxiu Zeng <zhaoxiu.zeng@gmail.com>

Signed-off-by: Zhaoxiu Zeng <zhaoxiu.zeng@gmail.com>
---
 drivers/crypto/sahara.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/sahara.c b/drivers/crypto/sahara.c
index c3f3d89..5c44a15 100644
--- a/drivers/crypto/sahara.c
+++ b/drivers/crypto/sahara.c
@@ -783,7 +783,7 @@ static u32 sahara_sha_init_hdr(struct sahara_dev *dev,
 	if (rctx->last)
 		hdr |= SAHARA_HDR_MDHA_PDATA;
 
-	if (hweight_long(hdr) % 2 == 0)
+	if (!parity_long(hdr))
 		hdr |= SAHARA_HDR_PARITY_BIT;
 
 	return hdr;
-- 
2.7.4

^ permalink raw reply related

* Re: [patch] crypto: tcrypt - add a missing tab
From: Herbert Xu @ 2016-06-29 14:43 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-crypto, kernel-janitors
In-Reply-To: <20160629144130.GC22818@mwanda>

On Wed, Jun 29, 2016 at 05:41:30PM +0300, Dan Carpenter wrote:
> The "goto out;" line isn't indented far enough.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Sorry, but this has already been fixed :)
-- 
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

* [patch] crypto: sha1-mb - cleanup a small | vs || typo
From: Dan Carpenter @ 2016-06-29 14:42 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David S. Miller, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Megha Dey, Tim Chen, Xiaodong Liu, Denys Vlasenko,
	Wang, Rui Y, linux-crypto, linux-kernel, kernel-janitors

|| and | behave basically the same here but || was intended.  It causes
a static checker warning when we mix up logical and bitwise operations.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/crypto/sha1-mb/sha1_mb.c b/arch/x86/crypto/sha1-mb/sha1_mb.c
index 561b286..96d80ad 100644
--- a/arch/x86/crypto/sha1-mb/sha1_mb.c
+++ b/arch/x86/crypto/sha1-mb/sha1_mb.c
@@ -304,7 +304,7 @@ static struct sha1_hash_ctx *sha1_ctx_mgr_submit(struct sha1_ctx_mgr *mgr,
 	 * Or if the user's buffer contains less than a whole block,
 	 * append as much as possible to the extra block.
 	 */
-	if ((ctx->partial_block_buffer_length) | (len < SHA1_BLOCK_SIZE)) {
+	if ((ctx->partial_block_buffer_length) || (len < SHA1_BLOCK_SIZE)) {
 		/*
 		 * Compute how many bytes to copy from user buffer into
 		 * extra block

^ permalink raw reply related

* [patch] crypto: sha256-mb - cleanup a || vs | typo
From: Dan Carpenter @ 2016-06-29 14:42 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David S. Miller, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, Tim Chen, Megha Dey, Wang, Rui Y, Denys Vlasenko,
	Xiaodong Liu, linux-crypto, linux-kernel, kernel-janitors

|| and | behave basically the same here but || is intended.  It causes a
static checker warning to mix up bitwise and logical operations.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/crypto/sha256-mb/sha256_mb.c b/arch/x86/crypto/sha256-mb/sha256_mb.c
index c9d5dcc..4ec895a 100644
--- a/arch/x86/crypto/sha256-mb/sha256_mb.c
+++ b/arch/x86/crypto/sha256-mb/sha256_mb.c
@@ -299,7 +299,7 @@ static struct sha256_hash_ctx *sha256_ctx_mgr_submit(struct sha256_ctx_mgr *mgr,
 	 * Or if the user's buffer contains less than a whole block,
 	 * append as much as possible to the extra block.
 	 */
-	if ((ctx->partial_block_buffer_length) | (len < SHA256_BLOCK_SIZE)) {
+	if ((ctx->partial_block_buffer_length) || (len < SHA256_BLOCK_SIZE)) {
 		/* Compute how many bytes to copy from user buffer into
 		 * extra block
 		 */

^ permalink raw reply related

* [patch] crypto: tcrypt - add a missing tab
From: Dan Carpenter @ 2016-06-29 14:41 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, kernel-janitors

The "goto out;" line isn't indented far enough.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 6ef7815..117f19e 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -629,7 +629,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 			printk(KERN_ERR
 				"template (%u) too big for tvmem (%lu)\n",
 					speed[i].blen, TVMEMSIZE * PAGE_SIZE);
-		goto out;
+			goto out;
 		}
 
 		if (speed[i].klen)

^ permalink raw reply related

* Re: Doing crypto in small stack buffers (bluetooth vs vmalloc-stack crash, etc)
From: George Spelvin @ 2016-06-29 12:10 UTC (permalink / raw)
  To: herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q,
	linux-+7tBnqSOmQ59SlIZoIAWRdHuzzzSOjJt
  Cc: linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, luto-kltTT9wpgjJwATOyAt5JVQ,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160629022049.GA23390-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>

>> Also not mentioned in the documentation is that some algorithms *do*
>> have different implementations depending on key size.  SHA-2 is the
>> classic example.

> What do you mean by that? SHA has no keying at all.

In this case, the analagous property is hash size.  Sorry, I thought
that was so obvious I didn't need to say it.

Specifically, SHA2-256 (and -224) and SHA2-512 (and -384) are separate
algorithms with similar structures but deparate implementations.

^ permalink raw reply

* [v4 PATCH 8/8] crypto: rsa-pkcs1pad - Avoid copying output when possible
From: Herbert Xu @ 2016-06-29 11:32 UTC (permalink / raw)
  To: Andrzej Zaborowski, Tadeusz Struk, Linux Crypto Mailing List,
	Tudor Ambarus, Stephan Mueller, Mat Martineau, Denis Kenzior,
	Salvatore Benedetto
In-Reply-To: <20160629113125.GA27643@gondor.apana.org.au>

In the vast majority of cases (2^-32 on 32-bit and 2^-64 on 64-bit)
cases, the result from encryption/signing will require no padding.

This patch makes these two operations write their output directly
to the final destination.  Only in the exceedingly rare cases where
fixup is needed to we copy it out and back to add the leading zeroes.

This patch also makes use of the crypto_akcipher_set_crypt API
instead of writing the akcipher request directly.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |  112 ++++++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 67 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ebd8514..8ccfdd7 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -185,37 +185,36 @@ static int pkcs1pad_encrypt_sign_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	size_t pad_len = ctx->key_size - req_ctx->child_req.dst_len;
-	size_t chunk_len, pad_left;
-	struct sg_mapping_iter miter;
-
-	if (!err) {
-		if (pad_len) {
-			sg_miter_start(&miter, req->dst,
-					sg_nents_for_len(req->dst, pad_len),
-					SG_MITER_ATOMIC | SG_MITER_TO_SG);
-
-			pad_left = pad_len;
-			while (pad_left) {
-				sg_miter_next(&miter);
-
-				chunk_len = min(miter.length, pad_left);
-				memset(miter.addr, 0, chunk_len);
-				pad_left -= chunk_len;
-			}
-
-			sg_miter_stop(&miter);
-		}
-
-		sg_pcopy_from_buffer(req->dst,
-				sg_nents_for_len(req->dst, ctx->key_size),
-				req_ctx->out_buf, req_ctx->child_req.dst_len,
-				pad_len);
-	}
+	unsigned int pad_len;
+	unsigned int len;
+	u8 *out_buf;
+
+	if (err)
+		goto out;
+
+	len = req_ctx->child_req.dst_len;
+	pad_len = ctx->key_size - len;
+
+	/* Four billion to one */
+	if (likely(!pad_len))
+		goto out;
+
+	out_buf = kzalloc(ctx->key_size, GFP_ATOMIC);
+	err = -ENOMEM;
+	if (!out_buf)
+		goto out;
+
+	sg_copy_to_buffer(req->dst, sg_nents_for_len(req->dst, len),
+			  out_buf + pad_len, len);
+	sg_copy_from_buffer(req->dst,
+			    sg_nents_for_len(req->dst, ctx->key_size),
+			    out_buf, ctx->key_size);
+	kzfree(out_buf);
+
+out:
 	req->dst_len = ctx->key_size;
 
 	kfree(req_ctx->in_buf);
-	kzfree(req_ctx->out_buf);
 
 	return err;
 }
@@ -255,15 +254,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -291,6 +281,10 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_encrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -372,12 +366,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size ;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -389,6 +377,11 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_decrypt_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_decrypt(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -422,15 +415,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	/*
-	 * Replace both input and output to add the padding in the input and
-	 * the potential missing leading zeros in the output.
-	 */
-	req_ctx->child_req.src = req_ctx->in_sg;
-	req_ctx->child_req.src_len = ctx->key_size - 1;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
 				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
@@ -447,19 +431,14 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
-	if (!req_ctx->out_buf) {
-		kfree(req_ctx->in_buf);
-		return -ENOMEM;
-	}
-
-	pkcs1pad_sg_set_buf(req_ctx->out_sg, req_ctx->out_buf,
-			ctx->key_size, NULL);
-
 	akcipher_request_set_tfm(&req_ctx->child_req, ctx->child);
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_encrypt_sign_complete_cb, req);
 
+	/* Reuse output buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req_ctx->in_sg,
+				   req->dst, ctx->key_size - 1, req->dst_len);
+
 	err = crypto_akcipher_sign(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||
@@ -559,12 +538,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	/* Reuse input buffer, output to a new buffer */
-	req_ctx->child_req.src = req->src;
-	req_ctx->child_req.src_len = req->src_len;
-	req_ctx->child_req.dst = req_ctx->out_sg;
-	req_ctx->child_req.dst_len = ctx->key_size;
-
 	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
@@ -576,6 +549,11 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	akcipher_request_set_callback(&req_ctx->child_req, req->base.flags,
 			pkcs1pad_verify_complete_cb, req);
 
+	/* Reuse input buffer, output to a new buffer */
+	akcipher_request_set_crypt(&req_ctx->child_req, req->src,
+				   req_ctx->out_sg, req->src_len,
+				   ctx->key_size);
+
 	err = crypto_akcipher_verify(&req_ctx->child_req);
 	if (err != -EINPROGRESS &&
 			(err != -EBUSY ||

^ permalink raw reply related

* [v4 PATCH 7/8] crypto: rsa-pkcs1pad - Move key size check to setkey
From: Herbert Xu @ 2016-06-29 11:32 UTC (permalink / raw)
  To: Andrzej Zaborowski, Tadeusz Struk, Linux Crypto Mailing List,
	Tudor Ambarus, Stephan Mueller, Mat Martineau, Denis Kenzior,
	Salvatore Benedetto
In-Reply-To: <20160629113125.GA27643@gondor.apana.org.au>

Rather than repeatedly checking the key size on each operation,
we should be checking it once when the key is set.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   56 +++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index db19284..ebd8514 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -111,40 +111,48 @@ static int pkcs1pad_set_pub_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_pub_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_set_priv_key(struct crypto_akcipher *tfm, const void *key,
 		unsigned int keylen)
 {
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
-	int err, size;
+	int err;
+
+	ctx->key_size = 0;
 
 	err = crypto_akcipher_set_priv_key(ctx->child, key, keylen);
+	if (err)
+		return err;
 
-	if (!err) {
-		/* Find out new modulus size from rsa implementation */
-		size = crypto_akcipher_maxsize(ctx->child);
+	/* Find out new modulus size from rsa implementation */
+	err = crypto_akcipher_maxsize(ctx->child);
+	if (err < 0)
+		return err;
 
-		ctx->key_size = size > 0 ? size : 0;
-		if (size <= 0)
-			err = size;
-	}
+	if (err > PAGE_SIZE)
+		return -ENOTSUPP;
 
-	return err;
+	ctx->key_size = err;
+	return 0;
 }
 
 static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
@@ -247,9 +255,6 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -367,9 +372,6 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len != ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;
@@ -420,9 +422,6 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 		return -EOVERFLOW;
 	}
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/*
 	 * Replace both input and output to add the padding in the input and
 	 * the potential missing leading zeros in the output.
@@ -560,9 +559,6 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	if (!ctx->key_size || req->src_len < ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->key_size > PAGE_SIZE)
-		return -ENOTSUPP;
-
 	/* Reuse input buffer, output to a new buffer */
 	req_ctx->child_req.src = req->src;
 	req_ctx->child_req.src_len = req->src_len;

^ permalink raw reply related

* [v4 PATCH 6/8] crypto: rsa-pkcs1pad - Always use GFP_KERNEL
From: Herbert Xu @ 2016-06-29 11:32 UTC (permalink / raw)
  To: Andrzej Zaborowski, Tadeusz Struk, Linux Crypto Mailing List,
	Tudor Ambarus, Stephan Mueller, Mat Martineau, Denis Kenzior,
	Salvatore Benedetto
In-Reply-To: <20160629113125.GA27643@gondor.apana.org.au>

We don't currently support using akcipher in atomic contexts,
so GFP_KERNEL should always be used.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index d9baefb..db19284 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -260,8 +260,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -274,9 +273,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -379,9 +376,7 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size ;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 
@@ -438,8 +433,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	req_ctx->child_req.dst_len = ctx->key_size;
 
 	req_ctx->in_buf = kmalloc(ctx->key_size - 1 - req->src_len,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+				  GFP_KERNEL);
 	if (!req_ctx->in_buf)
 		return -ENOMEM;
 
@@ -454,9 +448,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf) {
 		kfree(req_ctx->in_buf);
 		return -ENOMEM;
@@ -577,9 +569,7 @@ static int pkcs1pad_verify(struct akcipher_request *req)
 	req_ctx->child_req.dst = req_ctx->out_sg;
 	req_ctx->child_req.dst_len = ctx->key_size;
 
-	req_ctx->out_buf = kmalloc(ctx->key_size,
-			(req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
-			GFP_KERNEL : GFP_ATOMIC);
+	req_ctx->out_buf = kmalloc(ctx->key_size, GFP_KERNEL);
 	if (!req_ctx->out_buf)
 		return -ENOMEM;
 

^ permalink raw reply related

* [v4 PATCH 5/8] crypto: rsa-pkcs1pad - Remove bogus page splitting
From: Herbert Xu @ 2016-06-29 11:32 UTC (permalink / raw)
  To: Andrzej Zaborowski, Tadeusz Struk, Linux Crypto Mailing List,
	Tudor Ambarus, Stephan Mueller, Mat Martineau, Denis Kenzior,
	Salvatore Benedetto
In-Reply-To: <20160629113125.GA27643@gondor.apana.org.au>

The helper pkcs1pad_sg_set_buf tries to split a buffer that crosses
a page boundary into two SG entries.  This is unnecessary.  This
patch removes that.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 5c1c78e..d9baefb 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -103,7 +103,7 @@ struct pkcs1pad_inst_ctx {
 struct pkcs1pad_request {
 	struct akcipher_request child_req;
 
-	struct scatterlist in_sg[3], out_sg[2];
+	struct scatterlist in_sg[2], out_sg[1];
 	uint8_t *in_buf, *out_buf;
 };
 
@@ -163,19 +163,10 @@ static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
 static void pkcs1pad_sg_set_buf(struct scatterlist *sg, void *buf, size_t len,
 		struct scatterlist *next)
 {
-	int nsegs = next ? 1 : 0;
-
-	if (offset_in_page(buf) + len <= PAGE_SIZE) {
-		nsegs += 1;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg, buf, len);
-	} else {
-		nsegs += 2;
-		sg_init_table(sg, nsegs);
-		sg_set_buf(sg + 0, buf, PAGE_SIZE - offset_in_page(buf));
-		sg_set_buf(sg + 1, buf + PAGE_SIZE - offset_in_page(buf),
-				offset_in_page(buf) + len - PAGE_SIZE);
-	}
+	int nsegs = next ? 2 : 1;
+
+	sg_init_table(sg, nsegs);
+	sg_set_buf(sg, buf, len);
 
 	if (next)
 		sg_chain(sg, nsegs, next);

^ permalink raw reply related

* [v4 PATCH 4/8] crypto: rsa-pkcs1pad - Require hash to be present
From: Herbert Xu @ 2016-06-29 11:32 UTC (permalink / raw)
  To: Andrzej Zaborowski, Tadeusz Struk, Linux Crypto Mailing List,
	Tudor Ambarus, Stephan Mueller, Mat Martineau, Denis Kenzior,
	Salvatore Benedetto
In-Reply-To: <20160629113125.GA27643@gondor.apana.org.au>

The only user of rsa-pkcs1pad always uses the hash so there is
no reason to support the case of not having a hash.

This patch also changes the digest info lookup so that it is
only done once during template instantiation rather than on each
operation.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 crypto/rsa-pkcs1pad.c |   83 ++++++++++++++++++--------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index ead8dc0..5c1c78e 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -92,13 +92,12 @@ static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
 
 struct pkcs1pad_ctx {
 	struct crypto_akcipher *child;
-	const char *hash_name;
 	unsigned int key_size;
 };
 
 struct pkcs1pad_inst_ctx {
 	struct crypto_akcipher_spawn spawn;
-	const char *hash_name;
+	const struct rsa_asn1_template *digest_info;
 };
 
 struct pkcs1pad_request {
@@ -416,20 +415,16 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info = NULL;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	int err;
 	unsigned int ps_end, digest_size = 0;
 
 	if (!ctx->key_size)
 		return -EINVAL;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			return -EINVAL;
-
-		digest_size = digest_info->size;
-	}
+	digest_size = digest_info->size;
 
 	if (req->src_len + digest_size > ctx->key_size - 11)
 		return -EOVERFLOW;
@@ -462,10 +457,8 @@ static int pkcs1pad_sign(struct akcipher_request *req)
 	memset(req_ctx->in_buf + 1, 0xff, ps_end - 1);
 	req_ctx->in_buf[ps_end] = 0x00;
 
-	if (digest_info) {
-		memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
-		       digest_info->size);
-	}
+	memcpy(req_ctx->in_buf + ps_end + 1, digest_info->data,
+	       digest_info->size);
 
 	pkcs1pad_sg_set_buf(req_ctx->in_sg, req_ctx->in_buf,
 			ctx->key_size - 1 - req->src_len, req->src);
@@ -499,7 +492,9 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct pkcs1pad_request *req_ctx = akcipher_request_ctx(req);
-	const struct rsa_asn1_template *digest_info;
+	struct akcipher_instance *inst = akcipher_alg_instance(tfm);
+	struct pkcs1pad_inst_ctx *ictx = akcipher_instance_ctx(inst);
+	const struct rsa_asn1_template *digest_info = ictx->digest_info;
 	unsigned int pos;
 
 	if (err == -EOVERFLOW)
@@ -527,17 +522,11 @@ static int pkcs1pad_verify_complete(struct akcipher_request *req, int err)
 		goto done;
 	pos++;
 
-	if (ctx->hash_name) {
-		digest_info = rsa_lookup_asn1(ctx->hash_name);
-		if (!digest_info)
-			goto done;
+	if (memcmp(req_ctx->out_buf + pos, digest_info->data,
+		   digest_info->size))
+		goto done;
 
-		if (memcmp(req_ctx->out_buf + pos, digest_info->data,
-			   digest_info->size))
-			goto done;
-
-		pos += digest_info->size;
-	}
+	pos += digest_info->size;
 
 	err = 0;
 
@@ -626,12 +615,11 @@ static int pkcs1pad_init_tfm(struct crypto_akcipher *tfm)
 	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct crypto_akcipher *child_tfm;
 
-	child_tfm = crypto_spawn_akcipher(akcipher_instance_ctx(inst));
+	child_tfm = crypto_spawn_akcipher(&ictx->spawn);
 	if (IS_ERR(child_tfm))
 		return PTR_ERR(child_tfm);
 
 	ctx->child = child_tfm;
-	ctx->hash_name = ictx->hash_name;
 	return 0;
 }
 
@@ -648,12 +636,12 @@ static void pkcs1pad_free(struct akcipher_instance *inst)
 	struct crypto_akcipher_spawn *spawn = &ctx->spawn;
 
 	crypto_drop_akcipher(spawn);
-	kfree(ctx->hash_name);
 	kfree(inst);
 }
 
 static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 {
+	const struct rsa_asn1_template *digest_info;
 	struct crypto_attr_type *algt;
 	struct akcipher_instance *inst;
 	struct pkcs1pad_inst_ctx *ctx;
@@ -676,7 +664,11 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	hash_name = crypto_attr_alg_name(tb[2]);
 	if (IS_ERR(hash_name))
-		hash_name = NULL;
+		return PTR_ERR(hash_name);
+
+	digest_info = rsa_lookup_asn1(hash_name);
+	if (!digest_info)
+		return -EINVAL;
 
 	inst = kzalloc(sizeof(*inst) + sizeof(*ctx), GFP_KERNEL);
 	if (!inst)
@@ -684,7 +676,7 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	ctx = akcipher_instance_ctx(inst);
 	spawn = &ctx->spawn;
-	ctx->hash_name = hash_name ? kstrdup(hash_name, GFP_KERNEL) : NULL;
+	ctx->digest_info = digest_info;
 
 	crypto_set_spawn(&spawn->base, akcipher_crypto_instance(inst));
 	err = crypto_grab_akcipher(spawn, rsa_alg_name, 0,
@@ -696,27 +688,14 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = -ENAMETOOLONG;
 
-	if (!hash_name) {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_name) >=
-					CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)",
-			     rsa_alg->base.cra_driver_name) >=
-					CRYPTO_MAX_ALG_NAME)
+	if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)", rsa_alg->base.cra_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME ||
+	    snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
+		     "pkcs1pad(%s,%s)",
+		     rsa_alg->base.cra_driver_name, hash_name) >=
+	    CRYPTO_MAX_ALG_NAME)
 		goto out_drop_alg;
-	} else {
-		if (snprintf(inst->alg.base.cra_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_name, hash_name) >=
-				CRYPTO_MAX_ALG_NAME ||
-		    snprintf(inst->alg.base.cra_driver_name,
-			     CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)",
-			     rsa_alg->base.cra_driver_name, hash_name) >=
-					CRYPTO_MAX_ALG_NAME)
-		goto out_free_hash;
-	}
 
 	inst->alg.base.cra_flags = rsa_alg->base.cra_flags & CRYPTO_ALG_ASYNC;
 	inst->alg.base.cra_priority = rsa_alg->base.cra_priority;
@@ -738,12 +717,10 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb)
 
 	err = akcipher_register_instance(tmpl, inst);
 	if (err)
-		goto out_free_hash;
+		goto out_drop_alg;
 
 	return 0;
 
-out_free_hash:
-	kfree(ctx->hash_name);
 out_drop_alg:
 	crypto_drop_akcipher(spawn);
 out_free_inst:

^ permalink raw reply related


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