[PATCH 00/17] Multiple changes to crypto/ansi_cprng.c
Thank you all for your hospitality to my amateurish efforts.
Given that this all started with me reading the code in an attempt to
understand it, it is likely that some of the problems I address are
actually misunderstandings on my part. If all I get from this patch series
is some enlightenment, that's okay.
It's even more likely that some parts contain the germ of a good idea,
but will need considerable rework to be acceptable. I look forward
to that too.
Expecting that much more work will be needed, I've run the testmgr.h
test vectors on this code, but haven't desk-checked it as thoroughly as
security-sensitive code should be before final acceptance. If the ideas
pass muster, I'll double-check the implementatoin details.
Really, I'm just trying to understand the code. Patches are just
a very specific way of talking about it.
Here's an overview of the series. It's a lot of code cleanup, and
a bit of semantic change.
[01/17] crypto: ansi_cprng - Rename rand_data_valid more sensibly
I find it confusing, so I rename it to rand_data_pos
[02/17] crypto: ansi_cprng - Eliminate ctx-last_rand_data
Shrink the context structure.
[03/17] crypto: ansi_cprng - Eliminate ctx-I
Shrink it some more.
[04/17] crypto: ansi_cprng - simplify xor_vectors() to xor_block()
We don't need a size parameter.
[05/17] crypto: ansi_cprng - Add const annotations to hexdump()
I like const.
[06/17] crypto: ansi_cprng - Eliminate unused PRNG_FIXED_SIZE flag
It's dead code ACAICT.
[07/17] crypto: ansi_cprng - Shrink rand_read_pos flags
A little more context shrinking.
[08/17] crypto: ansi_cprng - Require non-null key V in reset_prng_context
Sue to the PRNG_NEED_RESET flag, cprng_init() doesn't need to call
reset_prng_context) directly.
[09/17] crypto: ansi_cprng - Clean up some variable types
Try to be more consistent about signed vs. unsigned.
[10/17] crypto: ansi_cprng - simplify get_prng_bytes
Code shrink simplification.
[11/17] crypto: ansi_cprng - unroll _get_more_prng_bytes
Slight object code size savings, and definite readability improvement.
[12/17] crypto: ansi_cprng - Create a block buffer data type
union { u8 bytes[16]; u32 ints[4]; u64 longs[2]; }, sort of.
[13/17] crypto: ansi_cprng - If DT is not provided, use a fresh timestamp
This is the big semantic change. I'm rather proud of the use
of get_random_int() as a timestamp, but feedback is definitely
solicited!
[14/17] crypto: ansi_cprng - If DT is omitted, don't buffer old output
Not sure if this is desirable or not.
[15/17] crypto: testmgr - Teach test_cprng to handle non-default seed sizes
I consider this a latent bug that patch 17 sould expose.
[16/17] crypto: testmgr - Merge seed arrays in struct cprng_testvec
I think this is a better way of solving the preceding problem,
but it's more intrusive. All the consts I add are not a critical
part of the patch, but an example of what I'd like to do to the
rest of the code.
[17/17] crypto: ansi_cprng - Shrink default seed size
This makes (some amount of) true entropy the default.
--
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 01/17] crypto: ansi_cprng - Rename rand_data_valid more sensibly
It's more like rand_data_invalid (data which has already been output),
so it's a pretty bad misnomer. But rand_data_pos is even better.
Signed-off-by: George Spelvin li...@horizon.com
---
crypto/ansi_cprng.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 97fe3110..c9e1684b 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -50,7 +50,7 @@ struct prng_context {
unsigned char DT[DEFAULT_BLK_SZ];
unsigned char I[DEFAULT_BLK_SZ];
unsigned char V[DEFAULT_BLK_SZ];
- u32 rand_data_valid;
+ u32 rand_read_pos; /* Offset into rand_data[] */
struct crypto_cipher *tfm;
u32 flags;
};
@@ -174,7 +174,7 @@ static int _get_more_prng_bytes(struct prng_context *ctx,
int cont_test)
}
dbgprint(Returning new block for context %p\n, ctx);
- ctx-rand_data_valid = 0;
+ ctx-rand_read_pos = 0;
hexdump(Output DT: , ctx-DT, DEFAULT_BLK_SZ);
hexdump(Output I: , ctx-I, DEFAULT_BLK_SZ);
@@ -217,7 +217,7 @@ static int get_prng_bytes(char *buf, size_t nbytes, struct
prng_context *ctx,
remainder:
- if (ctx-rand_data_valid == DEFAULT_BLK_SZ) {
+ if (ctx-rand_read_pos == DEFAULT_BLK_SZ) {
if (_get_more_prng_bytes(ctx, do_cont_test) 0) {
memset(buf, 0, nbytes);
err = -EINVAL;
@@ -230,12 +230,9 @@ remainder:
*/
if (byte_count DEFAULT_BLK_SZ) {
empty_rbuf:
- while (ctx-rand_data_valid DEFAULT_BLK_SZ) {
- *ptr = ctx-rand_data[ctx-rand_data_valid];
- ptr++;
- byte_count--;
- ctx-rand_data_valid++;
- if (byte_count == 0)
+ while (ctx-rand_read_pos DEFAULT_BLK_SZ) {
+ *ptr++ = ctx-rand_data[ctx-rand_read_pos++];
+ if (--byte_count == 0)
goto done;
}
}
@@ -244,17 +241,17 @@ empty_rbuf:
* Now copy whole blocks
*/
for (; byte_count = DEFAULT_BLK_SZ; byte_count -= DEFAULT_BLK_SZ) {
- if (ctx-rand_data_valid == DEFAULT_BLK_SZ) {
+ if (ctx-rand_read_pos == DEFAULT_BLK_SZ) {
if (_get_more_prng_bytes(ctx, do_cont_test) 0) {
memset(buf, 0, nbytes);
err = -EINVAL;
goto done;
}
}
- if (ctx-rand_data_valid 0)
+ if (ctx-rand_read_pos 0)
goto empty_rbuf;
memcpy(ptr, ctx-rand_data, DEFAULT_BLK_SZ);
- ctx-rand_data_valid += DEFAULT_BLK_SZ;
+ ctx-rand_read_pos += DEFAULT_BLK_SZ;
ptr += DEFAULT_BLK_SZ;
}
@@ -304,7 +301,7 @@ static int reset_prng_context(struct prng_context *ctx,
memset(ctx-rand_data, 0, DEFAULT_BLK_SZ);
memset(ctx-last_rand_data, 0, DEFAULT_BLK_SZ);
- ctx-rand_data_valid = DEFAULT_BLK_SZ;
+ ctx-rand_read_pos = DEFAULT_BLK_SZ;/* Force immediate refill */
ret = crypto_cipher_setkey(ctx-tfm, prng_key, klen);
if (ret) {
@@ -413,7 +410,7 @@ static int fips_cprng_reset(struct crypto_rng *tfm, u8
*seed, unsigned int slen)
/* this primes our continuity test */
rc = get_prng_bytes(rdata, DEFAULT_BLK_SZ, prng, 0);
- prng-rand_data_valid = DEFAULT_BLK_SZ;
+ prng-rand_read_pos = DEFAULT_BLK_SZ;
out:
return rc;
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/17] crypto: ansi_cprng - Eliminate ctx-last_rand_data
It's simply not necessary.
Signed-off-by: George Spelvin li...@horizon.com
---
crypto/ansi_cprng.c | 28 +++-
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index c9e1684b..c0a27288 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -46,7 +46,6 @@
struct prng_context {
spinlock_t prng_lock;
unsigned char rand_data[DEFAULT_BLK_SZ];
- unsigned char last_rand_data[DEFAULT_BLK_SZ];
unsigned char DT[DEFAULT_BLK_SZ];
unsigned char I[DEFAULT_BLK_SZ];
unsigned char V[DEFAULT_BLK_SZ];
@@ -89,8 +88,6 @@ static int _get_more_prng_bytes(struct prng_context *ctx, int
cont_test)
{
int i;
unsigned char tmp[DEFAULT_BLK_SZ];
- unsigned char *output = NULL;
-
dbgprint(KERN_CRIT Calling _get_more_prng_bytes for context %p\n,
ctx);
@@ -103,6 +100,7 @@ static int _get_more_prng_bytes(struct prng_context *ctx,
int cont_test)
* This algorithm is a 3 stage state machine
*/
for (i = 0; i 3; i++) {
+ unsigned char *output;
switch (i) {
case 0:
@@ -115,23 +113,23 @@ static int _get_more_prng_bytes(struct prng_context *ctx,
int cont_test)
hexdump(tmp stage 0: , tmp, DEFAULT_BLK_SZ);
break;
case 1:
-
/*
-* Next xor I with our secret vector V
-* encrypt that result to obtain our
-* pseudo random data which we output
+* Next xor I with our secret vector V.
+* Encrypt that result to obtain our pseudo random
+* data which we output. It is kept temporarily
+* in (no longer used) V until we have done the
+* anti-repetition compare.
*/
xor_vectors(ctx-I, ctx-V, tmp, DEFAULT_BLK_SZ);
hexdump(tmp stage 1: , tmp, DEFAULT_BLK_SZ);
- output = ctx-rand_data;
+ output = ctx-V;
break;
case 2:
/*
* First check that we didn't produce the same
-* random data that we did last time around through this
+* random data that we did last time around.
*/
- if (!memcmp(ctx-rand_data, ctx-last_rand_data,
- DEFAULT_BLK_SZ)) {
+ if (!memcmp(ctx-V, ctx-rand_data, DEFAULT_BLK_SZ)) {
if (cont_test) {
panic(cprng %p Failed repetition
check!\n,
ctx);
@@ -144,15 +142,13 @@ static int _get_more_prng_bytes(struct prng_context *ctx,
int cont_test)
ctx-flags |= PRNG_NEED_RESET;
return -EINVAL;
}
- memcpy(ctx-last_rand_data, ctx-rand_data,
- DEFAULT_BLK_SZ);
+ memcpy(ctx-rand_data, ctx-V, DEFAULT_BLK_SZ);
/*
* Lastly xor the random data with I
* and encrypt that to obtain a new secret vector V
*/
- xor_vectors(ctx-rand_data, ctx-I, tmp,
- DEFAULT_BLK_SZ);
+ xor_vectors(ctx-I, ctx-V, tmp, DEFAULT_BLK_SZ);
output = ctx-V;
hexdump(tmp stage 2: , tmp, DEFAULT_BLK_SZ);
break;
@@ -161,7 +157,6 @@ static int _get_more_prng_bytes(struct prng_context *ctx,
int cont_test)
/* do the encryption */
crypto_cipher_encrypt_one(ctx-tfm, output, tmp);
-
}
/*
@@ -299,7 +294,6 @@ static int reset_prng_context(struct prng_context *ctx,
memset(ctx-DT, 0, DEFAULT_BLK_SZ);
memset(ctx-rand_data, 0, DEFAULT_BLK_SZ);
- memset(ctx-last_rand_data, 0, DEFAULT_BLK_SZ);
ctx-rand_read_pos = DEFAULT_BLK_SZ;/* Force immediate refill */
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 04/17] crypto: ansi_cprng - simplify xor_vectors() to xor_block()
It doesn't need a second input or a length parameter.
Signed-off-by: George Spelvin li...@horizon.com
---
crypto/ansi_cprng.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 6b844f13..4856c84c7 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -74,14 +74,12 @@ if (dbg)\
printk(format, ##args);\
} while (0)
-static void xor_vectors(unsigned char *in1, unsigned char *in2,
- unsigned char *out, unsigned int size)
+static void xor_block(unsigned char const *in, unsigned char *out)
{
int i;
- for (i = 0; i size; i++)
- out[i] = in1[i] ^ in2[i];
-
+ for (i = 0; i DEFAULT_BLK_SZ; i++)
+ out[i] ^= in[i];
}
/*
* Returns DEFAULT_BLK_SZ bytes of random data per call
@@ -123,7 +121,7 @@ static int _get_more_prng_bytes(struct prng_context *ctx,
int cont_test)
* in (no longer used) V until we have done the
* anti-repetition compare.
*/
- xor_vectors(tmp, ctx-V, ctx-V, DEFAULT_BLK_SZ);
+ xor_block(tmp, ctx-V);
hexdump(input stage 1: , ctx-V, DEFAULT_BLK_SZ);
input = output = ctx-V;
break;
@@ -151,7 +149,7 @@ static int _get_more_prng_bytes(struct prng_context *ctx,
int cont_test)
* Lastly xor the random data with I
* and encrypt that to obtain a new secret vector V
*/
- xor_vectors(tmp, ctx-V, ctx-V, DEFAULT_BLK_SZ);
+ xor_block(tmp, ctx-V);
hexdump(input stage 2: , ctx-V, DEFAULT_BLK_SZ);
input = output = ctx-V;
break;
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/17] crypto: ansi_cprng - Shrink default seed size
The larger seed with deterministic DT is still supported, but make the
default non-deterministically random, with fresh DT.
This must come after the patch that makes testmgr.c not depend on
the default seedsize to allocate its buffers, since it tests the
non-default (larger seed) case.
Signed-off-by: George Spelvin li...@horizon.com
---
crypto/ansi_cprng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Patch 15 is a prerequisite for this one.
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 4ed7c0cf..875c4bf9 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -390,7 +390,7 @@ static struct crypto_alg rng_algs[] = { {
.rng = {
.rng_make_random= cprng_get_random,
.rng_reset = cprng_reset,
- .seedsize = DEFAULT_PRNG_KSZ + 2*DEFAULT_BLK_SZ,
+ .seedsize = DEFAULT_BLK_SZ + DEFAULT_PRNG_KSZ
}
}
#ifdef CONFIG_CRYPTO_FIPS
@@ -408,7 +408,7 @@ static struct crypto_alg rng_algs[] = { {
.rng = {
.rng_make_random= fips_cprng_get_random,
.rng_reset = fips_cprng_reset,
- .seedsize = DEFAULT_PRNG_KSZ + 2*DEFAULT_BLK_SZ,
+ .seedsize = DEFAULT_BLK_SZ + DEFAULT_PRNG_KSZ
}
}
#endif
--
2.1.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 02/17] crypto: ansi_cprng - Eliminate ctx-last_rand_data
From smuel...@chronox.de Tue Dec 02 08:57:23 2014 X-AuthUser: s...@eperm.de From: Stephan Mueller smuel...@chronox.de To: George Spelvin li...@horizon.com Cc: herb...@gondor.apana.org.au, nhor...@tuxdriver.com, linux-crypto@vger.kernel.org Subject: Re: [PATCH 02/17] crypto: ansi_cprng - Eliminate ctx-last_rand_data Date: Tue, 02 Dec 2014 09:57:17 +0100 User-Agent: KMail/4.14.2 (Linux/3.17.2-200.fc20.x86_64; KDE/4.14.2; x86_64; ; ) In-Reply-To: 20141202083550.17918.qm...@ns.horizon.com References: 20141202083550.17918.qm...@ns.horizon.com MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset=us-ascii Am Dienstag, 2. Dezember 2014, 03:35:50 schrieb George Spelvin: Hi George, It's simply not necessary. Can you please be a bit more verbose on why you think this is not necessary? Sorry, I thought the code made that obvious. The two buffers have to exist simultaneously very briefly in order to be compared, but the old data can be overwritten immediately thereafter. So what the revised code does is: I := E(DT) (The buffer is called tmp) V ^= I V := E(V) (This can be stored in V without problems) compare V with read_data read_data := V V ^= I V := E(V) Have you tested that change with reference test vectors -- what do testmgr test vectors say? As I explained in part 00, yes. The behaviour is identical. I should mention, however, that I did not exactly use testmgr; I cut pasted the relevant test vectors code into ansi_cprng.c, then verified that the tests passed with both old and modified code. I have so far been unable to figure out how to make the tcrypt module do anything useful. -- 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: Is ansi_cprng.c supposed to be an implmentation of X9.31?
On Mon, Dec 01, 2014 at 11:55:18PM -0500, George Spelvin wrote: The other one, which I have to think *very* hard about, is whether using the same seed material as /dev/random in this much weaker cryptographic construction will introduce a flaw in *it*. I have no idea what you are referring to here. The caller is requred to seed the DRNG. Typically that is done with a call to get_random_bytes. As Neil mentioned, the X9.31 DRNG implementation does not seed itself. Er, yes it does. Not the key, but the IV vector V[]. Here's the closest thing I can find on-line to a direct quote from the ANSI Spec: http://www.untruth.org/~josh/security/ansi931rng.PDF Let DT be a date/time vector which is updated on each iteration. This timestamp is a source of continuous reseed material. The spec doesn't impose specific resolution requirements, but it's hopefully obvious that the finer, the better. The goal is a vector which changes per iteration. That is an old version. The updated version (published in 2005), and specified in the ansi_cprng.c file removes that language. There are limitations due to its finite entropy and lack of catastrophic reseeding, as Kelsey Schneier pointed out: https://www.schneier.com/paper-prngs.html but it is intended as an entropy source. Again, that language is removed in the more recent version. Using DT as an entropy source, and updating it on each iteration also destroys the predictive nature of the cprng when two peers use it to communicate. That is to say, if a DT vector is updated every iteration, and the resultant R value is used as a key to encrypt data, the validitiy of that key at a peer host using an identically seeded cprng to decrypt is limited by the granularity of the DT value. That is to say, if you update DT every second in cprng, an entity that knows the secret key can only decrypt that data up to a second after its encryption, unless the decrypting entity also knows at exactly what time the data was encrypted. If you share the DT value, any entropy it provides becomes worthless, as it becomes widely known. The long and the short of it is that, if you want a cprng who's output can be predicted by any entity with the IV and KEY values, then DT has to be known initially and updated in a predictable fashion that is independent of the data being transmitted. Using a real date/time vector can't do that. Hopefully very soon I'll have a patch series to post. (Unfortunately, I just managed to oops my kernel and need to reboot to continue testing.) If I don't do much more, it'll be patch 13/17 in the series. I think the use of get_random_int() is a good compromise between the raw random_get_entropy() timestamp and the (too heavy) get_random_bytes(). -- 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 -- 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: Is ansi_cprng.c supposed to be an implmentation of X9.31?
On Tue, Dec 02, 2014 at 12:39:30AM -0500, George Spelvin wrote:
See Documentation/DocBook/crypto-API.tmpl in the cryptodev-2.6 tree.
There you will find tons of documentation (which will be merged during
3.19-rc1)
Yes, I've been reading that. It certainly helps a great deal, but
still leaves me with some significant questions.
I started researching the crypto layer when I proposed using Dan
Bernstein's SipHash elsewhere in the kernel and someone asked for a
crypto API wrapper for it. That seemed a simple enough request to me,
but it's been a deeper rabbit hole than I expected.
I started reading the code to another keyed hash, michael_mic, as a model,
but I'm stil trying to understand the intended difference between struct
crypto_shash and struct shash_desc, and in particular why both have
a copy of the key. The SHASH API documentation at
https://git.kernel.org/cgit/linux/kernel/git/herbert/cryptodev-2.6.git/tree/include/crypto/hash.h
isn't particularly enlightening. If the crypto_shash were entirely
read-only and the shash_desc were the entire volatile state, that would
make sense, but as it is I'm confused about the design intent.
Not sure what you're concerned about, or what you're even referencing. A
struct crypto_shash is the object retured by crypto_alloc_shash, and represents
an instance of a synchronous hash algorithm:
struct crypto_shash {
unsigned int descsize;
struct crypto_tfm base;
};
The key is stored in the base.__crt_ctx part of the structure in a algorithm
specific manner.
The shash_desc structure represents a discrete block of data that is being
hashed. It can be updated with new data, reset, finalized, etc. It only points
to the crypto_hash structure that it is associated with (as it must, given that
the crypto layer needs to know what algorithm to use to hash the data and what
key to use). But it doesn't store a second copy of the key on its own.
(On a related point, the general lack of const declarations throughout the
crypto layer has been a source of frustration.)
So fix it. Making large claims about what frustrates you about the code without
producing any fixes doesn't make people listen to you.
The other big issue I'm struggling with is how to get the tcrypt.ko module
to print ansi_cprng has passed its tests. All it has produced for me
so far is a few kilobytes of dmesg spam about ciphers which aren't even
configured in my kernel.
The tests only print out a pass fail notification in fips mode, that seems
pretty clear if you look at the alg_test function.
After a few hours of trying to figure out what the alg and type parameters
do, I gave up and cut and pasted the tests into prng_mod_init().
They're used to differentiate algorithms that can be used for multiple purposes.
See the CRYPTO_ALG_TYPE_* defines in crypto.h. For the CPRNG, they do nothing
since an RNG is only an RNG.
--
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
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: algif - Mark sgl end at the end of data
On Mon, Dec 01, 2014 at 07:03:00AM -0800, Tadeusz Struk wrote: Ok, I can look at the data, but do you think the idea with marking the end of data in TX sgl is worthwhile or should I just forget about it. Another question is - is an sgl with lots of empty buffers a valid input from an algorithm implementation point of view? I think marking the end is useful. How about doing the marking and unmarking whenever sgl-cur is updated? Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au 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 02/17] crypto: ansi_cprng - Eliminate ctx-last_rand_data
On Tue, Dec 02, 2014 at 03:35:50AM -0500, George Spelvin wrote: It's simply not necessary. Signed-off-by: George Spelvin li...@horizon.com NACK The assumption that its not needed is incorrect. In fips mode its explicitly needed to validate that the rng isn't reproducing identical random data. Neil -- 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 03/17] crypto: ansi_cprng - Eliminate ctx-I
On Tue, Dec 02, 2014 at 03:37:07AM -0500, George Spelvin wrote: It's also not necessary. We do have to change some debugging output. Signed-off-by: George Spelvin li...@horizon.com --- crypto/ansi_cprng.c | 39 --- 1 file changed, 20 insertions(+), 19 deletions(-) I'm only ok with removing I if you can continue to be able to output it. given that I is listed as part of the test sequences that NIST provides, I'd like to be able to compare the values. Neil -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: algif_skcipher - initialize upon init request
On Sun, Nov 30, 2014 at 10:55:26AM +0100, Stephan Mueller wrote: When using the algif_skcipher, the following call sequence causess a re-initialization: 1. sendmsg with ALG_SET_OP and iov == NULL, iovlen == 0 (i.e initializing the cipher, but not sending data) 2. sendmsg with msg-msg-controllen == 0 and iov != NULL (using the initalized cipher handle by sending data) In step 2, the cipher operation type (encryption or decryption) is reset to always decryption, because the local variable of enc is put into ctx-enc as ctx-user is still zero. The same applies when all send data is processed and ctx-used falls to zero followed by user space to send new data. This patch changes the behavior to only reset the cipher operation type (and the IV) if such configuration request is received. Signed-off-by: Stephan Mueller smuel...@chronox.de Patch applied. -- Email: Herbert Xu herb...@gondor.apana.org.au 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 07/17] crypto: ansi_cprng - Shrink rand_read_pos flags
On Tue, Dec 02, 2014 at 03:43:13AM -0500, George Spelvin wrote:
rand_read_pos is never more than 16, while there's only 1 flag
bit allocated, so we can shrink the context a little.
Signed-off-by: George Spelvin li...@horizon.com
---
crypto/ansi_cprng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
They're also reordered to avoid alignment holes.
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 93ed00f6..f40f54cd 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -51,9 +51,9 @@ struct prng_context {
unsigned char rand_data[DEFAULT_BLK_SZ];
unsigned char DT[DEFAULT_BLK_SZ];
unsigned char V[DEFAULT_BLK_SZ];
- u32 rand_read_pos; /* Offset into rand_data[] */
+ unsigned char rand_read_pos;/* Offset into rand_data[] */
u8 please. Also, not sure if this helps much, as I think the padding will just
get you back to word alignment on each of these.
+ unsigned char flags;
struct crypto_cipher *tfm;
- u32 flags;
};
static int dbg;
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: algif - Mark sgl end at the end of data
Hi, On 12/02/2014 06:33 AM, Herbert Xu wrote: I think marking the end is useful. How about doing the marking and unmarking whenever sgl-cur is updated? I have a v2 ready where I mark it based on the actual data. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] crypto: algif - Mark sgl end at the end of data
Hi,
algif_skcipher sends 127 sgl buffers for encryption regardless of how
many buffers acctually have data to process, where the few first with
valid len and the rest with zero len. This is not very eficient and may cause
problems when algs do something like this without checking the buff
lenght:
for_each_sg(sgl, sg, sg_nents, i)
sg_virt(sg)
This patch marks the last one with data as the last one to process.
Changes:
v2 - use data len to find the last buffer instead of nents in RX list.
Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
---
crypto/algif_skcipher.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index f2a88a7..cde507f 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -414,6 +414,23 @@ unlock:
return err ?: size;
}
+static int skcipher_get_nents_with_data(struct scatterlist *sg, int len)
+{
+ int nents;
+
+ for (nents = 0; sg; sg = sg_next(sg)) {
+ if (!sg)
+ return nents;
+
+ len -= sg-length;
+ if (!(len 0))
+ return nents;
+ nents++;
+ }
+
+ return 0;
+}
+
static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
struct msghdr *msg, size_t ignored, int flags)
{
@@ -437,6 +454,8 @@ static int skcipher_recvmsg(struct kiocb *unused, struct
socket *sock,
char __user *from = iov-iov_base;
while (seglen) {
+ int nents;
+
sgl = list_first_entry(ctx-tsgl,
struct skcipher_sg_list, list);
sg = sgl-sg;
@@ -464,6 +483,8 @@ static int skcipher_recvmsg(struct kiocb *unused, struct
socket *sock,
if (!used)
goto free;
+ nents = skcipher_get_nents_with_data(sg, used);
+ sg_mark_end(sg[nents]);
ablkcipher_request_set_crypt(ctx-req, sg,
ctx-rsgl.sg, used,
ctx-iv);
@@ -473,6 +494,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct
socket *sock,
crypto_ablkcipher_encrypt(ctx-req) :
crypto_ablkcipher_decrypt(ctx-req),
ctx-completion);
+ sg_unmark_end(sg[nents]);
free:
af_alg_free_sg(ctx-rsgl);
--
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: crypto: qat - Intel(R) QAT transport code
On 12/02/2014 04:21 AM, Dan Carpenter wrote: drivers/crypto/qat/qat_common/adf_transport.c 407 /* Enable IRQ coalescing always. This will allow to use 408 * the optimised flag and coalesc register. 409 * If it is disabled in the config file just use min time value */ 410 if (adf_get_cfg_int(accel_dev, Accelerator0, 411 ADF_ETRMGR_COALESCING_ENABLED_FORMAT, 412 bank_num, coalesc_enabled) coalesc_enabled) ^^^ This condition is reversed, so it only enables coalescing on error. Hello Dan, Yes, you are correct. Looks like we never enable interrupt coalescing. Thanks for reporting the issue. Patch fixing this will be out soon. Regards, Tadeusz -- 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 01/17] crypto: ansi_cprng - Rename rand_data_valid more sensibly
if (byte_count DEFAULT_BLK_SZ) {
empty_rbuf:
- while (ctx-rand_data_valid DEFAULT_BLK_SZ) {
- *ptr = ctx-rand_data[ctx-rand_data_valid];
- ptr++;
- byte_count--;
- ctx-rand_data_valid++;
- if (byte_count == 0)
+ while (ctx-rand_read_pos DEFAULT_BLK_SZ) {
+ *ptr++ = ctx-rand_data[ctx-rand_read_pos++];
+ if (--byte_count == 0)
goto done;
I am against such collapsing of constructs into one-liners. It is not
obvious at first sight, which value gets incremented in what order. Such
collapsing was the cause for CVE-2013-4345 as it caused an off-by one.
Given that this turns out to the the exact code where that happened, I can
see the sensitivity of the matter! But I disagree in this case, and
it took me a while to figure out how to phrase it.
It's a code style issue, which means that it doesn't have a clear
technical answer. It's more of a voting on what people think is
clearer thing.
In the case of if (--byte_count) issue, that's not something I feel very
strongly about.
But in the case of *ptr++ = ctx-rand_data[ctx-rand_read_pos++], it's
the opposite. While I'm not going to pick a fight over it, my opinion
is that this form is clearly better.
There are two advantages to the code in this form:
1. *dst++ = *src++ is a C idiom, like for (i = 0; i N; i++).
It's very easy to recognize and understand. A more broken-up form
less obviously does all the necessary accounting.
2. The original bug was NOT caused by using side effects. Consider the
bug fix:
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -230,11 +230,11 @@ remainder:
*/
if (byte_count DEFAULT_BLK_SZ) {
empty_rbuf:
- for (; ctx-rand_data_valid DEFAULT_BLK_SZ;
- ctx-rand_data_valid++) {
+ while (ctx-rand_data_valid DEFAULT_BLK_SZ) {
*ptr = ctx-rand_data[ctx-rand_data_valid];
ptr++;
byte_count--;
+ ctx-rand_data_valid++;
if (byte_count == 0)
goto done;
}
The problem was the *separation* of the copy and the associated increment.
In some situations, one was done and not the other, leading to data
duplication. The fix was to move the increment of ctx-rand_data_valid
to before the if (byte_count == 0) loop exit test.
However, putting the increment in the same line as the copy reduces the
separation that caused the original bug, and makes it *more* clear that
the parts always happen together.
This, fundamentally, is the reason that I actually find it preferable:
it's conceptually one operation that should always have all of its parts
done together, and putting it on one line makes that easier for
the reader to recognize.
The problem with the original was putting it in the for(), not
using side effect expressions.
The above logic doesn't apply to if (--byte_count), which is why I
don't care about it nearly as much.
All that said, there are two significant remaining points:
3. this patch claims it's just a variable rename; perhaps it should stick
to that, and
4. patch 10/15 totally deletes this code and replaces it with something
even simpler, so if that is acceptable this entire discussion may be moot.
--
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: Is ansi_cprng.c supposed to be an implmentation of X9.31?
That is an old version. The updated version (published in 2005), and specified in the ansi_cprng.c file removes that language. Oh! Thank you! I'm pretty sure I read the 1998 version. In fact, apparently there's a 2010 version: http://www.codesdownload.org/3761-ANSI-X9-TR-31-2010.html I need to figure out how to get my hands on that. Presumably this is the 2005 version with the 2009 supplement incorporated. If I could read Chinese, I might be able to find it here: http://www.docin.com/p-524511188.html The long and the short of it is that, if you want a cprng who's output can be predicted by any entity with the IV and KEY values, then DT has to be known initially and updated in a predictable fashion that is independent of the data being transmitted. Using a real date/time vector can't do that. Er, yes, this is all extremely obvious; I'm not quite sure why we're belabouring it. Fully deterministic generators have their uses, which is why I had to ask in the beginning what the design intent was. If this is *intended* to be purely deterministic, there's nothing to fix. I'd like to propose a small comment clarification because a quick reading confused me. But when I talked about making it random, you said send a patch, so I did. If you don't want the semantic change, I'm not upset. The other code cleanups are hopefully (after I've finished polishing them) useful; just stop before the whole union block business. -- 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: crypto: qat - Intel(R) QAT transport code
On 12/02/2014 08:49 AM, Tadeusz Struk wrote: On 12/02/2014 04:21 AM, Dan Carpenter wrote: drivers/crypto/qat/qat_common/adf_transport.c 407 /* Enable IRQ coalescing always. This will allow to use 408 * the optimised flag and coalesc register. 409 * If it is disabled in the config file just use min time value */ 410 if (adf_get_cfg_int(accel_dev, Accelerator0, 411 ADF_ETRMGR_COALESCING_ENABLED_FORMAT, 412 bank_num, coalesc_enabled) coalesc_enabled) ^^^ This condition is reversed, so it only enables coalescing on error. Hello Dan, Yes, you are correct. Looks like we never enable interrupt coalescing. Thanks for reporting the issue. Patch fixing this will be out soon. Regards, Tadeusz Hi, I had to take a closer look to see what's going on there. We do enable coalescing always, just as the comment states (in function adf_enable_ring_irq(), line 105). The adf_enable_coalesc() function only reads the coalescing timer from the configuration and stores it in the bank-irq_coalesc_timer. Your point is still valid - the condition is reversed so we have always used ADF_COALESCING_MIN_TIME. What's also missing is initialization of coalesc_enabled and the adf_enable_coalesc() function should really be called adf_get_coalesc_timer() I'll send a patch to fix it. Thanks again. Regards, Tadeusz -- 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: Is ansi_cprng.c supposed to be an implmentation of X9.31?
Not sure what you're concerned about, or what you're even referencing.
Sorry for the confusion, but it's genuine. See below for what I think is
the solution.
The shash_desc structure represents a discrete block of data that is
being hashed. It can be updated with new data, reset, finalized, etc.
It only points to the crypto_hash structure that it is associated with
(as it must, given that the crypto layer needs to know what algorithm
to use to hash the data and what key to use). But it doesn't store a
second copy of the key on its own.
Er, I saw the following code:
static int michael_setkey(struct crypto_shash *tfm, const u8 *key,
unsigned int keylen)
{
struct michael_mic_ctx *mctx = crypto_shash_ctx(tfm);
const __le32 *data = (const __le32 *)key;
if (keylen != 8) {
crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
return -EINVAL;
}
mctx-l = le32_to_cpu(data[0]);
mctx-r = le32_to_cpu(data[1]);
return 0;
}
and thought okay, the key is in mctx-l and mctx-r.
Then I saw
static int michael_init(struct shash_desc *desc)
{
struct michael_mic_desc_ctx *mctx = shash_desc_ctx(desc);
struct michael_mic_ctx *ctx = crypto_shash_ctx(desc-tfm);
mctx-pending_len = 0;
mctx-l = ctx-l;
mctx-r = ctx-r;
return 0;
}
and thought the key is being copied from the ctx to the desc. Why
the heck are they doing it in two steps?
I spent a long time trying to figure out why there were two separate
layers, as it looked like
struct michael_mic_ctx {
u32 l, r;
};
was a strict subset of
struct michael_mic_desc_ctx {
u8 pending[4];
size_t pending_len;
u32 l, r;
};
and thus the former was unnecessary.
(Another code optimization that jumps out at me: given that pending_len
is strictly between 0 and 3, using a 64-bit size_t unnecessarily bloats
the context structure to 24 bytes. Shrinking it to 32 bits would reduce
the context to 16 bytes, and given that at most three bytes of pending[]
are ever used, except transiently in the update() function, pending_len
could be stored in pending[3] and and the context shrunk to 12 bytes.)
I'm thinking that the confusion comes from my unfortunate choice of
which file to start reading and the peculiar way that michael_mic's key
is more like an IV, so there is neither key scheduling nor persistent
access to it.
== My guess as to how this works ==
(If I say anything wrong, please correct me!)
A ctx is an abstraction of a (scheduled) key. With symmetric ciphers,
it's very common to have a complex key set up (I'm looking at you, twofish)
which can be re-used for multiple messages/packets/whatever.
It has to be an opaque object because it might correspond to some
state in a hardware acclerator, with the software swapping keys in
and out like virtual memory.
A desc is the abstraction of an IV. It handles chaining and block
boundaries, to encrypt/hash/transform a stream/bvec/sk_buff of data.
The reason for the separation is that multiple descs may simultaneously
access the same ctx.
This is okay because the ctx reference is conceptually read-only.
(If there's swapping of hardware contexts going on behind the curtain,
the ctx may not be entirely immutable, but it's like a cache: you're
not supposed to notice the difference.)
There are some cases, like ECB, where a desc is a ridiculously thin
wrapper and seems like overkill, but it's there to provide a consistent
interface.
(Design choice: a single ctx can handle both encryption and decryption.
For algorithms that have different key schedules for the two directions,
it has to waste the space even if the cipher is only being used forward.)
(On a related point, the general lack of const declarations throughout the
crypto layer has been a source of frustration.)
So fix it. Making large claims about what frustrates you about the
code without producing any fixes doesn't make people listen to you.
I'm happy to! It's just a large, invasive, change and I wonder how welcome
it is. Maybe there's a reason for the omission that I'm not seeing?
You know the aphorism never attribute to malice that which can be
adequately explained by stupidity? Well, there's a follow-on, which
says be careful attributing to someone else's stupidity that which
can be adequately explained by your own.
It's a pattern I go through frequently, and I think others do too:
my initial reaction is what the @#$@# is this $#!+?, but I try to
consider whether the fault is actually mine before speaking aloud (or
hitting send, as the case may be).
If it's soemthing that annoys you too and you just haven't gotten around
to fixing it, I'd be delighted!
I just wanted to start with something smaller in scope, confined to
a single source file, where I felt like I understood the implications
better.
The other big issue I'm struggling with is how to get the
Re: [PATCH 02/17] crypto: ansi_cprng - Eliminate ctx-last_rand_data
NACK The assumption that its not needed is incorrect. In fips mode its explicitly needed to validate that the rng isn't reproducing identical random data. Please take a second look. The validation is still there; I fully understand that and preserved that. (Well, I broke it later getting over-eager looking for places to put memzero_explicit, but already sent a follow-on message about that.) Only the *buffer* is unnecessary and was deleted. -- 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 03/17] crypto: ansi_cprng - Eliminate ctx-I
I'm only ok with removing I if you can continue to be able to output it. given that I is listed as part of the test sequences that NIST provides, I'd like to be able to compare the values. I can do that easily, but I can't print the *input* I, which is the result of encrypting the previous DT, as it's thrown away earlier. You'd have to look further back in the debug messages to find it. Is changing the format of the debug messages okay? I'd like the debug messages to describe the code, but I don't know if you have something that parses the current output. The test output I see on p. 33 of http://csrc.nist.gov/groups/STM/cavp/documents/rng/RNGVS.pdf doesn't include I. Can you point me to a sample that includes I? It might be best to more significantly rework the debug messages to resemble the NIST test vectors. -- 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 07/17] crypto: ansi_cprng - Shrink rand_read_pos flags
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -51,9 +51,9 @@ struct prng_context {
unsigned char rand_data[DEFAULT_BLK_SZ];
unsigned char DT[DEFAULT_BLK_SZ];
unsigned char V[DEFAULT_BLK_SZ];
-u32 rand_read_pos; /* Offset into rand_data[] */
+unsigned char rand_read_pos;/* Offset into rand_data[] */
u8 please. Also, not sure if this helps much, as I think the padding
will just get you back to word alignment on each of these.
I noticed the unsigned char vs u8 issue, but didn't make the change
as I didn't think the readailility improvement was worth the code churn.
But I'd be happy to clean that up, too!
Should I convert all the buffers and function prototypes, or is there
some criterion for distinguishing which gets which? (E.g. buffers are
unsigned char but control variables are u8.)
And actually, you do win. spinlock_t is 16 bits on x86,
and the buffers are all 16 bytes. (80 bytes before my earlier
patches, 48 bytes after.)
So the the structure goes from:
32-bit 64-bit Variable
Offset SizeOffset Size
0 20 2 spinlock_t prng_lock
2 16 2 16 unsigned char rand_data[16]
18 16 18 16 unsigned char DT[16]
34 16 34 16 unsigned char V[16]
50 2 50 2 (alignemnt)
52 4 52 4 u32 rand_read_pos
56 4 56 8 struct crypto_cipher *tfm
60 4 64 4 u32 flags
68 4 (alignment)
64 72 (structure size)
to
32-bit 64-bit Variable
Offset SizeOffset Size
34 16 34 16 unsigned char V[16]
50 1 50 1 u8 rand_read_pos
51 1 51 1 u8 flags
52 4 (alignment)
52 4 56 8 struct crypto_cipher *tfm
56 64 (structure size)
You still get 4 bytes of alignment padding on x86-64, but given that
the structure has 60 bytes of payload, that's the minimum possible.
You save 6 bytes of variables and 2 bytes of padding on both
32- and 64-bit systems, for a total of 8 bytes, and that's enough
to knock you into a smaller slab object bin on 64-bit.
I forget where I read the terminology, but the most efficient
wat to pack a structure is in an organ pipe configuraiton where
the biggest (in terms of *alignment*) members are on the outside
and the structre and the smaller elements are on the inside.
Putting a 32-bit flags after a 64-bit pointer violates that.
--
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 11/17] crypto: ansi_cprng - unroll _get_more_prng_bytes
The order of the v1 patches is somewhat in order of increasing scale, reflecting my learning about the code. But if this unroll is okay, it would probably make the most sense to reorder things so it's first and then other changes can be made on top of the simpler code. Given the unusual implementation choice of a loop and a switch, it's obviously a deliberate one, so I assume there's a reason for it that I'm just not seeing. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] crypto: qat - fix problem with coalescing enable logic
Fixed issue reported by Dan Carpenter
410 if (adf_get_cfg_int(accel_dev, Accelerator0,
411 ADF_ETRMGR_COALESCING_ENABLED_FORMAT,
412 bank_num, coalesc_enabled) coalesc_enabled)
This condition is reversed, so it only enables coalescing on error.
Reported-by: Dan Carpenter dan.carpen...@oracle.com
Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
---
drivers/crypto/qat/qat_common/adf_transport.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/qat/qat_common/adf_transport.c
b/drivers/crypto/qat/qat_common/adf_transport.c
index 5f3fa45..5890992 100644
--- a/drivers/crypto/qat/qat_common/adf_transport.c
+++ b/drivers/crypto/qat/qat_common/adf_transport.c
@@ -376,8 +376,9 @@ static inline int adf_get_cfg_int(struct adf_accel_dev
*accel_dev,
return 0;
}
-static void adf_enable_coalesc(struct adf_etr_bank_data *bank,
- const char *section, uint32_t bank_num_in_accel)
+static void adf_get_coalesc_timer(struct adf_etr_bank_data *bank,
+ const char *section,
+ uint32_t bank_num_in_accel)
{
if (adf_get_cfg_int(bank-accel_dev, section,
ADF_ETRMGR_COALESCE_TIMER_FORMAT,
@@ -396,7 +397,7 @@ static int adf_init_bank(struct adf_accel_dev *accel_dev,
struct adf_hw_device_data *hw_data = accel_dev-hw_device;
struct adf_etr_ring_data *ring;
struct adf_etr_ring_data *tx_ring;
- uint32_t i, coalesc_enabled;
+ uint32_t i, coalesc_enabled = 0;
memset(bank, 0, sizeof(*bank));
bank-bank_number = bank_num;
@@ -407,10 +408,10 @@ static int adf_init_bank(struct adf_accel_dev *accel_dev,
/* Enable IRQ coalescing always. This will allow to use
* the optimised flag and coalesc register.
* If it is disabled in the config file just use min time value */
- if (adf_get_cfg_int(accel_dev, Accelerator0,
- ADF_ETRMGR_COALESCING_ENABLED_FORMAT,
- bank_num, coalesc_enabled) coalesc_enabled)
- adf_enable_coalesc(bank, Accelerator0, bank_num);
+ if ((adf_get_cfg_int(accel_dev, Accelerator0,
+ADF_ETRMGR_COALESCING_ENABLED_FORMAT, bank_num,
+coalesc_enabled) == 0) coalesc_enabled)
+ adf_get_coalesc_timer(bank, Accelerator0, bank_num);
else
bank-irq_coalesc_timer = ADF_COALESCING_MIN_TIME;
--
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
