2012-09-21 07:26:51

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 1/2] crypto: testmgr - make test_skcipher also test 'dst != src' code paths

Currrently test_skcipher uses same buffer for destination and source. However
in any places, 'dst != src' take different path than 'dst == src' case.

Therefore make test_skcipher also run tests with destination buffer being
different than source buffer.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
crypto/testmgr.c | 107 ++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 76 insertions(+), 31 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 8183777..00f54d5 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -761,8 +761,9 @@ out_nobuf:
return ret;
}

-static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
- struct cipher_testvec *template, unsigned int tcount)
+static int __test_skcipher(struct crypto_ablkcipher *tfm, int enc,
+ struct cipher_testvec *template, unsigned int tcount,
+ const bool diff_dst)
{
const char *algo =
crypto_tfm_alg_driver_name(crypto_ablkcipher_tfm(tfm));
@@ -770,16 +771,26 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
char *q;
struct ablkcipher_request *req;
struct scatterlist sg[8];
- const char *e;
+ struct scatterlist sgout[8];
+ const char *e, *d;
struct tcrypt_result result;
void *data;
char iv[MAX_IVLEN];
char *xbuf[XBUFSIZE];
+ char *xoutbuf[XBUFSIZE];
int ret = -ENOMEM;

if (testmgr_alloc_buf(xbuf))
goto out_nobuf;

+ if (diff_dst && testmgr_alloc_buf(xoutbuf))
+ goto out_nooutbuf;
+
+ if (diff_dst)
+ d = "-ddst";
+ else
+ d = "";
+
if (enc == ENCRYPT)
e = "encryption";
else
@@ -789,8 +800,8 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,

req = ablkcipher_request_alloc(tfm, GFP_KERNEL);
if (!req) {
- printk(KERN_ERR "alg: skcipher: Failed to allocate request "
- "for %s\n", algo);
+ pr_err("alg: skcipher%s: Failed to allocate request for %s\n",
+ d, algo);
goto out;
}

@@ -822,16 +833,21 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
ret = crypto_ablkcipher_setkey(tfm, template[i].key,
template[i].klen);
if (!ret == template[i].fail) {
- printk(KERN_ERR "alg: skcipher: setkey failed "
- "on test %d for %s: flags=%x\n", j,
- algo, crypto_ablkcipher_get_flags(tfm));
+ pr_err("alg: skcipher%s: setkey failed on test %d for %s: flags=%x\n",
+ d, j, algo,
+ crypto_ablkcipher_get_flags(tfm));
goto out;
} else if (ret)
continue;

sg_init_one(&sg[0], data, template[i].ilen);
+ if (diff_dst) {
+ data = xoutbuf[0];
+ sg_init_one(&sgout[0], data, template[i].ilen);
+ }

- ablkcipher_request_set_crypt(req, sg, sg,
+ ablkcipher_request_set_crypt(req, sg,
+ (diff_dst) ? sgout : sg,
template[i].ilen, iv);
ret = enc ?
crypto_ablkcipher_encrypt(req) :
@@ -850,16 +866,15 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
}
/* fall through */
default:
- printk(KERN_ERR "alg: skcipher: %s failed on "
- "test %d for %s: ret=%d\n", e, j, algo,
- -ret);
+ pr_err("alg: skcipher%s: %s failed on test %d for %s: ret=%d\n",
+ d, e, j, algo, -ret);
goto out;
}

q = data;
if (memcmp(q, template[i].result, template[i].rlen)) {
- printk(KERN_ERR "alg: skcipher: Test %d "
- "failed on %s for %s\n", j, e, algo);
+ pr_err("alg: skcipher%s: Test %d failed on %s for %s\n",
+ d, j, e, algo);
hexdump(q, template[i].rlen);
ret = -EINVAL;
goto out;
@@ -886,9 +901,8 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
ret = crypto_ablkcipher_setkey(tfm, template[i].key,
template[i].klen);
if (!ret == template[i].fail) {
- printk(KERN_ERR "alg: skcipher: setkey failed "
- "on chunk test %d for %s: flags=%x\n",
- j, algo,
+ pr_err("alg: skcipher%s: setkey failed on chunk test %d for %s: flags=%x\n",
+ d, j, algo,
crypto_ablkcipher_get_flags(tfm));
goto out;
} else if (ret)
@@ -897,6 +911,8 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
temp = 0;
ret = -EINVAL;
sg_init_table(sg, template[i].np);
+ if (diff_dst)
+ sg_init_table(sgout, template[i].np);
for (k = 0; k < template[i].np; k++) {
if (WARN_ON(offset_in_page(IDX[k]) +
template[i].tap[k] > PAGE_SIZE))
@@ -913,11 +929,24 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
q[template[i].tap[k]] = 0;

sg_set_buf(&sg[k], q, template[i].tap[k]);
+ if (diff_dst) {
+ q = xoutbuf[IDX[k] >> PAGE_SHIFT] +
+ offset_in_page(IDX[k]);
+
+ sg_set_buf(&sgout[k], q,
+ template[i].tap[k]);
+
+ memset(q, 0, template[i].tap[k]);
+ if (offset_in_page(q) +
+ template[i].tap[k] < PAGE_SIZE)
+ q[template[i].tap[k]] = 0;
+ }

temp += template[i].tap[k];
}

- ablkcipher_request_set_crypt(req, sg, sg,
+ ablkcipher_request_set_crypt(req, sg,
+ (diff_dst) ? sgout : sg,
template[i].ilen, iv);

ret = enc ?
@@ -937,23 +966,25 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
}
/* fall through */
default:
- printk(KERN_ERR "alg: skcipher: %s failed on "
- "chunk test %d for %s: ret=%d\n", e, j,
- algo, -ret);
+ pr_err("alg: skcipher%s: %s failed on chunk test %d for %s: ret=%d\n",
+ d, e, j, algo, -ret);
goto out;
}

temp = 0;
ret = -EINVAL;
for (k = 0; k < template[i].np; k++) {
- q = xbuf[IDX[k] >> PAGE_SHIFT] +
- offset_in_page(IDX[k]);
+ if (diff_dst)
+ q = xoutbuf[IDX[k] >> PAGE_SHIFT] +
+ offset_in_page(IDX[k]);
+ else
+ q = xbuf[IDX[k] >> PAGE_SHIFT] +
+ offset_in_page(IDX[k]);

if (memcmp(q, template[i].result + temp,
template[i].tap[k])) {
- printk(KERN_ERR "alg: skcipher: Chunk "
- "test %d failed on %s at page "
- "%u for %s\n", j, e, k, algo);
+ pr_err("alg: skcipher%s: Chunk test %d failed on %s at page %u for %s\n",
+ d, j, e, k, algo);
hexdump(q, template[i].tap[k]);
goto out;
}
@@ -962,11 +993,8 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
for (n = 0; offset_in_page(q + n) && q[n]; n++)
;
if (n) {
- printk(KERN_ERR "alg: skcipher: "
- "Result buffer corruption in "
- "chunk test %d on %s at page "
- "%u for %s: %u bytes:\n", j, e,
- k, algo, n);
+ pr_err("alg: skcipher%s: Result buffer corruption in chunk test %d on %s at page %u for %s: %u bytes:\n",
+ d, j, e, k, algo, n);
hexdump(q, n);
goto out;
}
@@ -979,11 +1007,28 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,

out:
ablkcipher_request_free(req);
+ if (diff_dst)
+ testmgr_free_buf(xoutbuf);
+out_nooutbuf:
testmgr_free_buf(xbuf);
out_nobuf:
return ret;
}

+static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
+ struct cipher_testvec *template, unsigned int tcount)
+{
+ int ret;
+
+ /* test 'dst == src' case */
+ ret = __test_skcipher(tfm, enc, template, tcount, false);
+ if (ret)
+ return ret;
+
+ /* test 'dst != src' case */
+ return __test_skcipher(tfm, enc, template, tcount, true);
+}
+
static int test_comp(struct crypto_comp *tfm, struct comp_testvec *ctemplate,
struct comp_testvec *dtemplate, int ctcount, int dtcount)
{


2012-09-21 07:26:55

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 2/2] crypto: testmgr - make test_aead also test 'dst != src' code paths

