* [PATCH] hw_random: core, sleep interruptible in read
@ 2015-11-26 19:56 Jiri Slaby
2015-11-27 13:06 ` Herbert Xu
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2015-11-26 19:56 UTC (permalink / raw)
To: herbert; +Cc: linux-kernel, Jiri Slaby, Matt Mackall, linux-crypto
hwrng kthread can be waiting via hwrng_fillfn for some data from a rng
like virtio-rng:
hwrng D ffff880093e17798 0 382 2 0x00000000
...
Call Trace:
[<ffffffff817339c6>] wait_for_completion_killable+0x96/0x210
[<ffffffffa00aa1b7>] virtio_read+0x57/0xf0 [virtio_rng]
[<ffffffff814f4a35>] hwrng_fillfn+0x75/0x130
[<ffffffff810aa243>] kthread+0xf3/0x110
And when some user program tries to read the /dev node in this state,
we get:
rngd D ffff880093e17798 0 762 1 0x00000004
...
Call Trace:
[<ffffffff817351ac>] mutex_lock_nested+0x15c/0x3e0
[<ffffffff814f478e>] rng_dev_read+0x6e/0x240
[<ffffffff81231958>] __vfs_read+0x28/0xe0
[<ffffffff81232393>] vfs_read+0x83/0x130
And this is indeed unkillable. So use mutex_lock_interruptible
instead of mutex_lock in rng_dev_read and exit immediatelly when
interrupted. And possibly return already read data, if any (as POSIX
allows).
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Matt Mackall <mpm@selenic.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: <linux-crypto@vger.kernel.org>
---
drivers/char/hw_random/core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index a064237ff362..f003df162e09 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -238,7 +238,10 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
goto out;
}
- mutex_lock(&reading_mutex);
+ if (mutex_lock_interruptible(&reading_mutex)) {
+ err = -EINTR;
+ goto out_put;
+ }
if (!data_avail) {
bytes_read = rng_get_data(rng, rng_buffer,
rng_buffer_size(),
@@ -288,6 +291,7 @@ out:
out_unlock_reading:
mutex_unlock(&reading_mutex);
+out_put:
put_rng(rng);
goto out;
}
--
2.6.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] hw_random: core, sleep interruptible in read
2015-11-26 19:56 [PATCH] hw_random: core, sleep interruptible in read Jiri Slaby
@ 2015-11-27 13:06 ` Herbert Xu
2015-11-27 15:40 ` Jiri Slaby
2015-11-27 15:50 ` [PATCH v2] " Jiri Slaby
0 siblings, 2 replies; 5+ messages in thread
From: Herbert Xu @ 2015-11-27 13:06 UTC (permalink / raw)
To: Jiri Slaby; +Cc: linux-kernel, Matt Mackall, linux-crypto
On Thu, Nov 26, 2015 at 08:56:29PM +0100, Jiri Slaby wrote:
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index a064237ff362..f003df162e09 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -238,7 +238,10 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> goto out;
> }
>
> - mutex_lock(&reading_mutex);
> + if (mutex_lock_interruptible(&reading_mutex)) {
> + err = -EINTR;
Shouldn't this be ERESTARTSYS?
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] 5+ messages in thread
* Re: [PATCH] hw_random: core, sleep interruptible in read
2015-11-27 13:06 ` Herbert Xu
@ 2015-11-27 15:40 ` Jiri Slaby
2015-11-27 15:50 ` [PATCH v2] " Jiri Slaby
1 sibling, 0 replies; 5+ messages in thread
From: Jiri Slaby @ 2015-11-27 15:40 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-kernel, Matt Mackall, linux-crypto
On 11/27/2015, 02:06 PM, Herbert Xu wrote:
> On Thu, Nov 26, 2015 at 08:56:29PM +0100, Jiri Slaby wrote:
>>
>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>> index a064237ff362..f003df162e09 100644
>> --- a/drivers/char/hw_random/core.c
>> +++ b/drivers/char/hw_random/core.c
>> @@ -238,7 +238,10 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
>> goto out;
>> }
>>
>> - mutex_lock(&reading_mutex);
>> + if (mutex_lock_interruptible(&reading_mutex)) {
>> + err = -EINTR;
>
> Shouldn't this be ERESTARTSYS?
Yes, it actually can, given the retval is returned only if no data were
read yet.
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] hw_random: core, sleep interruptible in read
2015-11-27 13:06 ` Herbert Xu
2015-11-27 15:40 ` Jiri Slaby
@ 2015-11-27 15:50 ` Jiri Slaby
2015-12-04 14:40 ` Herbert Xu
1 sibling, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2015-11-27 15:50 UTC (permalink / raw)
To: herbert; +Cc: linux-kernel, Jiri Slaby, Matt Mackall, linux-crypto
hwrng kthread can be waiting via hwrng_fillfn for some data from a rng
like virtio-rng:
hwrng D ffff880093e17798 0 382 2 0x00000000
...
Call Trace:
[<ffffffff817339c6>] wait_for_completion_killable+0x96/0x210
[<ffffffffa00aa1b7>] virtio_read+0x57/0xf0 [virtio_rng]
[<ffffffff814f4a35>] hwrng_fillfn+0x75/0x130
[<ffffffff810aa243>] kthread+0xf3/0x110
And when some user program tries to read the /dev node in this state,
we get:
rngd D ffff880093e17798 0 762 1 0x00000004
...
Call Trace:
[<ffffffff817351ac>] mutex_lock_nested+0x15c/0x3e0
[<ffffffff814f478e>] rng_dev_read+0x6e/0x240
[<ffffffff81231958>] __vfs_read+0x28/0xe0
[<ffffffff81232393>] vfs_read+0x83/0x130
And this is indeed unkillable. So use mutex_lock_interruptible
instead of mutex_lock in rng_dev_read and exit immediatelly when
interrupted. And possibly return already read data, if any (as POSIX
allows).
v2: use ERESTARTSYS instead of EINTR
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Matt Mackall <mpm@selenic.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: <linux-crypto@vger.kernel.org>
---
drivers/char/hw_random/core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index a064237ff362..ca5a5325eb6f 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -238,7 +238,10 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
goto out;
}
- mutex_lock(&reading_mutex);
+ if (mutex_lock_interruptible(&reading_mutex)) {
+ err = -ERESTARTSYS;
+ goto out_put;
+ }
if (!data_avail) {
bytes_read = rng_get_data(rng, rng_buffer,
rng_buffer_size(),
@@ -288,6 +291,7 @@ out:
out_unlock_reading:
mutex_unlock(&reading_mutex);
+out_put:
put_rng(rng);
goto out;
}
--
2.6.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] hw_random: core, sleep interruptible in read
2015-11-27 15:50 ` [PATCH v2] " Jiri Slaby
@ 2015-12-04 14:40 ` Herbert Xu
0 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2015-12-04 14:40 UTC (permalink / raw)
To: Jiri Slaby; +Cc: linux-kernel, Matt Mackall, linux-crypto
On Fri, Nov 27, 2015 at 04:50:43PM +0100, Jiri Slaby wrote:
> hwrng kthread can be waiting via hwrng_fillfn for some data from a rng
> like virtio-rng:
> hwrng D ffff880093e17798 0 382 2 0x00000000
> ...
> Call Trace:
> [<ffffffff817339c6>] wait_for_completion_killable+0x96/0x210
> [<ffffffffa00aa1b7>] virtio_read+0x57/0xf0 [virtio_rng]
> [<ffffffff814f4a35>] hwrng_fillfn+0x75/0x130
> [<ffffffff810aa243>] kthread+0xf3/0x110
>
> And when some user program tries to read the /dev node in this state,
> we get:
> rngd D ffff880093e17798 0 762 1 0x00000004
> ...
> Call Trace:
> [<ffffffff817351ac>] mutex_lock_nested+0x15c/0x3e0
> [<ffffffff814f478e>] rng_dev_read+0x6e/0x240
> [<ffffffff81231958>] __vfs_read+0x28/0xe0
> [<ffffffff81232393>] vfs_read+0x83/0x130
>
> And this is indeed unkillable. So use mutex_lock_interruptible
> instead of mutex_lock in rng_dev_read and exit immediatelly when
> interrupted. And possibly return already read data, if any (as POSIX
> allows).
>
> v2: use ERESTARTSYS instead of EINTR
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Matt Mackall <mpm@selenic.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: <linux-crypto@vger.kernel.org>
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] 5+ messages in thread
end of thread, other threads:[~2015-12-04 14:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-26 19:56 [PATCH] hw_random: core, sleep interruptible in read Jiri Slaby
2015-11-27 13:06 ` Herbert Xu
2015-11-27 15:40 ` Jiri Slaby
2015-11-27 15:50 ` [PATCH v2] " Jiri Slaby
2015-12-04 14:40 ` Herbert Xu
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).