Re: [tytso-DPNOqEs/LNQ@public.gmane.org: [PATCH, RFC -v3] random: introduce getrandom(2) system call]
From: Arnd Bergmann <hidden>
Date: 2014-07-18 13:16:18
On Friday 18 July 2014 08:56:06 Theodore Ts'o wrote:
The change in the v3 version of the commit was to eliminate potential short reads and EINTR returns when reading from urandom (once the urandom pool is initialized). This was based on comments and requests from Theo de Raadt. See the NOTES section in the suggested man page for a more in-depth discussion of the issues involved.
I think there is a problem with the completion:
quoted hunk
@@ -657,6 +661,7 @@ retry: r->entropy_total = 0; if (r == &nonblocking_pool) { prandom_reseed_late(); + complete_all(&urandom_initialized); pr_notice("random: %s pool is initialized\n", r->name); } }
complete_all() sets the 'done' part of the completion to 'UINT_MAX/2', which is enough to ensure that everyone waiting for the completion at a given time will wake up, but a completion should not be reused without calling init_completion again after a call to complete_all().
quoted hunk
+SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count, + unsigned int, flags) +{ + int r; + + if (flags & ~(GRND_NONBLOCK|GRND_RANDOM)) + return -EINVAL; + + if (count > INT_MAX) + count = INT_MAX; + + if (flags & GRND_RANDOM) + return _random_read(flags & GRND_NONBLOCK, buf, count); + if (flags & GRND_NONBLOCK) { + if (!completion_done(&urandom_initialized)) + return -EAGAIN; + } else { + r = wait_for_completion_interruptible(&urandom_initialized); + if (r) + return r; + } + return urandom_read(NULL, buf, count, NULL); +} +
However, here you can get called an arbitrary number of times. It seems entirely possible than an attacker can manage to call this function 2 billion times. Assuming a latency of 1 microsecond per syscall, that would take about half an hour. After that, you never again get any urandom data out of the syscall. I think you are better off using a plain wait_event() here. Arnd