linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* sha1_mb broken
@ 2016-08-26  1:15 Stephan Mueller
  2016-09-26 17:32 ` Stephan Mueller
  0 siblings, 1 reply; 11+ messages in thread
From: Stephan Mueller @ 2016-08-26  1:15 UTC (permalink / raw)
  To: linux-crypto

Hi,

I tried to execute tests with sha1_mb.

The execution simply stalls when invoking a digest operation -- i.e. the 
digest operation does not finish. After some time after invoking the hashing 
operation, the following log appears (note, the kccavs_* functions are my test 
code; that test code works perfectly well with all other hash 
implementations):

[  140.426026] INFO: rcu_sched detected stalls on CPUs/tasks:
[  140.426719] 	2-...: (1 GPs behind) idle=9c3/140000000000000/0 
softirq=2680/2707 fqs=14762 
[  140.427024] 	(detected by 0, t=60002 jiffies, g=655, c=654, q=35)
[  140.427024] Task dump for CPU 2:
[  140.427024] cavs_driver     R  running task        0   945    862 
0x00000008
[  140.427024]  ffffffff9b78d965 ffffa527b8bfa640 ffffa527bb505940 
ffffa52775c20c50
[  140.427024]  ffffa527bc300000 ffffa527b93d2a80 ffffa527bb857e00 
ffffa527bc2ffd78
[  140.427024]  ffffa527b93d2ac8 ffffa527bc2ffcc0 ffffffff9b78dfc8 
ffffa527bc2ffd70
[  140.427024] Call Trace:
[  140.427024]  [<ffffffff9b78d965>] ? __schedule+0x245/0x690
[  140.427024]  [<ffffffff9b78dfc8>] ? preempt_schedule_common+0x18/0x30
[  140.427024]  [<ffffffff9b791a68>] ? _raw_spin_lock_irq+0x28/0x30
[  140.427024]  [<ffffffff9b78ed98>] ? wait_for_completion_interruptible
+0x28/0x180
[  140.427024]  [<ffffffffc028541c>] ? sha1_mb_async_digest+0x6c/0x70 
[sha1_mb]
[  140.427024]  [<ffffffff9b3b1129>] ? crypto_ahash_op+0x29/0x70
[  140.427024]  [<ffffffffc0270148>] ? kccavs_test_ahash+0x198/0x2b0 
[kcapi_cavs]
[  140.427024]  [<ffffffffc026e20a>] ? kccavs_data_read+0xda/0x160 
[kcapi_cavs]
[  140.427024]  [<ffffffff9b370964>] ? full_proxy_read+0x54/0x90
[  140.427024]  [<ffffffff9b208088>] ? __vfs_read+0x28/0x110
[  140.427024]  [<ffffffff9b38a2c0>] ? security_file_permission+0xa0/0xc0
[  140.427024]  [<ffffffff9b20850e>] ? rw_verify_area+0x4e/0xb0
[  140.427024]  [<ffffffff9b208606>] ? vfs_read+0x96/0x130
[  140.427024]  [<ffffffff9b2099f6>] ? SyS_read+0x46/0xa0
[  140.427024]  [<ffffffff9b791cb6>] ? entry_SYSCALL_64_fastpath+0x1e/0xa8



Ciao
Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-08-26  1:15 sha1_mb broken Stephan Mueller
@ 2016-09-26 17:32 ` Stephan Mueller
  2016-09-28 17:58   ` Megha Dey
  0 siblings, 1 reply; 11+ messages in thread
From: Stephan Mueller @ 2016-09-26 17:32 UTC (permalink / raw)
  To: Megha Dey; +Cc: linux-crypto

Am Freitag, 26. August 2016, 03:15:06 CEST schrieb Stephan Mueller:

Hi Megha,

> Hi,
> 
> I tried to execute tests with sha1_mb.

Have you had a chance to look into this one?
> 
> The execution simply stalls when invoking a digest operation -- i.e. the
> digest operation does not finish. After some time after invoking the hashing
> operation, the following log appears (note, the kccavs_* functions are my
> test code; that test code works perfectly well with all other hash
> implementations):
> 
> [  140.426026] INFO: rcu_sched detected stalls on CPUs/tasks:
> [  140.426719] 	2-...: (1 GPs behind) idle=9c3/140000000000000/0
> softirq=2680/2707 fqs=14762
> [  140.427024] 	(detected by 0, t=60002 jiffies, g=655, c=654, q=35)
> [  140.427024] Task dump for CPU 2:
> [  140.427024] cavs_driver     R  running task        0   945    862
> 0x00000008
> [  140.427024]  ffffffff9b78d965 ffffa527b8bfa640 ffffa527bb505940
> ffffa52775c20c50
> [  140.427024]  ffffa527bc300000 ffffa527b93d2a80 ffffa527bb857e00
> ffffa527bc2ffd78
> [  140.427024]  ffffa527b93d2ac8 ffffa527bc2ffcc0 ffffffff9b78dfc8
> ffffa527bc2ffd70
> [  140.427024] Call Trace:
> [  140.427024]  [<ffffffff9b78d965>] ? __schedule+0x245/0x690
> [  140.427024]  [<ffffffff9b78dfc8>] ? preempt_schedule_common+0x18/0x30
> [  140.427024]  [<ffffffff9b791a68>] ? _raw_spin_lock_irq+0x28/0x30
> [  140.427024]  [<ffffffff9b78ed98>] ? wait_for_completion_interruptible
> +0x28/0x180
> [  140.427024]  [<ffffffffc028541c>] ? sha1_mb_async_digest+0x6c/0x70
> [sha1_mb]
> [  140.427024]  [<ffffffff9b3b1129>] ? crypto_ahash_op+0x29/0x70
> [  140.427024]  [<ffffffffc0270148>] ? kccavs_test_ahash+0x198/0x2b0
> [kcapi_cavs]
> [  140.427024]  [<ffffffffc026e20a>] ? kccavs_data_read+0xda/0x160
> [kcapi_cavs]
> [  140.427024]  [<ffffffff9b370964>] ? full_proxy_read+0x54/0x90
> [  140.427024]  [<ffffffff9b208088>] ? __vfs_read+0x28/0x110
> [  140.427024]  [<ffffffff9b38a2c0>] ? security_file_permission+0xa0/0xc0
> [  140.427024]  [<ffffffff9b20850e>] ? rw_verify_area+0x4e/0xb0
> [  140.427024]  [<ffffffff9b208606>] ? vfs_read+0x96/0x130
> [  140.427024]  [<ffffffff9b2099f6>] ? SyS_read+0x46/0xa0
> [  140.427024]  [<ffffffff9b791cb6>] ? entry_SYSCALL_64_fastpath+0x1e/0xa8
> 
> 
> 
> Ciao
> Stephan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Ciao
Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-09-26 17:32 ` Stephan Mueller
@ 2016-09-28 17:58   ` Megha Dey
  2016-09-28 18:25     ` Megha Dey
  0 siblings, 1 reply; 11+ messages in thread