Currrently test_aead uses same buffer for destination and source. However
in any places, 'dst != src' take different path than 'dst == src' case.

Therefore make test_aead also run tests with destination buffer being
different than source buffer.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
crypto/testmgr.c | 153 +++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 105 insertions(+), 48 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 00f54d5..941d75c 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -358,8 +358,9 @@ out_nobuf:
return ret;
}

-static int test_aead(struct crypto_aead *tfm, int enc,
- struct aead_testvec *template, unsigned int tcount)
+static int __test_aead(struct crypto_aead *tfm, int enc,
+ struct aead_testvec *template, unsigned int tcount,
+ const bool diff_dst)
{
const char *algo = crypto_tfm_alg_driver_name(crypto_aead_tfm(tfm));
unsigned int i, j, k, n, temp;
@@ -367,15 +368,18 @@ static int test_aead(struct crypto_aead *tfm, int enc,
char *q;
char *key;
struct aead_request *req;
- struct scatterlist sg[8];
- struct scatterlist asg[8];
- const char *e;
+ struct scatterlist *sg;
+ struct scatterlist *asg;
+ struct scatterlist *sgout;
+ const char *e, *d;
struct tcrypt_result result;
unsigned int authsize;
void *input;
+ void *output;
void *assoc;
char iv[MAX_IVLEN];
char *xbuf[XBUFSIZE];
+ char *xoutbuf[XBUFSIZE];
char *axbuf[XBUFSIZE];

if (testmgr_alloc_buf(xbuf))
@@ -383,6 +387,21 @@ static int test_aead(struct crypto_aead *tfm, int enc,
if (testmgr_alloc_buf(axbuf))
goto out_noaxbuf;

+ if (diff_dst && testmgr_alloc_buf(xoutbuf))
+ goto out_nooutbuf;
+
+ /* avoid "the frame size is larger than 1024 bytes" compiler warning */
+ sg = kmalloc(sizeof(*sg) * 8 * (diff_dst ? 3 : 2), GFP_KERNEL);
+ if (!sg)
+ goto out_nosg;
+ asg = &sg[8];
+ sgout = &asg[8];
+
+ if (diff_dst)
+ d = "-ddst";
+ else
+ d = "";
+
if (enc == ENCRYPT)
e = "encryption";
else
@@ -392,8 +411,8 @@ static int test_aead(struct crypto_aead *tfm, int enc,

req = aead_request_alloc(tfm, GFP_KERNEL);
if (!req) {
- printk(KERN_ERR "alg: aead: Failed to allocate request for "
- "%s\n", algo);
+ pr_err("alg: aead%s: Failed to allocate request for %s\n",
+ d, algo);
goto out;
}

@@ -432,9 +451,8 @@ static int test_aead(struct crypto_aead *tfm, int enc,
ret = crypto_aead_setkey(tfm, key,
template[i].klen);
if (!ret == template[i].fail) {
- printk(KERN_ERR "alg: aead: setkey failed on "
- "test %d for %s: flags=%x\n", j, algo,
- crypto_aead_get_flags(tfm));
+ pr_err("alg: aead%s: setkey failed on test %d for %s: flags=%x\n",
+ d, j, algo, crypto_aead_get_flags(tfm));
goto out;
} else if (ret)
continue;
@@ -442,18 +460,26 @@ static int test_aead(struct crypto_aead *tfm, int enc,
authsize = abs(template[i].rlen - template[i].ilen);
ret = crypto_aead_setauthsize(tfm, authsize);
if (ret) {
- printk(KERN_ERR "alg: aead: Failed to set "
- "authsize to %u on test %d for %s\n",
- authsize, j, algo);
+ pr_err("alg: aead%s: Failed to set authsize to %u on test %d for %s\n",
+ d, authsize, j, algo);
goto out;
}

sg_init_one(&sg[0], input,
template[i].ilen + (enc ? authsize : 0));

+ if (diff_dst) {
+ output = xoutbuf[0];
+ sg_init_one(&sgout[0], output,
+ template[i].ilen +
+ (enc ? authsize : 0));
+ } else {
+ output = input;
+ }
+
sg_init_one(&asg[0], assoc, template[i].alen);

- aead_request_set_crypt(req, sg, sg,
+ aead_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
template[i].ilen, iv);

aead_request_set_assoc(req, asg, template[i].alen);
@@ -466,10 +492,8 @@ static int test_aead(struct crypto_aead *tfm, int enc,
case 0:
if (template[i].novrfy) {
/* verification was supposed to fail */
- printk(KERN_ERR "alg: aead: %s failed "
- "on test %d for %s: ret was 0, "
- "expected -EBADMSG\n",
- e, j, algo);
+ pr_err("alg: aead%s: %s failed on test %d for %s: ret was 0, expected -EBADMSG\n",
+ d, e, j, algo);
/* so really, we got a bad message */
ret = -EBADMSG;
goto out;
@@ -489,15 +513,15 @@ static int test_aead(struct crypto_aead *tfm, int enc,
continue;
/* fall through */
default:
- printk(KERN_ERR "alg: aead: %s failed on test "
- "%d for %s: ret=%d\n", e, j, algo, -ret);
+ pr_err("alg: aead%s: %s failed on test %d for %s: ret=%d\n",
+ d, e, j, algo, -ret);
goto out;
}

- q = input;
+ q = output;
if (memcmp(q, template[i].result, template[i].rlen)) {
- printk(KERN_ERR "alg: aead: Test %d failed on "
- "%s for %s\n", j, e, algo);
+ pr_err("alg: aead%s: Test %d failed on %s for %s\n",
+ d, j, e, algo);
hexdump(q, template[i].rlen);
ret = -EINVAL;
goto out;
@@ -522,9 +546,8 @@ static int test_aead(struct crypto_aead *tfm, int enc,

ret = crypto_aead_setkey(tfm, key, template[i].klen);
if (!ret == template[i].fail) {
- printk(KERN_ERR "alg: aead: setkey failed on "
- "chunk test %d for %s: flags=%x\n", j,
- algo, crypto_aead_get_flags(tfm));
+ pr_err("alg: aead%s: setkey failed on chunk test %d for %s: flags=%x\n",
+ d, j, algo, crypto_aead_get_flags(tfm));
goto out;
} else if (ret)
continue;
@@ -533,6 +556,8 @@ static int test_aead(struct crypto_aead *tfm, int enc,

ret = -EINVAL;
sg_init_table(sg, template[i].np);
+ if (diff_dst)
+ sg_init_table(sgout, template[i].np);
for (k = 0, temp = 0; k < template[i].np; k++) {
if (WARN_ON(offset_in_page(IDX[k]) +
template[i].tap[k] > PAGE_SIZE))
@@ -551,14 +576,26 @@ static int test_aead(struct crypto_aead *tfm, int enc,
q[n] = 0;

sg_set_buf(&sg[k], q, template[i].tap[k]);
+
+ if (diff_dst) {
+ q = xoutbuf[IDX[k] >> PAGE_SHIFT] +
+ offset_in_page(IDX[k]);
+
+ memset(q, 0, template[i].tap[k]);
+ if (offset_in_page(q) + n < PAGE_SIZE)
+ q[n] = 0;
+
+ sg_set_buf(&sgout[k], q,
+ template[i].tap[k]);
+ }
+
temp += template[i].tap[k];
}

ret = crypto_aead_setauthsize(tfm, authsize);
if (ret) {
- printk(KERN_ERR "alg: aead: Failed to set "
- "authsize to %u on chunk test %d for "
- "%s\n", authsize, j, algo);
+ pr_err("alg: aead%s: Failed to set authsize to %u on chunk test %d for %s\n",
+ d, authsize, j, algo);
goto out;
}

@@ -571,6 +608,9 @@ static int test_aead(struct crypto_aead *tfm, int enc,
}

sg[k - 1].length += authsize;
+
+ if (diff_dst)
+ sgout[k - 1].length += authsize;
}

sg_init_table(asg, template[i].anp);
@@ -588,7 +628,7 @@ static int test_aead(struct crypto_aead *tfm, int enc,
temp += template[i].atap[k];
}

- aead_request_set_crypt(req, sg, sg,
+ aead_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
template[i].ilen,
iv);

@@ -602,10 +642,8 @@ static int test_aead(struct crypto_aead *tfm, int enc,
case 0:
if (template[i].novrfy) {
/* verification was supposed to fail */
- printk(KERN_ERR "alg: aead: %s failed "
- "on chunk test %d for %s: ret "
- "was 0, expected -EBADMSG\n",
- e, j, algo);
+ pr_err("alg: aead%s: %s failed on chunk test %d for %s: ret was 0, expected -EBADMSG\n",
+ d, e, j, algo);
/* so really, we got a bad message */
ret = -EBADMSG;
goto out;
@@ -625,32 +663,35 @@ static int test_aead(struct crypto_aead *tfm, int enc,
continue;
/* fall through */
default:
- printk(KERN_ERR "alg: aead: %s failed on "
- "chunk test %d for %s: ret=%d\n", e, j,
- algo, -ret);
+ pr_err("alg: aead%s: %s failed on chunk test %d for %s: ret=%d\n",
+ d, e, j, algo, -ret);
goto out;
}

ret = -EINVAL;
for (k = 0, temp = 0; k < template[i].np; k++) {
- q = xbuf[IDX[k] >> PAGE_SHIFT] +
- offset_in_page(IDX[k]);
+ if (diff_dst)
+ q = xoutbuf[IDX[k] >> PAGE_SHIFT] +
+ offset_in_page(IDX[k]);
+ else
+ q = xbuf[IDX[k] >> PAGE_SHIFT] +
+ offset_in_page(IDX[k]);

n = template[i].tap[k];
if (k == template[i].np - 1)
n += enc ? authsize : -authsize;

if (memcmp(q, template[i].result + temp, n)) {
- printk(KERN_ERR "alg: aead: Chunk "
- "test %d failed on %s at page "
- "%u for %s\n", j, e, k, algo);
+ pr_err("alg: aead%s: Chunk test %d failed on %s at page %u for %s\n",
+ d, j, e, k, algo);
hexdump(q, n);
goto out;
}

q += n;
if (k == template[i].np - 1 && !enc) {
- if (memcmp(q, template[i].input +
+ if (!diff_dst &&
+ memcmp(q, template[i].input +
temp + n, authsize))
n = authsize;
else
@@ -661,11 +702,8 @@ static int test_aead(struct crypto_aead *tfm, int enc,
;
}
if (n) {
- printk(KERN_ERR "alg: aead: Result "
- "buffer corruption in chunk "
- "test %d on %s at page %u for "
- "%s: %u bytes:\n", j, e, k,
- algo, n);
+ pr_err("alg: aead%s: Result buffer corruption in chunk test %d on %s at page %u for %s: %u bytes:\n",
+ d, j, e, k, algo, n);
hexdump(q, n);
goto out;
}
@@ -679,6 +717,11 @@ static int test_aead(struct crypto_aead *tfm, int enc,

out:
aead_request_free(req);
+ kfree(sg);
+out_nosg:
+ if (diff_dst)
+ testmgr_free_buf(xoutbuf);
+out_nooutbuf:
testmgr_free_buf(axbuf);
out_noaxbuf:
testmgr_free_buf(xbuf);
@@ -686,6 +729,20 @@ out_noxbuf:
return ret;
}

+static int test_aead(struct crypto_aead *tfm, int enc,
+ struct aead_testvec *template, unsigned int tcount)
+{
+ int ret;
+
+ /* test 'dst == src' case */
+ ret = __test_aead(tfm, enc, template, tcount, false);
+ if (ret)
+ return ret;
+
+ /* test 'dst != src' case */
+ return __test_aead(tfm, enc, template, tcount, true);
+}
+
static int test_cipher(struct crypto_cipher *tfm, int enc,
struct cipher_testvec *template, unsigned int tcount)
{

2012-09-21 16:11:52

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: testmgr - make test_skcipher also test 'dst != src' code paths

From: Jussi Kivilinna <[email protected]>
Date: Fri, 21 Sep 2012 10:26:47 +0300

> Currrently test_skcipher uses same buffer for destination and source. However
> in any places, 'dst != src' take different path than 'dst == src' case.
>
> Therefore make test_skcipher also run tests with destination buffer being
> different than source buffer.
>
> Signed-off-by: Jussi Kivilinna <[email protected]>

Acked-by: David S. Miller <[email protected]>

2012-09-21 16:12:00

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 2/2] crypto: testmgr - make test_aead also test 'dst != src' code paths

From: Jussi Kivilinna <[email protected]>
Date: Fri, 21 Sep 2012 10:26:52 +0300

> Currrently test_aead uses same buffer for destination and source. However
> in any places, 'dst != src' take different path than 'dst == src' case.
>
> Therefore make test_aead also run tests with destination buffer being
> different than source buffer.
>
> Signed-off-by: Jussi Kivilinna <[email protected]>

Acked-by: David S. Miller <[email protected]>

2012-09-27 05:45:23

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 2/2] crypto: testmgr - make test_aead also test 'dst != src' code paths

On Fri, Sep 21, 2012 at 12:11:58PM -0400, David Miller wrote:
> From: Jussi Kivilinna <[email protected]>
> Date: Fri, 21 Sep 2012 10:26:52 +0300
>
> > Currrently test_aead uses same buffer for destination and source. However
> > in any places, 'dst != src' take different path than 'dst == src' case.
> >
> > Therefore make test_aead also run tests with destination buffer being
> > different than source buffer.
> >
> > Signed-off-by: Jussi Kivilinna <[email protected]>
>
> Acked-by: David S. Miller <[email protected]>

Both patches applied. Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2013-11-12 11:12:07

by Horia Geantă

[permalink] [raw]
Subject: Re: [PATCH 2/2] crypto: testmgr - make test_aead also test 'dst != src' code paths

On 9/21/2012 10:26 AM, Jussi Kivilinna wrote:
> Currrently test_aead uses same buffer for destination and source. However
> in any places, 'dst != src' take different path than 'dst == src' case.
>
> Therefore make test_aead also run tests with destination buffer being
> different than source buffer.
>
> Signed-off-by: Jussi Kivilinna <[email protected]>
> ---
> crypto/testmgr.c | 153 +++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 105 insertions(+), 48 deletions(-)
>
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index 00f54d5..941d75c 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c

> @@ -442,18 +460,26 @@ static int test_aead(struct crypto_aead *tfm, int enc,
> authsize = abs(template[i].rlen - template[i].ilen);
> ret = crypto_aead_setauthsize(tfm, authsize);
> if (ret) {
> - printk(KERN_ERR "alg: aead: Failed to set "
> - "authsize to %u on test %d for %s\n",
> - authsize, j, algo);
> + pr_err("alg: aead%s: Failed to set authsize to %u on test %d for %s\n",
> + d, authsize, j, algo);
> goto out;
> }
>
> sg_init_one(&sg[0], input,
> template[i].ilen + (enc ? authsize : 0));
>
> + if (diff_dst) {
> + output = xoutbuf[0];
> + sg_init_one(&sgout[0], output,
> + template[i].ilen +
> + (enc ? authsize : 0));
> + } else {
> + output = input;
> + }

In case of diff_dst (src != dst), is there any assumption / convention
regarding allocation length of req->src and req->dst - are they supposed
be equal, even if it's not needed?
For example, in case of diff_dst && encryption, currently both req->src
and req->dst have then length = template[i].ilen + authsize. Shouldn't
length of req->src be template[i].ilen, i.e. no space allocated for ICV?

Thanks,
Horia

2013-11-12 21:54:22

by Jussi Kivilinna

[permalink] [raw]
Subject: Re: [PATCH 2/2] crypto: testmgr - make test_aead also test 'dst != src' code paths

On 12.11.2013 13:11, Horia Geantă wrote:
> On 9/21/2012 10:26 AM, Jussi Kivilinna wrote:
>> Currrently test_aead uses same buffer for destination and source. However
>> in any places, 'dst != src' take different path than 'dst == src' case.
>>
>> Therefore make test_aead also run tests with destination buffer being
>> different than source buffer.
>>
>> Signed-off-by: Jussi Kivilinna <[email protected]>
>> ---
>> crypto/testmgr.c | 153 +++++++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 105 insertions(+), 48 deletions(-)
>>
>> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
>> index 00f54d5..941d75c 100644
>> --- a/crypto/testmgr.c
>> +++ b/crypto/testmgr.c
>
>> @@ -442,18 +460,26 @@ static int test_aead(struct crypto_aead *tfm, int enc,
>> authsize = abs(template[i].rlen - template[i].ilen);
>> ret = crypto_aead_setauthsize(tfm, authsize);
>> if (ret) {
>> - printk(KERN_ERR "alg: aead: Failed to set "
>> - "authsize to %u on test %d for %s\n",
>> - authsize, j, algo);
>> + pr_err("alg: aead%s: Failed to set authsize to %u on test %d for %s\n",
>> + d, authsize, j, algo);
>> goto out;
>> }
>> sg_init_one(&sg[0], input,
>> template[i].ilen + (enc ? authsize : 0));
>> + if (diff_dst) {
>> + output = xoutbuf[0];
>> + sg_init_one(&sgout[0], output,
>> + template[i].ilen +
>> + (enc ? authsize : 0));
>> + } else {
>> + output = input;
>> + }
>
> In case of diff_dst (src != dst), is there any assumption / convention regarding allocation length of req->src and req->dst - are they supposed be equal, even if it's not needed?
> For example, in case of diff_dst && encryption, currently both req->src and req->dst have then length = template[i].ilen + authsize. Shouldn't length of req->src be template[i].ilen, i.e. no space allocated for ICV?

You're right, in diff_dst case, sg and sgout could be different size. That's what I thought at first, and so I changed the above part to:

if (diff_dst) {
output = xoutbuf[0];
output += align_offset;
sg_init_one(&sg[0], input, template[i].ilen);
sg_init_one(&sgout[0], output,
template[i].rlen);
} else {
sg_init_one(&sg[0], input,
template[i].ilen +
(enc ? authsize : 0));
output = input;
}

But this causes scatterwalk.c to throw BUG:

[ 62.213960] ------------[ cut here ]------------
[ 62.214031] kernel BUG at crypto/scatterwalk.c:37!
[ 62.214065] invalid opcode: 0000 [#1] PREEMPT SMP
[ 62.214101] Modules linked in: zlib seed rmd320 rmd256 rmd128 cts ccm ghash_generic gcm salsa20_generic salsa20_x86_64 fcrypt pcbc tgr192 anubis wp512 khazad tea michael_mic arc4 cast6_generic seqiv md4 ecb tcrypt(+) deflate ctr twofish_generic twofish_x86_64_3way twofish_x86_64 twofish_common camellia_generic camellia_x86_64 serpent_sse2_x86_64 glue_helper lrw serpent_generic xts gf128mul blowfish_generic blowfish_x86_64 blowfish_common ablk_helper cryptd cast5_generic cast_common des_generic cbc cmac xcbc rmd160 sha512_ssse3 sha512_generic sha256_ssse3 sha256_generic sha1_ssse3 sha1_generic md5 hmac crypto_null af_key xfrm_algo dm_crypt dm_mod ppdev parport_pc ac ohci_pci ohci_hcd i2c_piix4 lp parport nfsd nfs_acl rpcsec_gss_krb5 auth_rpcgss oid_registry nfsv4 nfs lockd sunrpc btrfs raid6_pq xor libcrc32c floppy intel_agp intel_gtt agpgart button ata_piix e1000
[ 62.214603] CPU: 1 PID: 2131 Comm: cryptomgr_test Not tainted 3.11.0-cryptotest-jk1+ #59
[ 62.214661] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 62.214720] task: ffff880037559c80 ti: ffff88003cb0c000 task.ti: ffff88003cb0c000
[ 62.214776] RIP: 0010:[<ffffffff8122c730>] [<ffffffff8122c730>] scatterwalk_bytes_sglen+0x70/0x70
[ 62.214847] RSP: 0018:ffff88003cb0dae0 EFLAGS: 00010246
[ 62.214880] RAX: 00000000000000f2 RBX: ffff8800376964b0 RCX: 0000000000000000
[ 62.214916] RDX: ffff880037694800 RSI: ffff880037694800 RDI: ffff88003cb0db18
[ 62.214952] RBP: ffff880037696c00 R08: 00000000d0597ff0 R09: 0000000000000000
[ 62.214989] R10: 000000006b5d5311 R11: 00000000d1bd7079 R12: ffff880037696c00
[ 62.215389] R13: 0000000000000000 R14: 0000000000002000 R15: 0000000000000000
[ 62.215730] FS: 0000000000000000(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000
[ 62.216411] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 62.216659] CR2: 00007f8d50b79420 CR3: 000000003c8b2000 CR4: 00000000000006e0
[ 62.217019] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 62.217247] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 62.217247] Stack:
[ 62.217247] ffffffff8122c758 ffffffffa0490418 ffff88003ca2a01e 000000003db65000
[ 62.217247] ffff88003cb0dfd8 ffff88003cb0dfd8 ffff8800376964c0 ffff880037694800
[ 62.217247] 000000d000000020 ffff8800376964b0 ffff880037696458 ffff880037696c00
[ 62.217247] Call Trace:
[ 62.217247] [<ffffffff8122c758>] ? scatterwalk_start+0x18/0x20
[ 62.217247] [<ffffffffa0490418>] ? get_data_to_compute+0x28/0x2b0 [ccm]
[ 62.217247] [<ffffffffa04907c7>] ? crypto_ccm_auth+0x127/0x190 [ccm]
[ 62.217247] [<ffffffffa049119a>] ? crypto_ccm_encrypt+0x6a/0x275 [ccm]
[ 62.217247] [<ffffffff8126c3b4>] ? sg_init_table+0x14/0x30
[ 62.217247] [<ffffffff81233f46>] ? __test_aead+0x3f6/0xf60
[ 62.217247] [<ffffffff8122c4c8>] ? crypto_spawn_tfm+0x38/0x70
[ 62.217247] [<ffffffff81131717>] ? __kmalloc+0x1d7/0x200
[ 62.217247] [<ffffffff8122a5c9>] ? __crypto_alloc_tfm+0xe9/0x160
[ 62.217247] [<ffffffff81234af9>] ? test_aead+0x49/0xa0
[ 62.217247] [<ffffffff81234b8a>] ? alg_test_aead+0x3a/0x90
[ 62.217247] [<ffffffff81232e03>] ? alg_test+0xc3/0x290
[ 62.217247] [<ffffffff8106f130>] ? finish_task_switch+0x40/0xc0
[ 62.217247] [<ffffffff81536947>] ? __schedule+0x287/0x760
[ 62.217247] [<ffffffff81231b30>] ? crypto_unregister_pcomp+0x10/0x10
[ 62.217247] [<ffffffff81231b68>] ? cryptomgr_test+0x38/0x40
[ 62.217247] [<ffffffff8106590f>] ? kthread+0xaf/0xc0
[ 62.217247] [<ffffffff81065860>] ? kthread_create_on_node+0x120/0x120
[ 62.217247] [<ffffffff8153902c>] ? ret_from_fork+0x7c/0xb0
[ 62.217247] [<ffffffff81065860>] ? kthread_create_on_node+0x120/0x120
[ 62.217247] Code: ff ff ff ff 0f 1f 80 00 00 00 00 f3 c3 66 0f 1f 44 00 00 48 8b 7f 20 48 83 e7 fc 41 0f 94 c0 eb bb 66 2e 0f 1f 84 00 00 00 00 00 <0f> 0b 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 48 89 37 44 8b
[ 62.217247] RIP [<ffffffff8122c730>] scatterwalk_bytes_sglen+0x70/0x70
[ 62.217247] RSP <ffff88003cb0dae0>
[ 62.227458] ---[ end trace 338f2122cbee98de ]---
[ 63.411157] ------------[ cut here ]------------


This should probably be fixed, right?

-Jussi

>
> Thanks,
> Horia
>
>
>
>

2013-11-28 13:11:15

by Horia Geantă

[permalink] [raw]
Subject: [PATCH 1/4] crypto: ccm - Fix handling of zero plaintext when computing mac

There are cases when cryptlen can be zero in crypto_ccm_auth():
-encryptiom: input scatterlist length is zero (no plaintext)
-decryption: input scatterlist contains only the mac
plus the condition of having different source and destination buffers
(or else scatterlist length = max(plaintext_len, ciphertext_len)).

These are not handled correctly, leading to crashes like:

root@p4080ds:~/crypto# insmod tcrypt.ko mode=45
------------[ cut here ]------------
kernel BUG at crypto/scatterwalk.c:37!
Oops: Exception in kernel mode, sig: 5 [#1]
SMP NR_CPUS=8 P4080 DS
Modules linked in: tcrypt(+) crc32c xts xcbc vmac pcbc ecb gcm ghash_generic gf128mul ccm ctr seqiv
CPU: 3 PID: 1082 Comm: cryptomgr_test Not tainted 3.11.0 #14
task: ee12c5b0 ti: eecd0000 task.ti: eecd0000
NIP: c0204d98 LR: f9225848 CTR: c0204d80
REGS: eecd1b70 TRAP: 0700 Not tainted (3.11.0)
MSR: 00029002 <CE,EE,ME> CR: 22044022 XER: 20000000

GPR00: f9225c94 eecd1c20 ee12c5b0 eecd1c28 ee879400 ee879400 00000000 ee607464
GPR08: 00000001 00000001 00000000 006b0000 c0204d80 00000000 00000002 c0698e20
GPR16: ee987000 ee895000 fffffff4 ee879500 00000100 eecd1d58 00000001 00000000
GPR24: ee879400 00000020 00000000 00000000 ee5b2800 ee607430 00000004 ee607460
NIP [c0204d98] scatterwalk_start+0x18/0x30
LR [f9225848] get_data_to_compute+0x28/0x2f0 [ccm]
Call Trace:
[eecd1c20] [f9225974] get_data_to_compute+0x154/0x2f0 [ccm] (unreliable)
[eecd1c70] [f9225c94] crypto_ccm_auth+0x184/0x1d0 [ccm]
[eecd1cb0] [f9225d40] crypto_ccm_encrypt+0x60/0x2d0 [ccm]
[eecd1cf0] [c020d77c] __test_aead+0x3ec/0xe20
[eecd1e20] [c020f35c] test_aead+0x6c/0xe0
[eecd1e40] [c020f420] alg_test_aead+0x50/0xd0
[eecd1e60] [c020e5e4] alg_test+0x114/0x2e0
[eecd1ee0] [c020bd1c] cryptomgr_test+0x4c/0x60
[eecd1ef0] [c0047058] kthread+0xa8/0xb0
[eecd1f40] [c000eb0c] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
0f080000 81290024 552807fe 0f080000 5529003a 4bffffb4 90830000 39400000
39000001 8124000c 2f890000 7d28579e <0f090000> 81240008 91230004 4e800020
---[ end trace 6d652dfcd1be37bd ]---

Cc: <[email protected]>
Cc: Jussi Kivilinna <[email protected]>
Signed-off-by: Horia Geanta <[email protected]>
---
crypto/ccm.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/crypto/ccm.c b/crypto/ccm.c
index 3e05499..1df8421 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -271,7 +271,8 @@ static int crypto_ccm_auth(struct aead_request *req, struct scatterlist *plain,
}

/* compute plaintext into mac */
- get_data_to_compute(cipher, pctx, plain, cryptlen);
+ if (cryptlen)
+ get_data_to_compute(cipher, pctx, plain, cryptlen);

out:
return err;
--
1.7.7.6

2013-11-28 13:17:46

by Horia Geantă

[permalink] [raw]
Subject: [PATCH 2/4] crypto: caam - fix aead sglen for case 'dst != src'

For aead case when source and destination buffers are different,
there is an incorrect assumption that the source length includes the ICV
length. Fix this, since it leads to an oops when using sg_count() to
find the number of nents in the scatterlist:

Unable to handle kernel paging request for data at address 0x00000004
Faulting instruction address: 0xf91f7634
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=8 P4080 DS
Modules linked in: caamalg(+) caam_jr caam
CPU: 1 PID: 1053 Comm: cryptomgr_test Not tainted 3.11.0 #16
task: eeb24ab0 ti: eeafa000 task.ti: eeafa000
NIP: f91f7634 LR: f91f7f24 CTR: f91f7ef0
REGS: eeafbbc0 TRAP: 0300 Not tainted (3.11.0)
MSR: 00029002 <CE,EE,ME> CR: 44044044 XER: 00000000
DEAR: 00000004, ESR: 00000000

GPR00: f91f7f24 eeafbc70 eeb24ab0 00000002 ee8e0900 ee8e0800 00000024 c45c4462
GPR08: 00000010 00000000 00000014 0c0e4000 24044044 00000000 00000000 c0691590
GPR16: eeab0000 eeb23000 00000000 00000000 00000000 00000001 00000001 eeafbcc8
GPR24: 000000d1 00000010 ee2d5000 ee49ea10 ee49ea10 ee46f640 ee46f640 c0691590
NIP [f91f7634] aead_edesc_alloc.constprop.14+0x144/0x780 [caamalg]
LR [f91f7f24] aead_encrypt+0x34/0x288 [caamalg]
Call Trace:
[eeafbc70] [a1004000] 0xa1004000 (unreliable)
[eeafbcc0] [f91f7f24] aead_encrypt+0x34/0x288 [caamalg]
[eeafbcf0] [c020d77c] __test_aead+0x3ec/0xe20
[eeafbe20] [c020f35c] test_aead+0x6c/0xe0
[eeafbe40] [c020f420] alg_test_aead+0x50/0xd0
[eeafbe60] [c020e5e4] alg_test+0x114/0x2e0
[eeafbee0] [c020bd1c] cryptomgr_test+0x4c/0x60
[eeafbef0] [c0047058] kthread+0xa8/0xb0
[eeafbf40] [c000eb0c] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
69084321 7d080034 5508d97e 69080001 0f080000 81290024 552807fe 0f080000
3a600001 5529003a 2f8a0000 40dd0028 <80e90004> 3ab50001 8109000c 70e30002
---[ end trace b3c3e23925c7484e ]---

While here, add a tcrypt mode for making it easy to test authenc
(needed for triggering case above).

Signed-off-by: Horia Geanta <[email protected]>
---
crypto/tcrypt.c | 4 +++
drivers/crypto/caam/caamalg.c | 51 +++++++++++++++++++++++-----------------
2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 25a5934..cc887ae 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1242,6 +1242,10 @@ static int do_test(int m)
ret += tcrypt_test("cmac(des3_ede)");
break;

+ case 155:
+ ret += tcrypt_test("authenc(hmac(sha1),cbc(aes))");
+ break;
+
case 200:
test_cipher_speed("ecb(aes)", ENCRYPT, sec, NULL, 0,
speed_template_16_24_32);
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 4f44b71..4cf5dec 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -818,7 +818,7 @@ static void aead_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
ivsize, 1);
print_hex_dump(KERN_ERR, "dst @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->dst),
- req->cryptlen, 1);
+ req->cryptlen - ctx->authsize, 1);
#endif

if (err) {
@@ -972,12 +972,9 @@ static void init_aead_job(u32 *sh_desc, dma_addr_t ptr,
(edesc->src_nents ? : 1);
in_options = LDST_SGF;
}
- if (encrypt)
- append_seq_in_ptr(desc, src_dma, req->assoclen + ivsize +
- req->cryptlen - authsize, in_options);
- else
- append_seq_in_ptr(desc, src_dma, req->assoclen + ivsize +
- req->cryptlen, in_options);
+
+ append_seq_in_ptr(desc, src_dma, req->assoclen + ivsize + req->cryptlen,
+ in_options);

if (likely(req->src == req->dst)) {
if (all_contig) {
@@ -998,7 +995,8 @@ static void init_aead_job(u32 *sh_desc, dma_addr_t ptr,
}
}
if (encrypt)
- append_seq_out_ptr(desc, dst_dma, req->cryptlen, out_options);
+ append_seq_out_ptr(desc, dst_dma, req->cryptlen + authsize,
+ out_options);
else
append_seq_out_ptr(desc, dst_dma, req->cryptlen - authsize,
out_options);
@@ -1048,8 +1046,8 @@ static void init_aead_giv_job(u32 *sh_desc, dma_addr_t ptr,
sec4_sg_index += edesc->assoc_nents + 1 + edesc->src_nents;
in_options = LDST_SGF;
}
- append_seq_in_ptr(desc, src_dma, req->assoclen + ivsize +
- req->cryptlen - authsize, in_options);
+ append_seq_in_ptr(desc, src_dma, req->assoclen + ivsize + req->cryptlen,
+ in_options);

if (contig & GIV_DST_CONTIG) {
dst_dma = edesc->iv_dma;
@@ -1066,7 +1064,8 @@ static void init_aead_giv_job(u32 *sh_desc, dma_addr_t ptr,
}
}

- append_seq_out_ptr(desc, dst_dma, ivsize + req->cryptlen, out_options);
+ append_seq_out_ptr(desc, dst_dma, ivsize + req->cryptlen + authsize,
+ out_options);
}

/*
@@ -1130,7 +1129,8 @@ static void init_ablkcipher_job(u32 *sh_desc, dma_addr_t ptr,
* allocate and map the aead extended descriptor
*/
static struct aead_edesc *aead_edesc_alloc(struct aead_request *req,
- int desc_bytes, bool *all_contig_ptr)
+ int desc_bytes, bool *all_contig_ptr,
+ bool encrypt)
{
struct crypto_aead *aead = crypto_aead_reqtfm(req);
struct caam_ctx *ctx = crypto_aead_ctx(aead);
@@ -1145,12 +1145,22 @@ static struct aead_edesc *aead_edesc_alloc(struct aead_request *req,
bool assoc_chained = false, src_chained = false, dst_chained = false;
int ivsize = crypto_aead_ivsize(aead);
int sec4_sg_index, sec4_sg_len = 0, sec4_sg_bytes;
+ unsigned int authsize = ctx->authsize;

assoc_nents = sg_count(req->assoc, req->assoclen, &assoc_chained);
- src_nents = sg_count(req->src, req->cryptlen, &src_chained);

- if (unlikely(req->dst != req->src))
- dst_nents = sg_count(req->dst, req->cryptlen, &dst_chained);
+ if (unlikely(req->dst != req->src)) {
+ src_nents = sg_count(req->src, req->cryptlen, &src_chained);
+ dst_nents = sg_count(req->dst,
+ req->cryptlen +
+ (encrypt ? authsize : (-authsize)),
+ &dst_chained);
+ } else {
+ src_nents = sg_count(req->src,
+ req->cryptlen +
+ (encrypt ? authsize : 0),
+ &src_chained);
+ }

sgc = dma_map_sg_chained(jrdev, req->assoc, assoc_nents ? : 1,
DMA_TO_DEVICE, assoc_chained);
@@ -1234,11 +1244,9 @@ static int aead_encrypt(struct aead_request *req)
u32 *desc;
int ret = 0;

- req->cryptlen += ctx->authsize;
-
/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, DESC_JOB_IO_LEN *
- CAAM_CMD_SZ, &all_contig);
+ CAAM_CMD_SZ, &all_contig, true);
if (IS_ERR(edesc))
return PTR_ERR(edesc);

@@ -1275,7 +1283,7 @@ static int aead_decrypt(struct aead_request *req)

/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, DESC_JOB_IO_LEN *
- CAAM_CMD_SZ, &all_contig);
+ CAAM_CMD_SZ, &all_contig, false);
if (IS_ERR(edesc))
return PTR_ERR(edesc);

@@ -1332,7 +1340,8 @@ static struct aead_edesc *aead_giv_edesc_alloc(struct aead_givcrypt_request
src_nents = sg_count(req->src, req->cryptlen, &src_chained);

if (unlikely(req->dst != req->src))
- dst_nents = sg_count(req->dst, req->cryptlen, &dst_chained);
+ dst_nents = sg_count(req->dst, req->cryptlen + ctx->authsize,
+ &dst_chained);

sgc = dma_map_sg_chained(jrdev, req->assoc, assoc_nents ? : 1,
DMA_TO_DEVICE, assoc_chained);
@@ -1426,8 +1435,6 @@ static int aead_givencrypt(struct aead_givcrypt_request *areq)
u32 *desc;
int ret = 0;

- req->cryptlen += ctx->authsize;
-
/* allocate extended descriptor */
edesc = aead_giv_edesc_alloc(areq, DESC_JOB_IO_LEN *
CAAM_CMD_SZ, &contig);
--
1.7.7.6

2013-11-28 13:17:53

by Horia Geantă

[permalink] [raw]
Subject: [PATCH 3/4] crypto: talitos - fix aead sglen for case 'dst != src'

For aead case when source and destination buffers are different,
there is an incorrect assumption that the source length includes the ICV
length. Fix this, since it leads to an oops when using sg_count() to
find the number of nents in the scatterlist:

Unable to handle kernel paging request for data at address 0x00000004
Faulting instruction address: 0xf2265a28
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=8 P2020 RDB
Modules linked in: talitos(+)
CPU: 1 PID: 2187 Comm: cryptomgr_test Not tainted 3.11.0 #12
task: c4e72e20 ti: ef634000 task.ti: ef634000
NIP: f2265a28 LR: f2266ad8 CTR: c000c900
REGS: ef635bb0 TRAP: 0300 Not tainted (3.11.0)
MSR: 00029000 <CE,EE,ME> CR: 42042084 XER: 00000000
DEAR: 00000004, ESR: 00000000

GPR00: f2266e10 ef635c60 c4e72e20 00000001 00000014 ef635c69 00000001 c11f3082
GPR08: 00000010 00000000 00000002 2f635d58 22044084 00000000 00000000 c0755c80
GPR16: c4bf1000 ef784000 00000000 00000000 00000020 00000014 00000010 ef2f6100
GPR24: ef2f6200 00000024 ef143210 ef2f6000 00000000 ef635d58 00000000 2f635d58
NIP [f2265a28] sg_count+0x1c/0xb4 [talitos]
LR [f2266ad8] talitos_edesc_alloc+0x12c/0x410 [talitos]
Call Trace:
[ef635c60] [c0552068] schedule_timeout+0x148/0x1ac (unreliable)
[ef635cc0] [f2266e10] aead_edesc_alloc+0x54/0x64 [talitos]
[ef635ce0] [f22680f0] aead_encrypt+0x24/0x70 [talitos]
[ef635cf0] [c024b948] __test_aead+0x494/0xf68
[ef635e20] [c024d54c] test_aead+0x64/0xcc
[ef635e40] [c024d604] alg_test_aead+0x50/0xc4
[ef635e60] [c024c838] alg_test+0x10c/0x2e4
[ef635ee0] [c0249d1c] cryptomgr_test+0x4c/0x54
[ef635ef0] [c005d598] kthread+0xa8/0xac
[ef635f40] [c000e3bc] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
81230024 552807fe 0f080000 5523003a 4bffff24 39000000 2c040000 99050000
408100a0 7c691b78 38c00001 38600000 <80e90004> 38630001 8109000c 70ea0002
---[ end trace 4498123cd8478591 ]---

Signed-off-by: Horia Geanta <[email protected]>
---
Please apply the following first (sent 11/19/2013):
crypto: talitos - corrrectly handle zero-length assoc data
https://www.mail-archive.com/[email protected]/msg09904.html

drivers/crypto/talitos.c | 47 ++++++++++++++++++++++-----------------------
1 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index af3e7dc..30c9c93 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1110,7 +1110,8 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
unsigned int authsize,
unsigned int ivsize,
int icv_stashing,
- u32 cryptoflags)
+ u32 cryptoflags,
+ bool encrypt)
{
struct talitos_edesc *edesc;
int assoc_nents = 0, src_nents, dst_nents, alloc_len, dma_len;
@@ -1143,19 +1144,17 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
assoc_nents = assoc_nents ? assoc_nents + 1 : 2;
}

- src_nents = sg_count(src, cryptlen + authsize, &src_chained);
- src_nents = (src_nents == 1) ? 0 : src_nents;
-
- if (!dst) {
- dst_nents = 0;
- } else {
- if (dst == src) {
- dst_nents = src_nents;
- } else {
- dst_nents = sg_count(dst, cryptlen + authsize,
- &dst_chained);
- dst_nents = (dst_nents == 1) ? 0 : dst_nents;
- }
+ if (!dst || dst == src) {
+ src_nents = sg_count(src, cryptlen + authsize, &src_chained);
+ src_nents = (src_nents == 1) ? 0 : src_nents;
+ dst_nents = dst ? src_nents : 0;
+ } else { /* dst && dst != src*/
+ src_nents = sg_count(src, cryptlen + (encrypt ? 0 : authsize),
+ &src_chained);
+ src_nents = (src_nents == 1) ? 0 : src_nents;
+ dst_nents = sg_count(dst, cryptlen + (encrypt ? authsize : 0),
+ &dst_chained);
+ dst_nents = (dst_nents == 1) ? 0 : dst_nents;
}

/*
@@ -1206,7 +1205,7 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
}

static struct talitos_edesc *aead_edesc_alloc(struct aead_request *areq, u8 *iv,
- int icv_stashing)
+ int icv_stashing, bool encrypt)
{
struct crypto_aead *authenc = crypto_aead_reqtfm(areq);
struct talitos_ctx *ctx = crypto_aead_ctx(authenc);
@@ -1215,7 +1214,7 @@ static struct talitos_edesc *aead_edesc_alloc(struct aead_request *areq, u8 *iv,
return talitos_edesc_alloc(ctx->dev, areq->assoc, areq->src, areq->dst,
iv, areq->assoclen, areq->cryptlen,
ctx->authsize, ivsize, icv_stashing,
- areq->base.flags);
+ areq->base.flags, encrypt);
}

static int aead_encrypt(struct aead_request *req)
@@ -1225,7 +1224,7 @@ static int aead_encrypt(struct aead_request *req)
struct talitos_edesc *edesc;

/* allocate extended descriptor */
- edesc = aead_edesc_alloc(req, req->iv, 0);
+ edesc = aead_edesc_alloc(req, req->iv, 0, true);
if (IS_ERR(edesc))
return PTR_ERR(edesc);

@@ -1248,7 +1247,7 @@ static int aead_decrypt(struct aead_request *req)
req->cryptlen -= authsize;

/* allocate extended descriptor */
- edesc = aead_edesc_alloc(req, req->iv, 1);
+ edesc = aead_edesc_alloc(req, req->iv, 1, false);
if (IS_ERR(edesc))
return PTR_ERR(edesc);

@@ -1294,7 +1293,7 @@ static int aead_givencrypt(struct aead_givcrypt_request *req)
struct talitos_edesc *edesc;

/* allocate extended descriptor */
- edesc = aead_edesc_alloc(areq, req->giv, 0);
+ edesc = aead_edesc_alloc(areq, req->giv, 0, true);
if (IS_ERR(edesc))
return PTR_ERR(edesc);

@@ -1450,7 +1449,7 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
}

static struct talitos_edesc *ablkcipher_edesc_alloc(struct ablkcipher_request *
- areq)
+ areq, bool encrypt)
{
struct crypto_ablkcipher *cipher = crypto_ablkcipher_reqtfm(areq);
struct talitos_ctx *ctx = crypto_ablkcipher_ctx(cipher);
@@ -1458,7 +1457,7 @@ static struct talitos_edesc *ablkcipher_edesc_alloc(struct ablkcipher_request *

return talitos_edesc_alloc(ctx->dev, NULL, areq->src, areq->dst,
areq->info, 0, areq->nbytes, 0, ivsize, 0,
- areq->base.flags);
+ areq->base.flags, encrypt);
}

static int ablkcipher_encrypt(struct ablkcipher_request *areq)
@@ -1468,7 +1467,7 @@ static int ablkcipher_encrypt(struct ablkcipher_request *areq)
struct talitos_edesc *edesc;

/* allocate extended descriptor */
- edesc = ablkcipher_edesc_alloc(areq);
+ edesc = ablkcipher_edesc_alloc(areq, true);
if (IS_ERR(edesc))
return PTR_ERR(edesc);

@@ -1485,7 +1484,7 @@ static int ablkcipher_decrypt(struct ablkcipher_request *areq)
struct talitos_edesc *edesc;

/* allocate extended descriptor */
- edesc = ablkcipher_edesc_alloc(areq);
+ edesc = ablkcipher_edesc_alloc(areq, false);
if (IS_ERR(edesc))
return PTR_ERR(edesc);

@@ -1637,7 +1636,7 @@ static struct talitos_edesc *ahash_edesc_alloc(struct ahash_request *areq,
struct talitos_ahash_req_ctx *req_ctx = ahash_request_ctx(areq);

return talitos_edesc_alloc(ctx->dev, NULL, req_ctx->psrc, NULL, NULL, 0,
- nbytes, 0, 0, 0, areq->base.flags);
+ nbytes, 0, 0, 0, areq->base.flags, false);
}

static int ahash_init(struct ahash_request *areq)
--
1.7.7.6

2013-11-28 13:18:13

by Horia Geantă

[permalink] [raw]
Subject: [PATCH 4/4] crypto: testmgr - fix sglen in test_aead for case 'dst != src'

Commit d8a32ac25698cd60b02bed2100379803c7f964e3 (crypto: testmgr - make
test_aead also test 'dst != src' code paths) added support for different
source and destination buffers in test_aead.

This patch modifies the source and destination buffer lengths accordingly:
the lengths are not equal since encryption / decryption adds / removes
the ICV.

Cc: Jussi Kivilinna <[email protected]>
Signed-off-by: Horia Geanta <[email protected]>
---
crypto/testmgr.c | 26 ++++++++++++--------------
1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index e091ef6..a130871 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -503,16 +503,16 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
goto out;
}

- sg_init_one(&sg[0], input,
- template[i].ilen + (enc ? authsize : 0));
-
if (diff_dst) {
output = xoutbuf[0];
output += align_offset;
+ sg_init_one(&sg[0], input, template[i].ilen);
sg_init_one(&sgout[0], output,
+ template[i].rlen);
+ } else {
+ sg_init_one(&sg[0], input,
template[i].ilen +
(enc ? authsize : 0));
- } else {
output = input;
}

@@ -612,12 +612,6 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
memcpy(q, template[i].input + temp,
template[i].tap[k]);

- n = template[i].tap[k];
- if (k == template[i].np - 1 && enc)
- n += authsize;
- if (offset_in_page(q) + n < PAGE_SIZE)
- q[n] = 0;
-
sg_set_buf(&sg[k], q, template[i].tap[k]);

if (diff_dst) {
@@ -625,13 +619,17 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
offset_in_page(IDX[k]);

memset(q, 0, template[i].tap[k]);
- if (offset_in_page(q) + n < PAGE_SIZE)
- q[n] = 0;

sg_set_buf(&sgout[k], q,
template[i].tap[k]);
}

+ n = template[i].tap[k];
+ if (k == template[i].np - 1 && enc)
+ n += authsize;
+ if (offset_in_page(q) + n < PAGE_SIZE)
+ q[n] = 0;
+
temp += template[i].tap[k];
}

@@ -650,10 +648,10 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
goto out;
}

- sg[k - 1].length += authsize;
-
if (diff_dst)
sgout[k - 1].length += authsize;
+ else
+ sg[k - 1].length += authsize;
}

sg_init_table(asg, template[i].anp);
--
1.7.7.6

2013-11-28 14:26:42

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 1/4] crypto: ccm - Fix handling of zero plaintext when computing mac

On Thu, Nov 28, 2013 at 03:11:15PM +0200, Horia Geanta wrote:
> There are cases when cryptlen can be zero in crypto_ccm_auth():
> -encryptiom: input scatterlist length is zero (no plaintext)
> -decryption: input scatterlist contains only the mac
> plus the condition of having different source and destination buffers
> (or else scatterlist length = max(plaintext_len, ciphertext_len)).

All four patches applied.

Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt