Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: algif_aead: AIO broken with more than one iocb
From: Stephan Mueller @ 2016-09-11 13:41 UTC (permalink / raw)
  To: noloader; +Cc: Herbert Xu, linux-crypto
In-Reply-To: <CAH8yC8knD0U4-BhgzkUfyHMm-KyNyDCx-3PPqQOm0bdVh4qjiA@mail.gmail.com>

Am Sonntag, 11. September 2016, 08:43:00 CEST schrieb Jeffrey Walton:

Hi Jeffrey,

> > The AIO support for algif_aead is broken when submitting more than one
> > iocb.
> > The break happens in aead_recvmsg_async at the following code:
> I think the kernel needs to take a half step back, and add the missing
> self tests and test cases to be more proactive in detecting breaks
> earlier. Speaking first hand, some of these breaks have existed for
> months.
> 
> I don't take the position you can't break things. I believe you can't
> make an omelet without breaking eggs; and if you're not breaking
> something, then you're probably not getting anything done. The
> engineering defect is not detecting the break.

The testing that is implemented for libkcapi should cover almost all code 
paths of AF_ALG in the kernel. However, I just added the AIO support to the 
library in the last few days as this logic is not straight forward. Thus these 
issues show up now.

If you wish to analyze the AIO support more, I can certainly push my current 
development branch of libkcapi to my github tree so that you would have a 
working AIO user space component.

Ciao
Stephan

^ permalink raw reply

* Re: algif_aead: AIO broken with more than one iocb
From: Jeffrey Walton @ 2016-09-11 12:43 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: Herbert Xu, linux-crypto
In-Reply-To: <6245755.LbXSUvPjJL@positron.chronox.de>

> The AIO support for algif_aead is broken when submitting more than one iocb.
> The break happens in aead_recvmsg_async at the following code:
>

I think the kernel needs to take a half step back, and add the missing
self tests and test cases to be more proactive in detecting breaks
earlier. Speaking first hand, some of these breaks have existed for
months.

I don't take the position you can't break things. I believe you can't
make an omelet without breaking eggs; and if you're not breaking
something, then you're probably not getting anything done. The
engineering defect is not detecting the break.

Jeff

^ permalink raw reply

* algif_aead: AIO broken with more than one iocb
From: Stephan Mueller @ 2016-09-11  2:59 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto

Hi Herbert,

The AIO support for algif_aead is broken when submitting more than one iocb. 
The break happens in aead_recvmsg_async at the following code:

        /* ensure output buffer is sufficiently large */
        if (usedpages < outlen)
                goto free;

The reason is that when submitting, say, two iocb, ctx->used contains the 
buffer length for two AEAD operations (as expected). However, the recvmsg code 
is invoked for each iocb individually and thus usedpages should only be 
expected to point to memory for one AEAD operation. But this violates the 
check above.

For example, I have two independent AEAD operations that I want to trigger. 
The input to each is 48 bytes (including space for AAD and tag). The output 
buffer that I have for each AEAD operation is also 48 bytes and thus 
sufficient for the AEAD operation. Yet, when submitting the two AEAD 
operations in one io_submit (i.e. using two iocb), ctx->used indicates that 
the kernel has 96 bytes to process. This is correct, but only half of it 
should be processed in one recvmsg_async invocation.

Note, the AIO operation works perfectly well, when io_submit only sends one 
iocb.

Do you have any idea on how to fix that?

Ciao
Stephan

^ permalink raw reply

* [PATCH -next] hwrng: st - Fix missing clk_disable_unprepare() on error in st_rng_probe()
From: Wei Yongjun @ 2016-09-10 12:03 UTC (permalink / raw)
  To: Patrice Chotard, Matt Mackall, Herbert Xu
  Cc: Wei Yongjun, linux-arm-kernel, kernel, linux-crypto

From: Wei Yongjun <weiyongjun1@huawei.com>

Fix the missing clk_disable_unprepare() before return
from st_rng_probe() in the error handling case.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/char/hw_random/st-rng.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/hw_random/st-rng.c b/drivers/char/hw_random/st-rng.c
index 7e8aa6b..938ec10 100644
--- a/drivers/char/hw_random/st-rng.c
+++ b/drivers/char/hw_random/st-rng.c
@@ -108,6 +108,7 @@ static int st_rng_probe(struct platform_device *pdev)
 	ret = hwrng_register(&ddata->ops);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register HW RNG\n");
+		clk_disable_unprepare(clk);
 		return ret;
 	}
 

^ permalink raw reply related

* [PATCH] crypto: call put_page on used pages only
From: Stephan Mueller @ 2016-09-10 11:50 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <8008966.Q0OrxOpoA1@positron.chronox.de>

Hi Herbert,

This patch fixes the reported BUG reliably that I was able to
create with my (faulty) libkcapi test code.

However, I am yet unable to pinpoint the code that allocates an
SG without an associated page that would trigger the BUG.

In any case, if you approve, I would recommend that this patch
should go to 4.8 and to stable as well.

---8<---

Ensure that put_page is only invoked on pages that were used by
algif_skcipher.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index d7acb73..bc36a9a 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -179,7 +179,7 @@ static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
 
 			if (sg[i].length)
 				return;
-			if (put)
+			if (put && page_ref_count(sg_page(sg + i)))
 				put_page(sg_page(sg + i));
 			sg_assign_page(sg + i, NULL);
 		}
-- 
2.7.4

^ permalink raw reply related

* Company Agent Needed
From: Shougang Group  @ 2016-09-10  8:37 UTC (permalink / raw)




-- 
Shougang Group
45 Huagong Road Xinji City,
Hebei Province China.
webpage: www.shougang.com.cn


This is an official request for Professional/consultants who will stand as
our regional representative to run logistics on behalf of Shougang Group.
We are only looking for individual or company from USA and Canada. You
will be entitle to ten percent (10%) of every payment you receive from our
customers in your region on behalf of the company.

NOTE: You are not require to travel, all communications with our customers
will be through email or phone. All fees, including taxes will be handled
by us.

You can apply for this position by filling the information below and send
back to us;

Full Name:
Residencial or office address:
Town:
State:
Zipcode:
Country:
Current Job:
If you own a company, please state the company name:
Prefered Email:
Gender:
Age:
Telephone:

If you have a current job, you can still be part of our business, as
services for us will not bother with your hours of work.

Respectfully,
Mr. Zhu Jimin (Chief Executive Officer)
Shougang Group

^ permalink raw reply

* [PATCH v2] crypto: only call put_page used pages
From: Stephan Mueller @ 2016-09-10  3:48 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <2555546.QkgIzR2raW@positron.chronox.de>

Hi Herbert,

Changes v2: use sg_page(sg) only once

---8<---

For asynchronous operation, SGs are allocated without a page mapped to
them. If the SGL is freed, the code must only call put_page for an SG if
there was a page assigned to it in the first place.