From: Megha Dey @ 2016-09-28 17:58 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto

Hi Stephan,

Could you give me more info on how I could reproduce this issue on my
end?

Also was this issue there all along? Which is the first kernel version
where you see this?

Thanks,
Megha

On Mon, 2016-09-26 at 19:32 +0200, Stephan Mueller wrote:
> Am Freitag, 26. August 2016, 03:15:06 CEST schrieb Stephan Mueller:
> 
> Hi Megha,
> 
> > Hi,
> > 
> > I tried to execute tests with sha1_mb.
> 
> Have you had a chance to look into this one?
> > 
> > The execution simply stalls when invoking a digest operation -- i.e. the
> > digest operation does not finish. After some time after invoking the hashing
> > operation, the following log appears (note, the kccavs_* functions are my
> > test code; that test code works perfectly well with all other hash
> > implementations):
> > 
> > [  140.426026] INFO: rcu_sched detected stalls on CPUs/tasks:
> > [  140.426719] 	2-...: (1 GPs behind) idle=9c3/140000000000000/0
> > softirq=2680/2707 fqs=14762
> > [  140.427024] 	(detected by 0, t=60002 jiffies, g=655, c=654, q=35)
> > [  140.427024] Task dump for CPU 2:
> > [  140.427024] cavs_driver     R  running task        0   945    862
> > 0x00000008
> > [  140.427024]  ffffffff9b78d965 ffffa527b8bfa640 ffffa527bb505940
> > ffffa52775c20c50
> > [  140.427024]  ffffa527bc300000 ffffa527b93d2a80 ffffa527bb857e00
> > ffffa527bc2ffd78
> > [  140.427024]  ffffa527b93d2ac8 ffffa527bc2ffcc0 ffffffff9b78dfc8
> > ffffa527bc2ffd70
> > [  140.427024] Call Trace:
> > [  140.427024]  [<ffffffff9b78d965>] ? __schedule+0x245/0x690
> > [  140.427024]  [<ffffffff9b78dfc8>] ? preempt_schedule_common+0x18/0x30
> > [  140.427024]  [<ffffffff9b791a68>] ? _raw_spin_lock_irq+0x28/0x30
> > [  140.427024]  [<ffffffff9b78ed98>] ? wait_for_completion_interruptible
> > +0x28/0x180
> > [  140.427024]  [<ffffffffc028541c>] ? sha1_mb_async_digest+0x6c/0x70
> > [sha1_mb]
> > [  140.427024]  [<ffffffff9b3b1129>] ? crypto_ahash_op+0x29/0x70
> > [  140.427024]  [<ffffffffc0270148>] ? kccavs_test_ahash+0x198/0x2b0
> > [kcapi_cavs]
> > [  140.427024]  [<ffffffffc026e20a>] ? kccavs_data_read+0xda/0x160
> > [kcapi_cavs]
> > [  140.427024]  [<ffffffff9b370964>] ? full_proxy_read+0x54/0x90
> > [  140.427024]  [<ffffffff9b208088>] ? __vfs_read+0x28/0x110
> > [  140.427024]  [<ffffffff9b38a2c0>] ? security_file_permission+0xa0/0xc0
> > [  140.427024]  [<ffffffff9b20850e>] ? rw_verify_area+0x4e/0xb0
> > [  140.427024]  [<ffffffff9b208606>] ? vfs_read+0x96/0x130
> > [  140.427024]  [<ffffffff9b2099f6>] ? SyS_read+0x46/0xa0
> > [  140.427024]  [<ffffffff9b791cb6>] ? entry_SYSCALL_64_fastpath+0x1e/0xa8
> > 
> > 
> > 
> > Ciao
> > Stephan
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> Ciao
> Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-09-28 17:58   ` Megha Dey
@ 2016-09-28 18:25     ` Megha Dey
  2016-09-28 18:45       ` Stephan Mueller
  0 siblings, 1 reply; 11+ messages in thread
From: Megha Dey @ 2016-09-28 18:25 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto, tim.c.chen

Hi Stephan,

There was a bug fix: Commit ID : 0851561d (introduced in 4.6-rc5).

Assuming that you are using an older kernel than this one, maybe we are
issuing the complete with the wrong pointer, so the original issuer of
the request never gets the complete back.

If you are using an older kernel, can you please use the lastest and let
me know if you still see this issue?

Also can you give more info on the test case? Does it issue single
request or multiple requests?

Thanks,
Megha

On Wed, 2016-09-28 at 10:58 -0700, Megha Dey wrote:
> Hi Stephan,
> 
> Could you give me more info on how I could reproduce this issue on my
> end?
> 
> Also was this issue there all along? Which is the first kernel version
> where you see this?
> 
> Thanks,
> Megha
> 
> On Mon, 2016-09-26 at 19:32 +0200, Stephan Mueller wrote:
> > Am Freitag, 26. August 2016, 03:15:06 CEST schrieb Stephan Mueller:
> > 
> > Hi Megha,
> > 
> > > Hi,
> > > 
> > > I tried to execute tests with sha1_mb.
> > 
> > Have you had a chance to look into this one?
> > > 
> > > The execution simply stalls when invoking a digest operation -- i.e. the
> > > digest operation does not finish. After some time after invoking the hashing
> > > operation, the following log appears (note, the kccavs_* functions are my
> > > test code; that test code works perfectly well with all other hash
> > > implementations):
> > > 
> > > [  140.426026] INFO: rcu_sched detected stalls on CPUs/tasks:
> > > [  140.426719] 	2-...: (1 GPs behind) idle=9c3/140000000000000/0
> > > softirq=2680/2707 fqs=14762
> > > [  140.427024] 	(detected by 0, t=60002 jiffies, g=655, c=654, q=35)
> > > [  140.427024] Task dump for CPU 2:
> > > [  140.427024] cavs_driver     R  running task        0   945    862
> > > 0x00000008
> > > [  140.427024]  ffffffff9b78d965 ffffa527b8bfa640 ffffa527bb505940
> > > ffffa52775c20c50
> > > [  140.427024]  ffffa527bc300000 ffffa527b93d2a80 ffffa527bb857e00
> > > ffffa527bc2ffd78
> > > [  140.427024]  ffffa527b93d2ac8 ffffa527bc2ffcc0 ffffffff9b78dfc8
> > > ffffa527bc2ffd70
> > > [  140.427024] Call Trace:
> > > [  140.427024]  [<ffffffff9b78d965>] ? __schedule+0x245/0x690
> > > [  140.427024]  [<ffffffff9b78dfc8>] ? preempt_schedule_common+0x18/0x30
> > > [  140.427024]  [<ffffffff9b791a68>] ? _raw_spin_lock_irq+0x28/0x30
> > > [  140.427024]  [<ffffffff9b78ed98>] ? wait_for_completion_interruptible
> > > +0x28/0x180
> > > [  140.427024]  [<ffffffffc028541c>] ? sha1_mb_async_digest+0x6c/0x70
> > > [sha1_mb]
> > > [  140.427024]  [<ffffffff9b3b1129>] ? crypto_ahash_op+0x29/0x70
> > > [  140.427024]  [<ffffffffc0270148>] ? kccavs_test_ahash+0x198/0x2b0
> > > [kcapi_cavs]
> > > [  140.427024]  [<ffffffffc026e20a>] ? kccavs_data_read+0xda/0x160
> > > [kcapi_cavs]
> > > [  140.427024]  [<ffffffff9b370964>] ? full_proxy_read+0x54/0x90
> > > [  140.427024]  [<ffffffff9b208088>] ? __vfs_read+0x28/0x110
> > > [  140.427024]  [<ffffffff9b38a2c0>] ? security_file_permission+0xa0/0xc0
> > > [  140.427024]  [<ffffffff9b20850e>] ? rw_verify_area+0x4e/0xb0
> > > [  140.427024]  [<ffffffff9b208606>] ? vfs_read+0x96/0x130
> > > [  140.427024]  [<ffffffff9b2099f6>] ? SyS_read+0x46/0xa0
> > > [  140.427024]  [<ffffffff9b791cb6>] ? entry_SYSCALL_64_fastpath+0x1e/0xa8
> > > 
> > > 
> > > 
> > > Ciao
> > > Stephan
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> > Ciao
> > Stephan
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-09-28 18:25     ` Megha Dey
@ 2016-09-28 18:45       ` Stephan Mueller
  2016-09-28 22:52         ` Dey, Megha
  0 siblings, 1 reply; 11+ messages in thread
