Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] random: Use wait_event_freezable() in add_hwgenerator_randomness()
@ 2019-09-05 16:41 Stephen Boyd
  2019-09-09  7:54 ` Herbert Xu
  0 siblings, 1 reply; 2+ messages in thread
From: Stephen Boyd @ 2019-09-05 16:41 UTC (permalink / raw)
  To: Herbert Xu
  Cc: linux-kernel, Theodore Ts'o, linux-crypto,
	Sebastian Andrzej Siewior, Keerthy

Sebastian reports that after commit ff296293b353 ("random: Support freezable
kthreads in add_hwgenerator_randomness()") we can call might_sleep() when the
task state is TASK_INTERRUPTIBLE (state=1). This leads to the following warning.

 do not call blocking ops when !TASK_RUNNING; state=1 set at [<00000000349d1489>] prepare_to_wait_event+0x5a/0x180
 WARNING: CPU: 0 PID: 828 at kernel/sched/core.c:6741 __might_sleep+0x6f/0x80
 Modules linked in:

 CPU: 0 PID: 828 Comm: hwrng Not tainted 5.3.0-rc7-next-20190903+ #46
 RIP: 0010:__might_sleep+0x6f/0x80

 Call Trace:
  kthread_freezable_should_stop+0x1b/0x60
  add_hwgenerator_randomness+0xdd/0x130
  hwrng_fillfn+0xbf/0x120
  kthread+0x10c/0x140
  ret_from_fork+0x27/0x50

We shouldn't call kthread_freezable_should_stop() from deep within the
wait_event code because the task state is still set as
TASK_INTERRUPTIBLE instead of TASK_RUNNING and
kthread_freezable_should_stop() will try to call into the freezer with
the task in the wrong state. Use wait_event_freezable() instead so that
it calls schedule() in the right place and tries to enter the freezer
when the task state is TASK_RUNNING instead.

Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Keerthy <j-keerthy@ti.com>
Fixes: ff296293b353 ("random: Support freezable kthreads in add_hwgenerator_randomness()")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

See https://lkml.kernel.org/r/20190904110038.2bx25byitrejlteu@flow for
context on the bug report.

 drivers/char/random.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 9b54cdb301d3..d3beed084c0a 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -327,6 +327,7 @@
 #include <linux/percpu.h>
 #include <linux/cryptohash.h>
 #include <linux/fips.h>
+#include <linux/freezer.h>
 #include <linux/ptrace.h>
 #include <linux/workqueue.h>
 #include <linux/irq.h>
@@ -2429,7 +2430,6 @@ void add_hwgenerator_randomness(const char *buffer, size_t count,
 				size_t entropy)
 {
 	struct entropy_store *poolp = &input_pool;
-	bool frozen = false;
 
 	if (unlikely(crng_init == 0)) {
 		crng_fast_load(buffer, count);
@@ -2440,13 +2440,11 @@ void add_hwgenerator_randomness(const char *buffer, size_t count,
 	 * We'll be woken up again once below random_write_wakeup_thresh,
 	 * or when the calling thread is about to terminate.
 	 */
-	wait_event_interruptible(random_write_wait,
-			kthread_freezable_should_stop(&frozen) ||
+	wait_event_freezable(random_write_wait,
+			kthread_should_stop() ||
 			ENTROPY_BITS(&input_pool) <= random_write_wakeup_bits);
-	if (!frozen) {
-		mix_pool_bytes(poolp, buffer, count);
-		credit_entropy_bits(poolp, entropy);
-	}
+	mix_pool_bytes(poolp, buffer, count);
+	credit_entropy_bits(poolp, entropy);
 }
 EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
 
-- 
Sent by a computer through tubes


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

* Re: [PATCH] random: Use wait_event_freezable() in add_hwgenerator_randomness()
  2019-09-05 16:41 [PATCH] random: Use wait_event_freezable() in add_hwgenerator_randomness() Stephen Boyd
@ 2019-09-09  7:54 ` Herbert Xu
  0 siblings, 0 replies; 2+ messages in thread
From: Herbert Xu @ 2019-09-09  7:54 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-kernel, Theodore Ts'o, linux-crypto,
	Sebastian Andrzej Siewior, Keerthy

On Thu, Sep 05, 2019 at 09:41:12AM -0700, Stephen Boyd wrote:
> Sebastian reports that after commit ff296293b353 ("random: Support freezable
> kthreads in add_hwgenerator_randomness()") we can call might_sleep() when the
> task state is TASK_INTERRUPTIBLE (state=1). This leads to the following warning.
> 
>  do not call blocking ops when !TASK_RUNNING; state=1 set at [<00000000349d1489>] prepare_to_wait_event+0x5a/0x180
>  WARNING: CPU: 0 PID: 828 at kernel/sched/core.c:6741 __might_sleep+0x6f/0x80
>  Modules linked in:
> 
>  CPU: 0 PID: 828 Comm: hwrng Not tainted 5.3.0-rc7-next-20190903+ #46
>  RIP: 0010:__might_sleep+0x6f/0x80
> 
>  Call Trace:
>   kthread_freezable_should_stop+0x1b/0x60
>   add_hwgenerator_randomness+0xdd/0x130
>   hwrng_fillfn+0xbf/0x120
>   kthread+0x10c/0x140
>   ret_from_fork+0x27/0x50
> 
> We shouldn't call kthread_freezable_should_stop() from deep within the
> wait_event code because the task state is still set as
> TASK_INTERRUPTIBLE instead of TASK_RUNNING and
> kthread_freezable_should_stop() will try to call into the freezer with
> the task in the wrong state. Use wait_event_freezable() instead so that
> it calls schedule() in the right place and tries to enter the freezer
> when the task state is TASK_RUNNING instead.
> 
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Keerthy <j-keerthy@ti.com>
> Fixes: ff296293b353 ("random: Support freezable kthreads in add_hwgenerator_randomness()")
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> 
> See https://lkml.kernel.org/r/20190904110038.2bx25byitrejlteu@flow for
> context on the bug report.
> 
>  drivers/char/random.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)

Patch applied.  Thanks.
-- 
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	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05 16:41 [PATCH] random: Use wait_event_freezable() in add_hwgenerator_randomness() Stephen Boyd
2019-09-09  7:54 ` Herbert Xu

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org linux-crypto@archiver.kernel.org
	public-inbox-index linux-crypto


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/ public-inbox