| Message ID | 1530713750-22453-1-git-send-email-stefan.bader@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,bionic] random: Make getrandom() ready earlier | expand |
On 04/07/18 15:15, Stefan Bader wrote: > From: Ben Hutchings <ben@decadent.org.uk> > > This effectively reverts commit 725e828 "random: fix crng_ready() > test" which was commit 43838a23a05f upstream. Unfortunately some > users of getrandom() don't expect it to block for long, and they need > to be fixed before we can allow this change into stable. > > This doesn't directly revert that commit, but only weakens the ready > condition used by getrandom() when the GRND_RANDOM flag is not set. > Calls to getrandom() that return before the RNG is fully seeded will > generate warnings, just like reads from /dev/urandom. > > https://bugs.launchpad.net/bugs/1780062 > > (backported from ://salsa.debian.org/kernel-team/linux/raw/stretch/debian/patches/debian/random-make-getrandom-ready-earlier.patch) > [smb: open code waiting in getrandom directly] > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > This is based on the patch from Ben Hutchings in Debian. But since > wait_for_random_bytes, which is used in the 4.15 code is exported > and used by other things, I did instead do its own wait in getrandom(). > > -Stefan > > drivers/char/random.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/random.c b/drivers/char/random.c > index d5f1211..6021405 100644 > --- a/drivers/char/random.c > +++ b/drivers/char/random.c > @@ -1976,10 +1976,10 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count, > if (flags & GRND_RANDOM) > return _random_read(flags & GRND_NONBLOCK, buf, count); > > - if (!crng_ready()) { > + if (crng_init == 0) { > if (flags & GRND_NONBLOCK) > return -EAGAIN; > - ret = wait_for_random_bytes(); > + ret = wait_event_interruptible(crng_init_wait, crng_init > 0); > if (unlikely(ret)) > return ret; > } > Looks good to me; this is a good workaround for now; fixes the issue on my X230 too, so I've tested this and it works. The bug link needs fixing up when this is applied. Thanks Stefan. Acked-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
On 2018-07-04 15:56:35 , Colin Ian King wrote: > On 04/07/18 15:15, Stefan Bader wrote: > > From: Ben Hutchings <ben@decadent.org.uk> > > > > This effectively reverts commit 725e828 "random: fix crng_ready() > > test" which was commit 43838a23a05f upstream. Unfortunately some > > users of getrandom() don't expect it to block for long, and they need > > to be fixed before we can allow this change into stable. > > > > This doesn't directly revert that commit, but only weakens the ready > > condition used by getrandom() when the GRND_RANDOM flag is not set. > > Calls to getrandom() that return before the RNG is fully seeded will > > generate warnings, just like reads from /dev/urandom. > > > > https://bugs.launchpad.net/bugs/1780062 > > > > (backported from ://salsa.debian.org/kernel-team/linux/raw/stretch/debian/patches/debian/random-make-getrandom-ready-earlier.patch) > > [smb: open code waiting in getrandom directly] > > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > > --- > > This is based on the patch from Ben Hutchings in Debian. But since > > wait_for_random_bytes, which is used in the 4.15 code is exported > > and used by other things, I did instead do its own wait in getrandom(). > > > > -Stefan > > > > drivers/char/random.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/char/random.c b/drivers/char/random.c > > index d5f1211..6021405 100644 > > --- a/drivers/char/random.c > > +++ b/drivers/char/random.c > > @@ -1976,10 +1976,10 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count, > > if (flags & GRND_RANDOM) > > return _random_read(flags & GRND_NONBLOCK, buf, count); > > > > - if (!crng_ready()) { > > + if (crng_init == 0) { > > if (flags & GRND_NONBLOCK) > > return -EAGAIN; > > - ret = wait_for_random_bytes(); > > + ret = wait_event_interruptible(crng_init_wait, crng_init > 0); > > if (unlikely(ret)) > > return ret; > > } > > > > Looks good to me; this is a good workaround for now; fixes the issue on > my X230 too, so I've tested this and it works. > > The bug link needs fixing up when this is applied. The buglink seems to work fine for me (just don't copy the closing parenthesis) > > Thanks Stefan. > > Acked-by: Colin Ian King <colin.king@canonical.com> > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 2018-07-04 11:12:36 , Khaled Elmously wrote: > On 2018-07-04 15:56:35 , Colin Ian King wrote: > > On 04/07/18 15:15, Stefan Bader wrote: > > > From: Ben Hutchings <ben@decadent.org.uk> > > > > > > This effectively reverts commit 725e828 "random: fix crng_ready() > > > test" which was commit 43838a23a05f upstream. Unfortunately some > > > users of getrandom() don't expect it to block for long, and they need > > > to be fixed before we can allow this change into stable. > > > > > > This doesn't directly revert that commit, but only weakens the ready > > > condition used by getrandom() when the GRND_RANDOM flag is not set. > > > Calls to getrandom() that return before the RNG is fully seeded will > > > generate warnings, just like reads from /dev/urandom. > > > > > > https://bugs.launchpad.net/bugs/1780062 > > > > > > (backported from ://salsa.debian.org/kernel-team/linux/raw/stretch/debian/patches/debian/random-make-getrandom-ready-earlier.patch) > > > [smb: open code waiting in getrandom directly] > > > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > > > --- > > > This is based on the patch from Ben Hutchings in Debian. But since > > > wait_for_random_bytes, which is used in the 4.15 code is exported > > > and used by other things, I did instead do its own wait in getrandom(). > > > > > > -Stefan > > > > > > drivers/char/random.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/char/random.c b/drivers/char/random.c > > > index d5f1211..6021405 100644 > > > --- a/drivers/char/random.c > > > +++ b/drivers/char/random.c > > > @@ -1976,10 +1976,10 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count, > > > if (flags & GRND_RANDOM) > > > return _random_read(flags & GRND_NONBLOCK, buf, count); > > > > > > - if (!crng_ready()) { > > > + if (crng_init == 0) { > > > if (flags & GRND_NONBLOCK) > > > return -EAGAIN; > > > - ret = wait_for_random_bytes(); > > > + ret = wait_event_interruptible(crng_init_wait, crng_init > 0); > > > if (unlikely(ret)) > > > return ret; > > > } > > > > > > > Looks good to me; this is a good workaround for now; fixes the issue on > > my X230 too, so I've tested this and it works. > > > > The bug link needs fixing up when this is applied. > > The buglink seems to work fine for me (just don't copy the closing > parenthesis) > Sorry, I misspoke. I meant the "patch link" works fine for me (though the bug link seems to work fine too) > > > > Thanks Stefan. > > > > Acked-by: Colin Ian King <colin.king@canonical.com> > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 07/04/18 16:15, Stefan Bader wrote: > From: Ben Hutchings <ben@decadent.org.uk> > > This effectively reverts commit 725e828 "random: fix crng_ready() > test" which was commit 43838a23a05f upstream. Unfortunately some > users of getrandom() don't expect it to block for long, and they need > to be fixed before we can allow this change into stable. > > This doesn't directly revert that commit, but only weakens the ready > condition used by getrandom() when the GRND_RANDOM flag is not set. > Calls to getrandom() that return before the RNG is fully seeded will > generate warnings, just like reads from /dev/urandom. > > https://bugs.launchpad.net/bugs/1780062 > > (backported from ://salsa.debian.org/kernel-team/linux/raw/stretch/debian/patches/debian/random-make-getrandom-ready-earlier.patch) > [smb: open code waiting in getrandom directly] > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > This is based on the patch from Ben Hutchings in Debian. But since > wait_for_random_bytes, which is used in the 4.15 code is exported > and used by other things, I did instead do its own wait in getrandom(). > > -Stefan > > drivers/char/random.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/random.c b/drivers/char/random.c > index d5f1211..6021405 100644 > --- a/drivers/char/random.c > +++ b/drivers/char/random.c > @@ -1976,10 +1976,10 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count, > if (flags & GRND_RANDOM) > return _random_read(flags & GRND_NONBLOCK, buf, count); > > - if (!crng_ready()) { > + if (crng_init == 0) { > if (flags & GRND_NONBLOCK) > return -EAGAIN; > - ret = wait_for_random_bytes(); > + ret = wait_event_interruptible(crng_init_wait, crng_init > 0); > if (unlikely(ret)) > return ret; > } > This patch has been applied to bionic/master-next branch. Thanks, Kleber
On Wed, Jul 04, 2018 at 04:15:50PM +0200, Stefan Bader wrote: > From: Ben Hutchings <ben@decadent.org.uk> > > This effectively reverts commit 725e828 "random: fix crng_ready() > test" which was commit 43838a23a05f upstream. Unfortunately some > users of getrandom() don't expect it to block for long, and they need > to be fixed before we can allow this change into stable. > > This doesn't directly revert that commit, but only weakens the ready > condition used by getrandom() when the GRND_RANDOM flag is not set. > Calls to getrandom() that return before the RNG is fully seeded will > generate warnings, just like reads from /dev/urandom. > > https://bugs.launchpad.net/bugs/1780062 > > (backported from ://salsa.debian.org/kernel-team/linux/raw/stretch/debian/patches/debian/random-make-getrandom-ready-earlier.patch) > [smb: open code waiting in getrandom directly] > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Applied to cosmic/master-next and unstable/master, thanks!
diff --git a/drivers/char/random.c b/drivers/char/random.c index d5f1211..6021405 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1976,10 +1976,10 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count, if (flags & GRND_RANDOM) return _random_read(flags & GRND_NONBLOCK, buf, count); - if (!crng_ready()) { + if (crng_init == 0) { if (flags & GRND_NONBLOCK) return -EAGAIN; - ret = wait_for_random_bytes(); + ret = wait_event_interruptible(crng_init_wait, crng_init > 0); if (unlikely(ret)) return ret; }