This fixes a kernel crash when using io_submit with more than one iocb.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 28556fc..d7acb73 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -86,8 +86,13 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
 	}
 	sgl = sreq->tsg;
 	n = sg_nents(sgl);
-	for_each_sg(sgl, sg, n, i)
-		put_page(sg_page(sg));
+	for_each_sg(sgl, sg, n, i) {
+		struct page *page = sg_page(sg);
+
+		/* some SGs may not have a page mapped */
+		if (page_ref_count(page))
+			put_page(page);
+	}
 
 	kfree(sreq->tsg);
 }
-- 
2.7.4

^ permalink raw reply related

* [PATCH] crypto: only call put_page used pages
From: Stephan Mueller @ 2016-09-10  3:42 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <2637498.CLV37KecV5@positron.chronox.de>

Hi Herbert,

This patch was tested with up to 64 iocb submitted with one io_submit call.

If you approve of this fix, I recommend that should go to the current 4.8 development cycle and to stable.

---8<---

For asynchronous operation, SGs are allocated without a page mapped to
them. If the SGL is freed, the code must only call put_page for an SG if
there was a page assigned to it in the first place.

This fixes a kernel crash when using io_submit with more than one iocb.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 28556fc..58ec57a 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -86,8 +86,13 @@ static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
 	}
 	sgl = sreq->tsg;
 	n = sg_nents(sgl);
-	for_each_sg(sgl, sg, n, i)
-		put_page(sg_page(sg));
+	for_each_sg(sgl, sg, n, i) {
+		struct page *page = sg_page(sg);
+
+		/* some SGs may not have a page mapped */
+		if (page_ref_count(page))
+			put_page(sg_page(sg));
+	}
 
 	kfree(sreq->tsg);
 }
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] softirq: fix tasklet_kill() and its users
From: Santosh Shilimkar @ 2016-09-10  1:41 UTC (permalink / raw)
  To: Santosh Shilimkar, linux-kernel
  Cc: qat-linux, linux-crypto, netdev, Greg Kroah-Hartman,
	Andrew Morton, Thomas Gleixner, Tadeusz Struk, Herbert Xu,
	David S. Miller, Paul Bolle, Giovanni Cabiddu,
	Salvatore Benedetto, Karsten Keil, Peter Zijlstra (Intel)
In-Reply-To: <1472089950-2724-1-git-send-email-ssantosh@kernel.org>

Ping !!