From: Stephan Mueller @ 2016-09-28 18:45 UTC (permalink / raw)
  To: Megha Dey; +Cc: linux-crypto, tim.c.chen

Am Mittwoch, 28. September 2016, 11:25:47 CEST schrieb Megha Dey:

Hi Megha,

> Hi Stephan,
> 
> There was a bug fix: Commit ID : 0851561d (introduced in 4.6-rc5).

I use the current cryptodev-2.6 tree.
> 
> Assuming that you are using an older kernel than this one, maybe we are
> issuing the complete with the wrong pointer, so the original issuer of
> the request never gets the complete back.
> 
> If you are using an older kernel, can you please use the lastest and let
> me know if you still see this issue?
> 
> Also can you give more info on the test case? Does it issue single
> request or multiple requests?

It is a single test with the message "8c899bba" where I expect 
"ac6d8c4851beacf61c175aed0699053d8f632df8"

The code is the following which works with any other hash:

struct kccavs_ahash_def {
	struct crypto_ahash *tfm;
	struct ahash_request *req;
	struct kccavs_tcrypt_res result;
};

/* Callback function */
static void kccavs_ahash_cb(struct crypto_async_request *req, int error)
{
	struct kccavs_tcrypt_res *result = req->data;

	if (error == -EINPROGRESS)
		return;
	result->err = error;
	complete(&result->completion);
	dbg("Encryption finished successfully\n");
}

/* Perform hash */
static unsigned int kccavs_ahash_op(struct kccavs_ahash_def *ahash)
{
	int rc = 0;

	rc = crypto_ahash_digest(ahash->req);

	switch (rc) {
	case 0:
		break;
	case -EINPROGRESS:
	case -EBUSY:
		rc = wait_for_completion_interruptible(&ahash->result.completion);
		if (!rc && !ahash->result.err) {
#ifdef OLDASYNC
			INIT_COMPLETION(aead->result.completion);
#else
			reinit_completion(&ahash->result.completion);
#endif
			break;
		}
	default:
		dbg("ahash cipher operation returned with %d result"
		    " %d\n",rc, ahash->result.err);
		break;
	}
	init_completion(&ahash->result.completion);

	return rc;
}

static int kccavs_test_ahash(size_t nbytes)
{
	int ret;
	struct crypto_ahash *tfm;
	struct ahash_request *req = NULL;
	struct kccavs_ahash_def ahash;
	struct kccavs_data *data = &kccavs_test->data;
	struct kccavs_data *key = &kccavs_test->key;
	unsigned char *digest = NULL;
	struct scatterlist sg;

	/*
	 * We explicitly do not check the input buffer as we allow
	 * an empty string.
	 */

	/* allocate synchronous hash */
	tfm = crypto_alloc_ahash(kccavs_test->name, 0, 0);
	if (IS_ERR(tfm)) {
		pr_info("could not allocate digest TFM handle for %s\n", kccavs_test-
>name);
		return PTR_ERR(tfm);
	}

	digest = kzalloc(crypto_ahash_digestsize(tfm), GFP_KERNEL);
	if (!digest) {
		ret = -ENOMEM;
		goto out;
	}

	req = ahash_request_alloc(tfm, GFP_KERNEL);
	if (!req) {
		pr_info("could not allocate request queue\n");
		ret = -ENOMEM;
		goto out;
	}
	ahash.tfm = tfm;
	ahash.req = req;

	if (key->len) {
		dbg("set key for HMAC\n");
		ret = crypto_ahash_setkey(tfm, key->data, key->len);
		if (ret < 0)
			goto out;
	}

	sg_init_one(&sg, data->data, data->len);
	ahash_request_set_crypt(req, &sg, digest, data->len);
	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
				   kccavs_ahash_cb, &ahash.result);

	ret = kccavs_ahash_op(&ahash);
	if (!ret) {
		data->len = crypto_ahash_digestsize(tfm);
		memcpy(data->data, digest, data->len);
	}