On 8/24/2016 6:52 PM, Santosh Shilimkar wrote:
> Semantically the expectation from the tasklet init/kill API
> should be as below.
>
> tasklet_init() == Init and Enable scheduling
> tasklet_kill() == Disable scheduling and Destroy
>
> tasklet_init() API exibit above behavior but not the
> tasklet_kill(). The tasklet handler can still get scheduled
> and run even after the tasklet_kill().
>
> There are 2, 3 places where drivers are working around
> this issue by calling tasklet_disable() which will add an
> usecount and there by avoiding the handlers being called.
>
> tasklet_enable/tasklet_disable is a pair API and expected
> to be used together. Usage of tasklet_disable() *just* to
> workround tasklet scheduling after kill is probably not the
> correct and inteded use of the API as done the API.
> We also happen to see similar issue where in shutdown path
> the tasklet_handler was getting called even after the
> tasklet_kill().
>
> We fix this be making sure tasklet_kill() does right
> thing and there by ensuring tasklet handler won't run after
> tasklet_kil() with very simple change. Patch fixes the tasklet
> code and also few drivers workarounds.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tadeusz Struk <tadeusz.struk@intel.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Paul Bolle <pebolle@tiscali.nl>
> Cc: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> Cc: Salvatore Benedetto <salvatore.benedetto@intel.com>
> Cc: Karsten Keil <isdn@linux-pingi.de>
> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
>
> Signed-off-by: Santosh Shilimkar <ssantosh@kernel.org>
> ---
> Removed RFC tag from last post and dropped atmel serial
> driver which seems to have been fixed in 4.8
>
> https://lkml.org/lkml/2016/8/7/7
>
>  drivers/crypto/qat/qat_common/adf_isr.c    | 1 -
>  drivers/crypto/qat/qat_common/adf_sriov.c  | 1 -
>  drivers/crypto/qat/qat_common/adf_vf_isr.c | 2 --
>  drivers/isdn/gigaset/interface.c           | 1 -
>  kernel/softirq.c                           | 7 ++++---
>  5 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/crypto/qat/qat_common/adf_isr.c b/drivers/crypto/qat/qat_common/adf_isr.c
> index 06d4901..fd5e900 100644
> --- a/drivers/crypto/qat/qat_common/adf_isr.c
> +++ b/drivers/crypto/qat/qat_common/adf_isr.c
> @@ -296,7 +296,6 @@ static void adf_cleanup_bh(struct adf_accel_dev *accel_dev)
>  	int i;
>
>  	for (i = 0; i < hw_data->num_banks; i++) {
> -		tasklet_disable(&priv_data->banks[i].resp_handler);
>  		tasklet_kill(&priv_data->banks[i].resp_handler);
>  	}
>  }
> diff --git a/drivers/crypto/qat/qat_common/adf_sriov.c b/drivers/crypto/qat/qat_common/adf_sriov.c
> index 9320ae1..bc7c2fa 100644
> --- a/drivers/crypto/qat/qat_common/adf_sriov.c
> +++ b/drivers/crypto/qat/qat_common/adf_sriov.c
> @@ -204,7 +204,6 @@ void adf_disable_sriov(struct adf_accel_dev *accel_dev)
>  	}
>
>  	for (i = 0, vf = accel_dev->pf.vf_info; i < totalvfs; i++, vf++) {
> -		tasklet_disable(&vf->vf2pf_bh_tasklet);
>  		tasklet_kill(&vf->vf2pf_bh_tasklet);
>  		mutex_destroy(&vf->pf2vf_lock);
>  	}
> diff --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c
> index bf99e11..6e38bff 100644
> --- a/drivers/crypto/qat/qat_common/adf_vf_isr.c
> +++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c
> @@ -191,7 +191,6 @@ static int adf_setup_pf2vf_bh(struct adf_accel_dev *accel_dev)
>
>  static void adf_cleanup_pf2vf_bh(struct adf_accel_dev *accel_dev)
>  {
> -	tasklet_disable(&accel_dev->vf.pf2vf_bh_tasklet);
>  	tasklet_kill(&accel_dev->vf.pf2vf_bh_tasklet);
>  	mutex_destroy(&accel_dev->vf.vf2pf_lock);
>  }
> @@ -268,7 +267,6 @@ static void adf_cleanup_bh(struct adf_accel_dev *accel_dev)
>  {
>  	struct adf_etr_data *priv_data = accel_dev->transport;
>
> -	tasklet_disable(&priv_data->banks[0].resp_handler);
>  	tasklet_kill(&priv_data->banks[0].resp_handler);
>  }
>
> diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c
> index 600c79b..2ce63b6 100644
> --- a/drivers/isdn/gigaset/interface.c
> +++ b/drivers/isdn/gigaset/interface.c
> @@ -524,7 +524,6 @@ void gigaset_if_free(struct cardstate *cs)
>  	if (!drv->have_tty)
>  		return;
>
> -	tasklet_disable(&cs->if_wake_tasklet);
>  	tasklet_kill(&cs->if_wake_tasklet);
>  	cs->tty_dev = NULL;
>  	tty_unregister_device(drv->tty, cs->minor_index);
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 17caf4b..21397eb 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -498,7 +498,7 @@ static void tasklet_action(struct softirq_action *a)
>  		list = list->next;
>
>  		if (tasklet_trylock(t)) {
> -			if (!atomic_read(&t->count)) {
> +			if (atomic_read(&t->count) == 1) {
>  				if (!test_and_clear_bit(TASKLET_STATE_SCHED,
>  							&t->state))
>  					BUG();
> @@ -534,7 +534,7 @@ static void tasklet_hi_action(struct softirq_action *a)
>  		list = list->next;
>
>  		if (tasklet_trylock(t)) {
> -			if (!atomic_read(&t->count)) {
> +			if (atomic_read(&t->count) == 1) {
>  				if (!test_and_clear_bit(TASKLET_STATE_SCHED,
>  							&t->state))
>  					BUG();
> @@ -559,7 +559,7 @@ void tasklet_init(struct tasklet_struct *t,
>  {
>  	t->next = NULL;
>  	t->state = 0;
> -	atomic_set(&t->count, 0);
> +	atomic_set(&t->count, 1);
>  	t->func = func;
>  	t->data = data;
>  }
> @@ -576,6 +576,7 @@ void tasklet_kill(struct tasklet_struct *t)
>  		} while (test_bit(TASKLET_STATE_SCHED, &t->state));
>  	}
>  	tasklet_unlock_wait(t);
> +	atomic_dec(&t->count);
>  	clear_bit(TASKLET_STATE_SCHED, &t->state);
>  }
>  EXPORT_SYMBOL(tasklet_kill);
>

^ permalink raw reply

* Re: Kernel panic - encryption/decryption failed when open file on Arm64
From: Ard Biesheuvel @ 2016-09-09 10:56 UTC (permalink / raw)
  To: xiakaixu
  Cc: Herbert Xu, David S. Miller, Theodore Ts'o, Jaegeuk Kim,
	nhorman, mh1, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org, Bintian, liushuoran, Huxinwei,
	zhangzhibin.zhang
In-Reply-To: <CAKv+Gu9JDCHDteOvgpEg4SJP4OKgM6=euF3pzBcXDJxTt=dAYg@mail.gmail.com>

On 9 September 2016 at 11:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 September 2016 at 11:19, xiakaixu <xiakaixu@huawei.com> wrote:
>> Hi,
>>
>> After a deeply research about this crash, seems it is a specific
>> bug that only exists in armv8 board. And it occurs in this function
>> in arch/arm64/crypto/aes-glue.c.
>>
>> static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
>>                        struct scatterlist *src, unsigned int nbytes)
>> {
>>        ...
>>
>>         desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
>>         blkcipher_walk_init(&walk, dst, src, nbytes);
>>         err = blkcipher_walk_virt_block(desc, &walk, AES_BLOCK_SIZE); --->
>> page allocation failed
>>
>>         ...
>>
>>         while ((blocks = (walk.nbytes / AES_BLOCK_SIZE))) {           ---->
>> walk.nbytes = 0, and skip this loop
>>                 aes_ctr_encrypt(walk.dst.virt.addr, walk.src.virt.addr,
>>                                 (u8 *)ctx->key_enc, rounds, blocks, walk.iv,
>>                                 first);
>>         ...
>>                 err = blkcipher_walk_done(desc, &walk,
>>                                           walk.nbytes % AES_BLOCK_SIZE);
>>         }
>>         if (nbytes) {                                                 ---->
>> enter this if() statement
>>                 u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
>>                 u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
>>         ...
>>
>>                 aes_ctr_encrypt(tail, tsrc, (u8 *)ctx->key_enc, rounds,
>> ----> the the sencond input parameter is NULL, so crash...
>>                                 blocks, walk.iv, first);
>>         ...
>>         }
>>         ...
>> }
>>
>>
>> If the page allocation failed in the function blkcipher_walk_virt_block(),
>> the variable walk.nbytes = 0, so it will skip the while() loop and enter
>> the if(nbytes) statment. But here the varibale tsrc is NULL and it is also
>> the sencond input parameter of the function aes_ctr_encrypt()... Kernel
>> Panic...
>>
>> I have also researched the similar function in other architectures, and
>> there if(walk.nbytes) is used, not this if(nbytes) statement in the armv8.
>> so I think this armv8 function ctr_encrypt() should deal with the page
>> allocation failed situation.
>>

Does this solve your problem?

diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
index 5c888049d061..6b2aa0fd6cd0 100644
--- a/arch/arm64/crypto/aes-glue.c
+++ b/arch/arm64/crypto/aes-glue.c
@@ -216,7 +216,7 @@ static int ctr_encrypt(struct blkcipher_desc
*desc, struct scatterlist *dst,
                err = blkcipher_walk_done(desc, &walk,
                                          walk.nbytes % AES_BLOCK_SIZE);
        }
-       if (nbytes) {
+       if (walk.nbytes % AES_BLOCK_SIZE) {
                u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
                u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
                u8 __aligned(8) tail[AES_BLOCK_SIZE];

^ permalink raw reply related

* Re: Kernel panic - encryption/decryption failed when open file on Arm64
From: Ard Biesheuvel @ 2016-09-09 10:31 UTC (permalink / raw)
  To: xiakaixu
  Cc: Herbert Xu, David S. Miller, Theodore Ts'o, Jaegeuk Kim,
	nhorman, mh1, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org, Bintian, liushuoran, Huxinwei,
	zhangzhibin.zhang
In-Reply-To: <57D28CB8.4080904@huawei.com>

On 9 September 2016 at 11:19, xiakaixu <xiakaixu@huawei.com> wrote:
> Hi,
>
> After a deeply research about this crash, seems it is a specific
> bug that only exists in armv8 board. And it occurs in this function
> in arch/arm64/crypto/aes-glue.c.
>
> static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
>                        struct scatterlist *src, unsigned int nbytes)
> {
>        ...
>
>         desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
>         blkcipher_walk_init(&walk, dst, src, nbytes);
>         err = blkcipher_walk_virt_block(desc, &walk, AES_BLOCK_SIZE); --->
> page allocation failed
>
>         ...
>
>         while ((blocks = (walk.nbytes / AES_BLOCK_SIZE))) {           ---->
> walk.nbytes = 0, and skip this loop
>                 aes_ctr_encrypt(walk.dst.virt.addr, walk.src.virt.addr,
>                                 (u8 *)ctx->key_enc, rounds, blocks, walk.iv,
>                                 first);
>         ...
>                 err = blkcipher_walk_done(desc, &walk,
>                                           walk.nbytes % AES_BLOCK_SIZE);
>         }
>         if (nbytes) {                                                 ---->
> enter this if() statement
>                 u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
>                 u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
>         ...
>
>                 aes_ctr_encrypt(tail, tsrc, (u8 *)ctx->key_enc, rounds,
> ----> the the sencond input parameter is NULL, so crash...
>                                 blocks, walk.iv, first);
>         ...
>         }
>         ...
> }
>
>
> If the page allocation failed in the function blkcipher_walk_virt_block(),
> the variable walk.nbytes = 0, so it will skip the while() loop and enter
> the if(nbytes) statment. But here the varibale tsrc is NULL and it is also
> the sencond input parameter of the function aes_ctr_encrypt()... Kernel
> Panic...
>
> I have also researched the similar function in other architectures, and
> there if(walk.nbytes) is used, not this if(nbytes) statement in the armv8.
> so I think this armv8 function ctr_encrypt() should deal with the page
> allocation failed situation.
>

OK, thanks for the report, and for the analysis. I will investigate,
and propose a fix

Thanks,
Ard.

^ permalink raw reply

* Re: Kernel panic - encryption/decryption failed when open file on Arm64
From: xiakaixu @ 2016-09-09 10:19 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, tytso, jaegeuk, nhorman, ard.biesheuvel, mh1, linux-crypto,
	linux-kernel, Bintian, liushuoran, Huxinwei, zhangzhibin.zhang
In-Reply-To: <20160908124709.GA26586@gondor.apana.org.au>

Hi,

After a deeply research about this crash, seems it is a specific
bug that only exists in armv8 board. And it occurs in this function
in arch/arm64/crypto/aes-glue.c.

static int ctr_encrypt(struct blkcipher_desc *desc, struct scatterlist *dst,
                        struct scatterlist *src, unsigned int nbytes)
{
        ...

         desc->flags &= ~CRYPTO_TFM_REQ_MAY_SLEEP;
         blkcipher_walk_init(&walk, dst, src, nbytes);
         err = blkcipher_walk_virt_block(desc, &walk, AES_BLOCK_SIZE); ---> page allocation failed

	...

         while ((blocks = (walk.nbytes / AES_BLOCK_SIZE))) {           ----> walk.nbytes = 0, and skip this loop
                 aes_ctr_encrypt(walk.dst.virt.addr, walk.src.virt.addr,
                                 (u8 *)ctx->key_enc, rounds, blocks, walk.iv,
                                 first);
	...
                 err = blkcipher_walk_done(desc, &walk,
                                           walk.nbytes % AES_BLOCK_SIZE);
         }
         if (nbytes) {                                                 ----> enter this if() statement
                 u8 *tdst = walk.dst.virt.addr + blocks * AES_BLOCK_SIZE;
                 u8 *tsrc = walk.src.virt.addr + blocks * AES_BLOCK_SIZE;
	...

                 aes_ctr_encrypt(tail, tsrc, (u8 *)ctx->key_enc, rounds,  ----> the the sencond input parameter is NULL, so crash...
                                 blocks, walk.iv, first);
	...
         }
         ...
}


If the page allocation failed in the function blkcipher_walk_virt_block(),
the variable walk.nbytes = 0, so it will skip the while() loop and enter
the if(nbytes) statment. But here the varibale tsrc is NULL and it is also
the sencond input parameter of the function aes_ctr_encrypt()... Kernel Panic...

I have also researched the similar function in other architectures, and
there if(walk.nbytes) is used, not this if(nbytes) statement in the armv8.
so I think this armv8 function ctr_encrypt() should deal with the page
allocation failed situation.

Regards
Kaixu Xia


> On Thu, Sep 08, 2016 at 08:38:43PM +0800, xiakaixu wrote:
>> Hi,
>>
>> I am using the encryption/decryption feature on arm64 board and a kernel
>> panic occurs just when open a file.  As the memory size of the board
>> is limited
>> and there are some page allocation failures before the panic.
>>
>> Seems it is a kernel bug from the call trace log.
>>
>>      ...
>>      - fscrypt_get_encryption_info
>>        - get_crypt_info.part.1
>>         - validate_user_key.isra.0
>>          - derive_aes_gcm_key
>>           - crypto_gcm_decrypt
>>            - ablk_decrypt
>>             - ctr_encrypt
>>              - blkcipher_walk_done
>>                - blkcipher_walk_next
>>                 -  __get_free_pages
>> ----------------------------------> page allocation failure
>>        ...
>>             - aes_ctr_encrypt
>> -----------------------------------------> the input parameter is
>> NULL pointer as the page allocation failure
>>
>>
>> The input parameter of function aes_ctr_encrypt() comes from the
>> /struct blkcipher_walk//
>> //walk/, and this variable /walk /is allocated by the function
>> __get_free_pages(). So if this
>> page allocate failed, the input parameter of function
>> aes_ctr_encrypt() will be NULL. The
>> panic will occurs if we don't check the input parameter.
>>
>> Not sure about this and wish to get your opinions!
>
> If the page allocation fails in blkcipher_walk_next it'll simply
> switch over to processing it block by block. so I don't think the
> warning is related to the crash.
>
> Cheers,
>

^ permalink raw reply

* [PATCH] hwrng: amd-rng - Migrate to managed API
From: PrasannaKumar Muralidharan @ 2016-09-09  7:58 UTC (permalink / raw)
  To: mpm, herbert, clabbe.montjoie, linux-crypto; +Cc: PrasannaKumar Muralidharan

Managed API eliminates error handling code, thus reduces several lines
of code.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 drivers/char/hw_random/amd-rng.c | 48 +++++++++-------------------------------
 1 file changed, 11 insertions(+), 37 deletions(-)

diff --git a/drivers/char/hw_random/amd-rng.c b/drivers/char/hw_random/amd-rng.c
index 9959c76..4dbc5aa 100644
--- a/drivers/char/hw_random/amd-rng.c
+++ b/drivers/char/hw_random/amd-rng.c
@@ -55,7 +55,6 @@ MODULE_DEVICE_TABLE(pci, pci_tbl);
 struct amd768_priv {
 	void __iomem *iobase;
 	struct pci_dev *pcidev;
-	u32 pmbase;
 };
 
 static int amd_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
@@ -149,58 +148,33 @@ found:
 	if (pmbase == 0)
 		return -EIO;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (IS_ERR(priv))
+		return PTR_ERR(priv);
 
-	if (!request_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE, DRV_NAME)) {
+	if (!devm_request_region(&pdev->dev, pmbase + PMBASE_OFFSET,
+				PMBASE_SIZE, DRV_NAME)) {
 		dev_err(&pdev->dev, DRV_NAME " region 0x%x already in use!\n",
 			pmbase + 0xF0);
-		err = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 
-	priv->iobase = ioport_map(pmbase + PMBASE_OFFSET, PMBASE_SIZE);
-	if (!priv->iobase) {
+	priv->iobase = devm_ioport_map(&pdev->dev, pmbase + PMBASE_OFFSET,
+			PMBASE_SIZE);
+	if (IS_ERR(priv->iobase)) {
 		pr_err(DRV_NAME "Cannot map ioport\n");
-		err = -EINVAL;
-		goto err_iomap;
+		return PTR_ERR(priv->iobase);
 	}
 
 	amd_rng.priv = (unsigned long)priv;
-	priv->pmbase = pmbase;
 	priv->pcidev = pdev;
 
 	pr_info(DRV_NAME " detected\n");
-	err = hwrng_register(&amd_rng);
-	if (err) {
-		pr_err(DRV_NAME " registering failed (%d)\n", err);
-		goto err_hwrng;
-	}
-	return 0;
-
-err_hwrng:
-	ioport_unmap(priv->iobase);
-err_iomap:
-	release_region(pmbase + PMBASE_OFFSET, PMBASE_SIZE);
-out:
-	kfree(priv);
-	return err;
+	return devm_hwrng_register(&pdev->dev, &amd_rng);
 }
 
 static void __exit mod_exit(void)
 {
-	struct amd768_priv *priv;
-
-	priv = (struct amd768_priv *)amd_rng.priv;
-
-	hwrng_unregister(&amd_rng);
-
-	ioport_unmap(priv->iobase);
-
-	release_region(priv->pmbase + PMBASE_OFFSET, PMBASE_SIZE);
-
-	kfree(priv);
 }
 
 module_init(mod_init);
-- 
2.5.0

^ permalink raw reply related

* Re: Kernel panic - encryption/decryption failed when open file on Arm64
From: xiakaixu @ 2016-09-09  4:08 UTC (permalink / raw)
  To: Herbert Xu
  Cc: davem, tytso, jaegeuk, nhorman, ard.biesheuvel, mh1, linux-crypto,
	linux-kernel, Bintian, liushuoran, Huxinwei, zhangzhibin.zhang
In-Reply-To: <20160908124709.GA26586@gondor.apana.org.au>

Sorry for resend this email, just add the linux-crypto@vger.kernel.org
and linux-kernel@vger.kernel.org.


Hi,

Firstly, thanks for your reply!

To reproduce this kernel panic, I test the encryption/decryption feature 
on arm64 board with more memory. Just add the following
change:

     diff --git a/crypto/blkcipher.c b/crypto/blkcipher.c
     index 0122bec..10ef3f4 100644
     --- a/crypto/blkcipher.c
     +++ b/crypto/blkcipher.c
     @@ -240,6 +240,7 @@ static int blkcipher_walk_next(struct 
blkcipher_desc *desc,
                     walk->flags |= BLKCIPHER_WALK_COPY;
                     if (!walk->page) {
                             walk->page = (void 
*)__get_free_page(GFP_ATOMIC);
     +                      walk->page = NULL;
                             if (!walk->page)
                                     n = 0;
                     }


This change just set the walk->page to NULL manually.
I get the same crash when open file with the above change log. So I 
think this NULL page failure is not be handled correctly in current code.

Regards
Kaixu Xia

> On Thu, Sep 08, 2016 at 08:38:43PM +0800, xiakaixu wrote:
>> Hi,
>>
>> I am using the encryption/decryption feature on arm64 board and a kernel
>> panic occurs just when open a file.  As the memory size of the board
>> is limited
>> and there are some page allocation failures before the panic.
>>
>> Seems it is a kernel bug from the call trace log.
>>
>>      ...
>>      - fscrypt_get_encryption_info
>>        - get_crypt_info.part.1
>>         - validate_user_key.isra.0
>>          - derive_aes_gcm_key
>>           - crypto_gcm_decrypt
>>            - ablk_decrypt
>>             - ctr_encrypt
>>              - blkcipher_walk_done
>>                - blkcipher_walk_next
>>                 -  __get_free_pages
>> ----------------------------------> page allocation failure
>>        ...
>>             - aes_ctr_encrypt
>> -----------------------------------------> the input parameter is
>> NULL pointer as the page allocation failure
>>
>>
>> The input parameter of function aes_ctr_encrypt() comes from the
>> /struct blkcipher_walk//
>> //walk/, and this variable /walk /is allocated by the function
>> __get_free_pages(). So if this
>> page allocate failed, the input parameter of function
>> aes_ctr_encrypt() will be NULL. The
>> panic will occurs if we don't check the input parameter.
>>
>> Not sure about this and wish to get your opinions!
>
> If the page allocation fails in blkcipher_walk_next it'll simply
> switch over to processing it block by block. so I don't think the
> warning is related to the crash.
>
> Cheers,
>

^ permalink raw reply

* test
From: xiakaixu @ 2016-09-09  3:47 UTC (permalink / raw)
  To: linux-kernel, linux-crypto

Sorry for noise!
-- 
Regards
Kaixu Xia

^ permalink raw reply

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
From: Herbert Xu @ 2016-09-08 17:02 UTC (permalink / raw)
  To: Romain Perier
  Cc: PrasannaKumar Muralidharan, dsaxena, mpm, Gregory Clement,
	Thomas Petazzoni, Nadav Haklai, Omri Itach, Shadi Ammouri,
	Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits, Igal Liberman,
	Marcin Wojtas, linux-crypto
In-Reply-To: <57D18801.5050901@free-electrons.com>

On Thu, Sep 08, 2016 at 05:47:13PM +0200, Romain Perier wrote:
>
> I was wondering something. hwrng_unregister does not check the kref
> reference counter of the object... so technically if the current
> rng_device is in use, with or without devm... calling
> platform->remove will break the driver anyway because
> hwrng_unregister will unbind the device from /dev/hwrng. What I mean
> is that I think that we had this issue even without
> devm_hwrng_register.
> 
> Herbert, could you confirm ?

Right.  remove can happen at any time and the driver needs to
cope with it by returning an error from data_read if the hardware
disappears in the middle of an operation.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: BUG in recvmsg using io_submit
From: Stephan Mueller @ 2016-09-08 15:50 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <2637498.CLV37KecV5@positron.chronox.de>

Am Donnerstag, 8. September 2016, 04:52:08 CEST schrieb Stephan Mueller:

Hi,

> Hi Herbert,
> 
> another one, different than the first report

Please note that the error seems to be triggered due to a bad use of the user 
space interface: When submitting the iov, I told the kernel that iovlen is the 
size of the buffer instead of the number of iovs.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
From: Romain Perier @ 2016-09-08 15:47 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: dsaxena, mpm, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas,
	linux-crypto
In-Reply-To: <CANc+2y5yzMNeHEgf6FfQHiN=-528YZgB3JwVcqa3C-s8s_RCfA@mail.gmail.com>



Le 07/09/2016 16:45, PrasannaKumar Muralidharan a écrit :
> On 7 September 2016 at 19:53, Romain Perier
> <romain.perier@free-electrons.com> wrote:
>> Hello,
>>
>>
>> Le 06/09/2016 18:31, PrasannaKumar Muralidharan a écrit :
>>>>
>>>> Use devm_hwrng_register instead of hwrng_register. It avoids the need
>>>> to handle unregistration explicitly from the remove function.
>>>>
>>>> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
>>>> ---
>>>>    drivers/char/hw_random/omap-rng.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/char/hw_random/omap-rng.c
>>>> b/drivers/char/hw_random/omap-rng.c
>>>> index d47b24d..171c3e8 100644
>>>> --- a/drivers/char/hw_random/omap-rng.c
>>>> +++ b/drivers/char/hw_random/omap-rng.c
>>>> @@ -381,7 +381,7 @@ static int omap_rng_probe(struct platform_device
>>>> *pdev)
>>>>           if (ret)
>>>>                   goto err_ioremap;
>>>>
>>>> -       ret = hwrng_register(&omap_rng_ops);
>>>> +       ret = devm_hwrng_register(dev, &omap_rng_ops);
>>>>           if (ret)
>>>>                   goto err_register;
>>>>
>>>> @@ -402,8 +402,6 @@ static int omap_rng_remove(struct platform_device
>>>> *pdev)
>>>>    {
>>>>           struct omap_rng_dev *priv = platform_get_drvdata(pdev);
>>>>
>>>> -       hwrng_unregister(&omap_rng_ops);
>>>> -
>>>>           priv->pdata->cleanup(priv);
>>>>
>>>>           pm_runtime_put_sync(&pdev->dev);
>>>> --
>>>
>>>
>>> If devm_hwrng_register is used hwrng_unregister will be called after
>>> pm_runtime_disable is called. If RNG device is in use calling
>>> omap_rng_remove may not work properly.
>>>
>>
>> The case where the remove function is called is if you unbind the driver by
>> hand or you call rmmod while the RNG device is used.
>> I don't think that the kernel will call platform->remove is the device is in
>> use (so /dev/hwrng). I mean the argument that the unregister function is
>> called after pm_runtime_disable is correct, but I don't think that the
>> remove function might be called while the device is in use. There is
>> necessarily a mutual exclusive case between "use the device" and "call the
>> remove function of the device". However, I am open to suggestions.
>
> The way you explained is good :D. Good point too. But the device is
> created by hw_random core (hwrng_modinit in core.c) so the device can
> be in use when omap-rng module is removed. Please feel free to correct
> me if I am wrong.
>
> Cheers,
> PrasannaKumar
>

Hi,

I was wondering something. hwrng_unregister does not check the kref 
reference counter of the object... so technically if the current 
rng_device is in use, with or without devm... calling platform->remove 
will break the driver anyway because hwrng_unregister will unbind the 
device from /dev/hwrng. What I mean is that I think that we had this 
issue even without devm_hwrng_register.

Herbert, could you confirm ?

Romain
-- 
Romain Perier, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH] crypto: engine: fix linux-next merge conflict
From: Arnd Bergmann @ 2016-09-08 13:57 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Petr Mladek, Corentin Labbe, Herbert Xu, Stephen Rothwell,
	Arnd Bergmann, David S. Miller, linux-crypto, linux-kernel

A merge conflict between the akpm-current tree and the crypto tree
caused a build failure in ARM allmodconfig today:

crypto/crypto_engine.c: In function 'crypto_transfer_hash_request':
crypto/crypto_engine.c:234:3: error: implicit declaration of function 'queue_kthread_work' [-Werror=implicit-function-declaration]

This adapts the crypto code to the API change.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 4cba7cf025f3 ("crypto: engine - permit to enqueue ashash_request")
Fixes: 8ca76638a2d0 ("kthread: kthread worker API cleanup")
---
 crypto/crypto_engine.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index 64fcd9ee3e2e..6989ba0046df 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -231,7 +231,7 @@ int crypto_transfer_hash_request(struct crypto_engine *engine,
 	ret = ahash_enqueue_request(&engine->queue, req);
 
 	if (!engine->busy && need_pump)
-		queue_kthread_work(&engine->kworker, &engine->pump_requests);
+		kthread_queue_work(&engine->kworker, &engine->pump_requests);
 
 	spin_unlock_irqrestore(&engine->queue_lock, flags);
 	return ret;
@@ -284,7 +284,7 @@ void crypto_finalize_cipher_request(struct crypto_engine *engine,
 
 	req->base.complete(&req->base, err);
 
-	queue_kthread_work(&engine->kworker, &engine->pump_requests);
+	kthread_queue_work(&engine->kworker, &engine->pump_requests);
 }
 EXPORT_SYMBOL_GPL(crypto_finalize_cipher_request);
 
-- 
2.9.0

^ permalink raw reply related

* Re: Kernel panic - encryption/decryption failed when open file on Arm64
From: Herbert Xu @ 2016-09-08 12:47 UTC (permalink / raw)
  To: xiakaixu
  Cc: davem, tytso, jaegeuk, nhorman, ard.biesheuvel, mh1, linux-crypto,
	linux-kernel, Bintian, liushuoran, Huxinwei, zhangzhibin.zhang
In-Reply-To: <57D15BD3.40903@huawei.com>

On Thu, Sep 08, 2016 at 08:38:43PM +0800, xiakaixu wrote:
> Hi,
> 
> I am using the encryption/decryption feature on arm64 board and a kernel
> panic occurs just when open a file.  As the memory size of the board
> is limited
> and there are some page allocation failures before the panic.
> 
> Seems it is a kernel bug from the call trace log.
> 
>     ...
>     - fscrypt_get_encryption_info
>       - get_crypt_info.part.1
>        - validate_user_key.isra.0
>         - derive_aes_gcm_key
>          - crypto_gcm_decrypt
>           - ablk_decrypt
>            - ctr_encrypt
>             - blkcipher_walk_done
>               - blkcipher_walk_next
>                -  __get_free_pages
> ----------------------------------> page allocation failure
>       ...
>            - aes_ctr_encrypt
> -----------------------------------------> the input parameter is
> NULL pointer as the page allocation failure
> 
> 
> The input parameter of function aes_ctr_encrypt() comes from the
> /struct blkcipher_walk//
> //walk/, and this variable /walk /is allocated by the function
> __get_free_pages(). So if this
> page allocate failed, the input parameter of function
> aes_ctr_encrypt() will be NULL. The
> panic will occurs if we don't check the input parameter.
> 
> Not sure about this and wish to get your opinions!

If the page allocation fails in blkcipher_walk_next it'll simply
switch over to processing it block by block. so I don't think the
warning is related to the crash.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: rmmod crypto driver when ipsec is in use
From: Herbert Xu @ 2016-09-08 11:19 UTC (permalink / raw)
  To: Harsh Jain; +Cc: linux-crypto, Stephan Mueller
In-Reply-To: <CAFXBA==cKF9DgBtMSEK42D-ukw4VX6NRVXgCAbiLf-XqMUAGOA@mail.gmail.com>

On Thu, Sep 08, 2016 at 03:17:46PM +0530, Harsh Jain wrote:
> Hi,
> 
> What is the expected behavior when driver is unregistered(Rmmod ) with
> active ipsec session.?
> I am getting stacktrace(BUG_ON in crypto_unregister_alg) instead of
> "module in use".

The driver is supposed to set cra_module to itself.  If it doesn't
then it's buggy.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* rmmod crypto driver when ipsec is in use
From: Harsh Jain @ 2016-09-08  9:47 UTC (permalink / raw)
  To: linux-crypto, Herbert Xu, Stephan Mueller

Hi,

What is the expected behavior when driver is unregistered(Rmmod ) with
active ipsec session.?
I am getting stacktrace(BUG_ON in crypto_unregister_alg) instead of
"module in use".


Regards
Harsh Jain

^ permalink raw reply

* BUG in recvmsg using io_submit
From: Stephan Mueller @ 2016-09-08  2:52 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto

Hi Herbert,

another one, different than the first report



[71972.773115] page:ffffea0001fabf40 count:0 mapcount:0 mapping:          (null) index:0x0
[71972.773118] flags: 0x1ffffc00000000()
[71972.773119] page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
[71972.773140] ------------[ cut here ]------------
[71972.774961] kernel BUG at include/linux/mm.h:420!
[71972.776738] invalid opcode: 0000 [#4] SMP
[71972.778521] Modules linked in: serpent_avx2 serpent_avx_x86_64 serpent_sse2_x86_64 serpent_generic loop crypto_user vhost_net vhost macvtap macvlan rfcomm xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun ip6t_REJECT nf_reject_ipv6 ip6t_rpfilter xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_security ip6table_mangle ip6table_raw ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 iptable_security iptable_mangle iptable_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack ebtable_filter ebtables ip6table_filter ip6_tables bnep nls_utf8 hfsplus intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel brcmfmac kvm joydev iTCO_wdt iTCO_vendor_support applesmc irqbypass input_polldev brcmutil intel_cstate inte
 l_uncore cfg80211
[71972.782836]  btusb intel_rapl_perf snd_hda_codec_hdmi snd_hda_codec_cirrus snd_hda_codec_generic btrtl btbcm snd_hda_intel snd_hda_codec btintel bluetooth snd_hda_core snd_hwdep mmc_core snd_seq i2c_i801 intel_pch_thermal snd_seq_device thunderbolt snd_pcm rfkill bcm5974 snd_timer mei_me snd lpc_ich mei shpchp dw_dmac_pci soundcore spi_pxa2xx_pci acpi_als kfifo_buf industrialio spi_pxa2xx_platform sbs apple_bl tpm_tis sbshc tpm nfsd auth_rpcgss binfmt_misc nfs_acl lockd grace sunrpc dm_crypt hid_apple i915 crct10dif_pclmul crc32_pclmul crc32c_intel i2c_algo_bit drm_kms_helper uas usb_storage ghash_clmulni_intel drm fjes video
[71972.789497] CPU: 3 PID: 26006 Comm: kcapi Tainted: G      D         4.7.2-201.fc24.x86_64 #1
[71972.792255] Hardware name: Apple Inc. MacBookPro12,1/Mac-E43C1C25D4880AD6, BIOS MBP121.88Z.0167.B17.1606231721 06/23/2016
[71972.794492] task: ffff8803fe6e1e80 ti: ffff880046c40000 task.ti: ffff880046c40000
[71972.796679] RIP: 0010:[<ffffffff8139957e>]  [<ffffffff8139957e>] skcipher_free_async_sgls+0xde/0x120
[71972.798884] RSP: 0018:ffff880046c43bd8  EFLAGS: 00010246
[71972.801085] RAX: 000000000000003e RBX: ffffea0001fabf40 RCX: 0000000000000006
[71972.803300] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88046ecce060
[71972.805478] RBP: ffff880046c43c00 R08: 000000000000097d R09: 0000000000000005
[71972.807592] R10: ffff88041fcbccc0 R11: ffffffff81f3edad R12: 0000000000000000
[71972.809670] R13: ffff88041fcba000 R14: 0000000000000001 R15: ffff88041128cf40
[71972.811749] FS:  00007fd2fc61e700(0000) GS:ffff88046ecc0000(0000) knlGS:0000000000000000
[71972.813839] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[71972.815931] CR2: 00007fd2fc649000 CR3: 000000007ead7000 CR4: 00000000003406e0
[71972.817978] Stack:
[71972.819968]  ffff88022e035da8 0000000000000010 ffff88041fcba000 0000000000000000
[71972.822444]  0000000000000001 ffff880046c43ca0 ffffffff81399b07 ffff88041fcba3d8
[71972.825780]  ffff88041fcba2b0 ffff88041fcba008 0000000000000000 0000000000000000
[71972.828995] Call Trace:
[71972.832162]  [<ffffffff81399b07>] skcipher_recvmsg+0x547/0x720
[71972.834912]  [<ffffffff816ae640>] ? sock_recvmsg+0x50/0x50
[71972.837625]  [<ffffffff816ae62d>] sock_recvmsg+0x3d/0x50
[71972.840318]  [<ffffffff816ae6d0>] sock_read_iter+0x90/0xe0
[71972.842997]  [<ffffffff8129677b>] aio_run_iocb+0x12b/0x2d0
[71972.845696]  [<ffffffff812976c6>] ? do_io_submit+0x196/0x550
[71972.845698]  [<ffffffff81264b53>] ? __fdget+0x13/0x20
[71972.845699]  [<ffffffff812977c1>] do_io_submit+0x291/0x550
[71972.845701]  [<ffffffff81297a90>] SyS_io_submit+0x10/0x20
[71972.845703]  [<ffffffff817eb872>] entry_SYSCALL_64_fastpath+0x1a/0xa4
[71972.845721] Code: 49 8b 1f 48 83 e3 fc 48 8b 43 20 48 8d 50 ff a8 01 48 0f 45 da 8b 43 1c 85 c0 75 bb 48 c7 c6 68 46 a1 81 48 89 df e8 b2 d9 e4 ff <0f> 0b 48 89 df e8 58 e4 e2 ff 48 8b 03 48 c1 e8 34 83 e0 07 83 
[71972.845724] RIP  [<ffffffff8139957e>] skcipher_free_async_sgls+0xde/0x120
[71972.845724]  RSP <ffff880046c43bd8>
[71972.845762] ---[ end trace e16e0f71a7a79254 ]---

Ciao
Stephan

^ permalink raw reply

* BUG while working on algif_skcipher AIO support
From: Stephan Mueller @ 2016-09-08  2:23 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto

Hi Herbert,

here is a reliably producable bug that I triggered while educating my libkcapi 
about AIO support.



[70129.671557] page:ffffea0001361d80 count:0 mapcount:0 mapping:          
(null) index:0x0
[70129.671560] flags: 0x1ffffc00000000()
[70129.671562] page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
[70129.671581] ------------[ cut here ]------------
[70129.671599] kernel BUG at include/linux/mm.h:420!
[70129.671612] invalid opcode: 0000 [#2] SMP
[70129.671623] Modules linked in: serpent_avx2 serpent_avx_x86_64 
serpent_sse2_x86_64 serpent_generic loop crypto_user vhost_net vhost macvtap 
macvlan rfcomm xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun 
ip6t_REJECT nf_reject_ipv6 ip6t_rpfilter xt_conntrack ip_set nfnetlink 
ebtable_nat ebtable_broute bridge stp llc ip6table_security ip6table_mangle 
ip6table_raw ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 
iptable_security iptable_mangle iptable_raw iptable_nat nf_conntrack_ipv4 
nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack ebtable_filter ebtables 
ip6table_filter ip6_tables bnep nls_utf8 hfsplus intel_rapl 
x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel brcmfmac kvm joydev 
iTCO_wdt iTCO_vendor_support applesmc irqbypass input_polldev brcmutil 
intel_cstate intel_uncore cfg80211
[70129.671875]  btusb intel_rapl_perf snd_hda_codec_hdmi snd_hda_codec_cirrus 
snd_hda_codec_generic btrtl btbcm snd_hda_intel snd_hda_codec btintel 
bluetooth snd_hda_core snd_hwdep mmc_core snd_seq i2c_i801 intel_pch_thermal 
snd_seq_device thunderbolt snd_pcm rfkill bcm5974 snd_timer mei_me snd lpc_ich 
mei shpchp dw_dmac_pci soundcore spi_pxa2xx_pci acpi_als kfifo_buf 
industrialio spi_pxa2xx_platform sbs apple_bl tpm_tis sbshc tpm nfsd 
auth_rpcgss binfmt_misc nfs_acl lockd grace sunrpc dm_crypt hid_apple i915 
crct10dif_pclmul crc32_pclmul crc32c_intel i2c_algo_bit drm_kms_helper uas 
usb_storage ghash_clmulni_intel drm fjes video
[70129.672075] CPU: 2 PID: 24751 Comm: kcapi Tainted: G      D         
4.7.2-201.fc24.x86_64 #1
[70129.672095] Hardware name: Apple Inc. MacBookPro12,1/Mac-E43C1C25D4880AD6, 
BIOS MBP121.88Z.0167.B17.1606231721 06/23/2016
[70129.672120] task: ffff8801f2f53d00 ti: ffff88004f5c4000 task.ti: 
ffff88004f5c4000
[70129.672138] RIP: 0010:[<ffffffff81398811>]  [<ffffffff81398811>] 
skcipher_pull_sgl+0x171/0x180
[70129.672163] RSP: 0018:ffff88004f5c7d38  EFLAGS: 00010246
[70129.672176] RAX: 000000000000003e RBX: ffff880228ae1018 RCX: 
0000000000000006
[70129.672193] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 
ffff88046ec8e060
[70129.672210] RBP: ffff88004f5c7d80 R08: 0000000000000922 R09: 
0000000000000005
[70129.672227] R10: ffff8803fc90efb0 R11: ffffffff81f3edad R12: 
0000000000000001
[70129.672244] R13: 0000000000000000 R14: 0000000000000000 R15: 
ffff88044a671800
[70129.672262] FS:  00007f99962aa700(0000) GS:ffff88046ec80000(0000) knlGS:
0000000000000000
[70129.672281] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[70129.672296] CR2: 000000000170e1d8 CR3: 00000000250b0000 CR4: 
00000000003406e0
[70129.672313] Stack:
[70129.672318]  ffff88044a677800 ffff8803fc90e1d0 ffff8803fc90e0b0 
ffff8803fc90e1d0
[70129.672340]  ffff88044a677ac8 ffff88044a671800 ffff88044a677800 
ffff88044a4a6ba8
[70129.672361]  ffff88020f5ae900 ffff88004f5c7db0 ffffffff8139889a 
ffff88044a677ac8
[70129.672382] Call Trace:
[70129.672390]  [<ffffffff8139889a>] skcipher_sock_destruct+0x7a/0xc0
[70129.672406]  [<ffffffff816b2746>] __sk_destruct+0x26/0x140
[70129.672420]  [<ffffffff816b42a0>] sk_destruct+0x20/0x30
[70129.672434]  [<ffffffff816b42f3>] __sk_free+0x43/0xa0
[70129.672447]  [<ffffffff816b4368>] sk_free+0x18/0x20
[70129.672460]  [<ffffffff81397963>] af_alg_release+0x23/0x30
[70129.672474]  [<ffffffff816ad79f>] sock_release+0x1f/0x80
[70129.672488]  [<ffffffff816ad812>] sock_close+0x12/0x20
[70129.672502]  [<ffffffff812481df>] __fput+0xdf/0x1f0
[70129.672514]  [<ffffffff8124832e>] ____fput+0xe/0x10
[70129.672527]  [<ffffffff810bd923>] task_work_run+0x83/0xb0
[70129.672542]  [<ffffffff81003342>] exit_to_usermode_loop+0xc2/0xd0
[70129.672557]  [<ffffffff81003ce1>] syscall_return_slowpath+0xa1/0xb0
[70129.672574]  [<ffffffff817eb8fa>] entry_SYSCALL_64_fastpath+0xa2/0xa4
[70129.672589] Code: 00 00 00 75 08 41 c6 87 fd 02 00 00 00 48 83 c4 20 5b 41 
5c 41 5d 41 5e 41 5f 5d c3 48 c7 c6 68 46 a1 81 48 89 c7 e8 1f e7 e4 ff <0f> 
0b 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 
[70129.672692] RIP  [<ffffffff81398811>] skcipher_pull_sgl+0x171/0x180
[70129.672709]  RSP <ffff88004f5c7d38>
[70129.672726] ---[ end trace e16e0f71a7a79252 ]---

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] crypto: qce: Initialize core src clock @100Mhz
From: Iaroslav Gridin @ 2016-09-07 17:25 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: herbert, davem, linux-crypto, linux-kernel, andy.gross,
	david.brown, linux-arm-msm, linux-soc
In-Reply-To: <4e509050-6e8e-069c-f00d-eca9a0f3b33d@mm-sol.com>

 
> > +	ret = clk_set_rate(qce->core_src, 100000000);
> 
> Could you point me from where you got this number? 

I got it from codeaurora qce driver:
https://android.googlesource.com/kernel/msm/+/android-msm-hammerhead-3.4-kk-r1/drivers/crypto/msm/qce50.c#3386

^ permalink raw reply


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