out:
	kzfree(digest);
	if (req)
		ahash_request_free(req);
	crypto_free_ahash(tfm);
	return ret;
}


Ciao
Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: sha1_mb broken
  2016-09-28 18:45       ` Stephan Mueller
@ 2016-09-28 22:52         ` Dey, Megha
  2016-09-29  5:30           ` Stephan Mueller
  0 siblings, 1 reply; 11+ messages in thread
From: Dey, Megha @ 2016-09-28 22:52 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto, tim.c.chen



-----Original Message-----
From: Stephan Mueller [mailto:smueller@chronox.de] 
Sent: Wednesday, September 28, 2016 11:46 AM
To: Dey, Megha <megha.dey@intel.com>
Cc: linux-crypto@vger.kernel.org; tim.c.chen@linux.intel.com
Subject: Re: sha1_mb broken

Am Mittwoch, 28. September 2016, 11:25:47 CEST schrieb Megha Dey:

Hi Megha,

> Hi Stephan,
> 
> There was a bug fix: Commit ID : 0851561d (introduced in 4.6-rc5).

I use the current cryptodev-2.6 tree.
> 
> Assuming that you are using an older kernel than this one, maybe we 
> are issuing the complete with the wrong pointer, so the original 
> issuer of the request never gets the complete back.
> 
> If you are using an older kernel, can you please use the lastest and 
> let me know if you still see this issue?
> 
> Also can you give more info on the test case? Does it issue single 
> request or multiple requests?

It is a single test with the message "8c899bba" where I expect "ac6d8c4851beacf61c175aed0699053d8f632df8"

>> For the message 8c899bba, the expected hash is  not ac6d8c4851beacf61c175aed0699053d8f632df8 but 
2e2816f6241fef89d78dd7afc926adc03ca94ace. I used this hash value in the existing tcrypt test and the test passes.

I would like to duplicate the test case you are using so that I can have a better look.
Can you provide the complete code for the test case? The code you sent earlier are missing some structure definitions. I will try to incorporate this test in the tcrypt test suite and try to reproduce this issue.

The code is the following which works with any other hash:

struct kccavs_ahash_def {
	struct crypto_ahash *tfm;
	struct ahash_request *req;
	struct kccavs_tcrypt_res result;
};

/* Callback function */
static void kccavs_ahash_cb(struct crypto_async_request *req, int error) {
	struct kccavs_tcrypt_res *result = req->data;

	if (error == -EINPROGRESS)
		return;
	result->err = error;
	complete(&result->completion);
	dbg("Encryption finished successfully\n"); }

/* Perform hash */
static unsigned int kccavs_ahash_op(struct kccavs_ahash_def *ahash) {
	int rc = 0;

	rc = crypto_ahash_digest(ahash->req);

	switch (rc) {
	case 0:
		break;
	case -EINPROGRESS:
	case -EBUSY:
		rc = wait_for_completion_interruptible(&ahash->result.completion);
		if (!rc && !ahash->result.err) {
#ifdef OLDASYNC
			INIT_COMPLETION(aead->result.completion);
#else
			reinit_completion(&ahash->result.completion);
#endif
			break;
		}
	default:
		dbg("ahash cipher operation returned with %d result"
		    " %d\n",rc, ahash->result.err);
		break;
	}
	init_completion(&ahash->result.completion);

	return rc;
}

static int kccavs_test_ahash(size_t nbytes) {
	int ret;
	struct crypto_ahash *tfm;
	struct ahash_request *req = NULL;
	struct kccavs_ahash_def ahash;
	struct kccavs_data *data = &kccavs_test->data;
	struct kccavs_data *key = &kccavs_test->key;
	unsigned char *digest = NULL;
	struct scatterlist sg;

	/*
	 * We explicitly do not check the input buffer as we allow
	 * an empty string.
	 */

	/* allocate synchronous hash */
	tfm = crypto_alloc_ahash(kccavs_test->name, 0, 0);
	if (IS_ERR(tfm)) {
		pr_info("could not allocate digest TFM handle for %s\n", kccavs_test-
>name);
		return PTR_ERR(tfm);
	}

	digest = kzalloc(crypto_ahash_digestsize(tfm), GFP_KERNEL);
	if (!digest) {
		ret = -ENOMEM;
		goto out;
	}

	req = ahash_request_alloc(tfm, GFP_KERNEL);
	if (!req) {
		pr_info("could not allocate request queue\n");
		ret = -ENOMEM;
		goto out;
	}
	ahash.tfm = tfm;
	ahash.req = req;

	if (key->len) {
		dbg("set key for HMAC\n");
		ret = crypto_ahash_setkey(tfm, key->data, key->len);
		if (ret < 0)
			goto out;
	}

	sg_init_one(&sg, data->data, data->len);
	ahash_request_set_crypt(req, &sg, digest, data->len);
	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
				   kccavs_ahash_cb, &ahash.result);

	ret = kccavs_ahash_op(&ahash);
	if (!ret) {
		data->len = crypto_ahash_digestsize(tfm);
		memcpy(data->data, digest, data->len);
	}

out:
	kzfree(digest);
	if (req)
		ahash_request_free(req);
	crypto_free_ahash(tfm);
	return ret;
}


Ciao
Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-09-28 22:52         ` Dey, Megha
@ 2016-09-29  5:30           ` Stephan Mueller
  2016-10-04  0:25             ` Dey, Megha
  0 siblings, 1 reply; 11+ messages in thread
From: Stephan Mueller @ 2016-09-29  5:30 UTC (permalink / raw)
  To: Dey, Megha; +Cc: linux-crypto, tim.c.chen

[-- Attachment #1: Type: text/plain, Size: 136 bytes --]

Am Mittwoch, 28. September 2016, 22:52:46 CEST schrieb Dey, Megha:

Hi Megha,

see a self contained example code attached.

Ciao
Stephan

[-- Attachment #2: sha1_mb.tar.xz --]
[-- Type: application/x-xz-compressed-tar, Size: 2092 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: sha1_mb broken
  2016-09-29  5:30           ` Stephan Mueller
@ 2016-10-04  0:25             ` Dey, Megha
  2016-10-04 14:10               ` Stephan Mueller
  0 siblings, 1 reply; 11+ messages in thread
From: Dey, Megha @ 2016-10-04  0:25 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: linux-crypto, tim.c.chen



-----Original Message-----
From: Stephan Mueller [mailto:smueller@chronox.de] 
Sent: Wednesday, September 28, 2016 10:31 PM
To: Dey, Megha <megha.dey@intel.com>
Cc: linux-crypto@vger.kernel.org; tim.c.chen@linux.intel.com
Subject: Re: sha1_mb broken

Am Mittwoch, 28. September 2016, 22:52:46 CEST schrieb Dey, Megha:

Hi Megha,

see a self contained example code attached.
 
> Hi Stephan,
>
> Your test code initialized the completion structure incorrectly, that led to the missing completion from being received. The init_completion call should be made before the crypto_ahash_digest call. The following change to your test code fixes things. ( I have also fixed what I believe is a typo aead->ahash)

> @@ -74,6 +74,8 @@ static unsigned int kccavs_ahash_op(struct kccavs_ahash_def *ahash)
> {
>        int rc = 0;
>
> +       init_completion(&ahash->result.completion);
> +
>        rc = crypto_ahash_digest(ahash->req);
>
>        switch (rc) {
>@@ -84,7 +86,7 @@ static unsigned int kccavs_ahash_op(struct kccavs_ahash_def *ahash)
>                rc = wait_for_completion_interruptible(&ahash->result.completion);
>                if (!rc && !ahash->result.err) {
> #ifdef OLDASYNC
> -                       INIT_COMPLETION(aead->result.completion);
> +                       INIT_COMPLETION(&ahash->result.completion);
> #else
>                       reinit_completion(&ahash->result.completion);
> #endif
> @@ -95,7 +97,6 @@ static unsigned int kccavs_ahash_op(struct kccavs_ahash_def *ahash)
>                    " %d\n",rc, ahash->result.err);
>                break;
>       }
> -       init_completion(&ahash->result.completion);
>
>     return rc;
>}
> This initialization of the completion structure happens correctly in the tcrypt test module used by the kernel, hence I did not come across this issue earlier.
> Thanks,
> Megha
Ciao
Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-10-04  0:25             ` Dey, Megha
@ 2016-10-04 14:10               ` Stephan Mueller
  2016-10-04 16:08                 ` Tim Chen
  0 siblings, 1 reply; 11+ messages in thread
From: Stephan Mueller @ 2016-10-04 14:10 UTC (permalink / raw)
  To: Dey, Megha; +Cc: linux-crypto, tim.c.chen

Am Dienstag, 4. Oktober 2016, 00:25:07 CEST schrieb Dey, Megha:

Hi Megha,

> 
> > Hi Stephan,
> > 
> > Your test code initialized the completion structure incorrectly, that led
> > to the missing completion from being received. The init_completion call
> > should be made before the crypto_ahash_digest call. The following change

Thanks a lot for pointing that one out. Can you help me understand why your 
code trips over that issue whereas other ahash implementations do not (all 
other SHA-1 or SHA-2 implementations work perfectly fine with that code)?

Thanks again!

Ciao
Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-10-04 14:10               ` Stephan Mueller
@ 2016-10-04 16:08                 ` Tim Chen
  2016-10-04 16:27                   ` Stephan Mueller
  0 siblings, 1 reply; 11+ messages in thread
From: Tim Chen @ 2016-10-04 16:08 UTC (permalink / raw)
  To: Stephan Mueller, Dey, Megha; +Cc: linux-crypto

On Tue, 2016-10-04 at 16:10 +0200, Stephan Mueller wrote:
> Am Dienstag, 4. Oktober 2016, 00:25:07 CEST schrieb Dey, Megha:
> 
> Hi Megha,
> 
> > 
> > 
> > > 
> > > Hi Stephan,
> > > 
> > > Your test code initialized the completion structure incorrectly, that led
> > > to the missing completion from being received. The init_completion call
> > > should be made before the crypto_ahash_digest call. The following change
> Thanks a lot for pointing that one out. Can you help me understand why your 
> code trips over that issue whereas other ahash implementations do not (all 
> other SHA-1 or SHA-2 implementations work perfectly fine with that code)?
> 

There is a spin lock protecting the completion's wait_queue on the processes waiting for
the completion of the job, and the queue head.  My suspicion is if these
structures are not initialized properly, we fail to look up the waiting process in the queue
properly to call it.  For the other tested cases, they may not be a true ahash operation
in the sense of passing request through the crypto daemon, and have to context switch
to let crypto daemon complete the job.  The computation proceeds
and returns in the same call chain.

Thanks.

Tim

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: sha1_mb broken
  2016-10-04 16:08                 ` Tim Chen
@ 2016-10-04 16:27                   ` Stephan Mueller
  0 siblings, 0 replies; 11+ messages in thread
From: Stephan Mueller @ 2016-10-04 16:27 UTC (permalink / raw)
  To: Tim Chen; +Cc: Dey, Megha, linux-crypto

Am Dienstag, 4. Oktober 2016, 09:08:42 CEST schrieb Tim Chen:

Hi Tim,

> There is a spin lock protecting the completion's wait_queue on the processes
> waiting for the completion of the job, and the queue head.  My suspicion is
> if these structures are not initialized properly, we fail to look up the
> waiting process in the queue properly to call it.  For the other tested
> cases, they may not be a true ahash operation in the sense of passing
> request through the crypto daemon, and have to context switch to let crypto
> daemon complete the job.  The computation proceeds
> and returns in the same call chain.

Thanks a lot for the clarification.

Ciao
Stephan

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-10-04 16:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-26  1:15 sha1_mb broken Stephan Mueller
2016-09-26 17:32 ` Stephan Mueller
2016-09-28 17:58   ` Megha Dey
2016-09-28 18:25     ` Megha Dey
2016-09-28 18:45       ` Stephan Mueller
2016-09-28 22:52         ` Dey, Megha
2016-09-29  5:30           ` Stephan Mueller
2016-10-04  0:25             ` Dey, Megha
2016-10-04 14:10               ` Stephan Mueller
2016-10-04 16:08                 ` Tim Chen
2016-10-04 16:27                   ` Stephan Mueller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).