| Message ID | 1351408746-8623-1-git-send-email-akinobu.mita@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Oct 28, 2012 at 04:18:58PM +0900, Akinobu Mita wrote: > /** > + * prandom32_get_bytes - get the requested number of pseudo-random bytes > + * @state: pointer to state structure holding seeded state. > + * @buf: where to copy the pseudo-random bytes to > + * @bytes: the requested number of bytes > + * > + * This is used for pseudo-randomness with no outside seeding. > + * For more random results, use random32_get_bytes(). > + */ > + > +/** > + * random32_get_bytes - get the requested number of pseudo-random bytes > + * @buf: where to copy the pseudo-random bytes to > + * @bytes: the requested number of bytes > + */ This naming scheme is going to be very confusing. If the function is going to return a pseudo-random number, it *must* have a "prandom" suffix. Otherwise some kernel developer, somewhere, will get confused between get_random_bytes() and random32_get_bytes(), and the result may be a very embarassing security exposure. How about prandom32_get_bytes_state() and prandom32_get_bytes() instead? - Ted
2012/10/30 Theodore Ts'o <tytso@mit.edu>: > On Sun, Oct 28, 2012 at 04:18:58PM +0900, Akinobu Mita wrote: >> /** >> + * prandom32_get_bytes - get the requested number of pseudo-random bytes >> + * @state: pointer to state structure holding seeded state. >> + * @buf: where to copy the pseudo-random bytes to >> + * @bytes: the requested number of bytes >> + * >> + * This is used for pseudo-randomness with no outside seeding. >> + * For more random results, use random32_get_bytes(). >> + */ >> + >> +/** >> + * random32_get_bytes - get the requested number of pseudo-random bytes >> + * @buf: where to copy the pseudo-random bytes to >> + * @bytes: the requested number of bytes >> + */ > > This naming scheme is going to be very confusing. If the function is > going to return a pseudo-random number, it *must* have a "prandom" > suffix. Otherwise some kernel developer, somewhere, will get confused > between get_random_bytes() and random32_get_bytes(), and the result > may be a very embarassing security exposure. > > How about prandom32_get_bytes_state() and prandom32_get_bytes() instead? I agree with your suggestion. I'll rename them and try again. By the way, should we also rename the existing random32() and prandom32() in the future? Specifically, rename random32() to prandom32(), and prandom32() to prandom32_state(). As a result, it will cause a little confusion between old and new prandom32(). But the number of arguments will be changed from 3 to 2, so gcc can detect the misuse of prandom32(). Is there any other idea of renaming? Or should we keep them as is?
2012/10/30 Akinobu Mita <akinobu.mita@gmail.com>: > 2012/10/30 Theodore Ts'o <tytso@mit.edu>: >> On Sun, Oct 28, 2012 at 04:18:58PM +0900, Akinobu Mita wrote: >>> /** >>> + * prandom32_get_bytes - get the requested number of pseudo-random bytes >>> + * @state: pointer to state structure holding seeded state. >>> + * @buf: where to copy the pseudo-random bytes to >>> + * @bytes: the requested number of bytes >>> + * >>> + * This is used for pseudo-randomness with no outside seeding. >>> + * For more random results, use random32_get_bytes(). >>> + */ >>> + >>> +/** >>> + * random32_get_bytes - get the requested number of pseudo-random bytes >>> + * @buf: where to copy the pseudo-random bytes to >>> + * @bytes: the requested number of bytes >>> + */ >> >> This naming scheme is going to be very confusing. If the function is >> going to return a pseudo-random number, it *must* have a "prandom" >> suffix. Otherwise some kernel developer, somewhere, will get confused >> between get_random_bytes() and random32_get_bytes(), and the result >> may be a very embarassing security exposure. >> >> How about prandom32_get_bytes_state() and prandom32_get_bytes() instead? > > I agree with your suggestion. I'll rename them and try again. > > By the way, should we also rename the existing random32() and > prandom32() in the future? > > Specifically, rename random32() to prandom32(), and prandom32() to > prandom32_state(). As a result, it will cause a little confusion > between old and new prandom32(). But the number of arguments will > be changed from 3 to 2, so gcc can detect the misuse of prandom32(). Oops, I intended to say "the number of arguments of prandom32() will be changed from 1 to 0". And I realized that the exisiting srandom32() also should be renamed to sprandom32().
On Tue, Oct 30, 2012 at 08:12:39PM +0900, Akinobu Mita wrote: > >> > >> How about prandom32_get_bytes_state() and prandom32_get_bytes() instead? > > > > I agree with your suggestion. I'll rename them and try again. > > > > By the way, should we also rename the existing random32() and > > prandom32() in the future? I suppose the other way to go is to just use random32 as the common prefix, and just have random32() and random32_state(). My concern was that people might assume that prandom32() and random32() might imply that only prandom32() was the one using a pseudo-random number generator. This might be easier since there are large number of uses of random32() in the source tree, but only a relative few using prandom32(). - Ted
2012/10/31 Theodore Ts'o <tytso@mit.edu>: > On Tue, Oct 30, 2012 at 08:12:39PM +0900, Akinobu Mita wrote: >> >> >> >> How about prandom32_get_bytes_state() and prandom32_get_bytes() instead? >> > >> > I agree with your suggestion. I'll rename them and try again. >> > >> > By the way, should we also rename the existing random32() and >> > prandom32() in the future? > > I suppose the other way to go is to just use random32 as the common > prefix, and just have random32() and random32_state(). My concern was > that people might assume that prandom32() and random32() might imply > that only prandom32() was the one using a pseudo-random number > generator. This might be easier since there are large number of uses > of random32() in the source tree, but only a relative few using > prandom32(). Using random32 as the common prefix sounds good idea. I'm going to prepare the following patch set: [patch 1] rename prandom32 to randome32_state [patch 2] introduce random32_get_bytes and random32_get_bytes_state [patch 3-] proliferate random32_get_bytes and random32_get_bytes_state
diff --git a/include/linux/random.h b/include/linux/random.h index 6330ed4..eedf429b 100644 --- a/include/linux/random.h +++ b/include/linux/random.h @@ -27,8 +27,10 @@ unsigned long randomize_range(unsigned long start, unsigned long end, unsigned l u32 random32(void); void srandom32(u32 seed); +void random32_get_bytes(void *buf, int nbytes); u32 prandom32(struct rnd_state *); +void prandom32_get_bytes(struct rnd_state *state, void *buf, int nbytes); /* * Handle minimum values for seeds diff --git a/lib/random32.c b/lib/random32.c index 938bde5..aee87dd 100644 --- a/lib/random32.c +++ b/lib/random32.c @@ -61,6 +61,44 @@ u32 prandom32(struct rnd_state *state) EXPORT_SYMBOL(prandom32); /** + * prandom32_get_bytes - get the requested number of pseudo-random bytes + * @state: pointer to state structure holding seeded state. + * @buf: where to copy the pseudo-random bytes to + * @bytes: the requested number of bytes + * + * This is used for pseudo-randomness with no outside seeding. + * For more random results, use random32_get_bytes(). + */ +void prandom32_get_bytes(struct rnd_state *state, void *buf, int bytes) +{ + unsigned char *p = buf; + + for (; bytes > 0 && ((unsigned long)p) % sizeof(u32); bytes--, p++) + *p = prandom32(state); + + for (; bytes > sizeof(u32) - 1; bytes -= sizeof(u32), p += sizeof(u32)) + *(u32 *)p = prandom32(state); + + for (; bytes > 0; bytes--, p++) + *p = prandom32(state); +} +EXPORT_SYMBOL(prandom32_get_bytes); + +/** + * random32_get_bytes - get the requested number of pseudo-random bytes + * @buf: where to copy the pseudo-random bytes to + * @bytes: the requested number of bytes + */ +void random32_get_bytes(void *buf, int bytes) +{ + struct rnd_state *state = &get_cpu_var(net_rand_state); + + prandom32_get_bytes(state, buf, bytes); + put_cpu_var(state); +} +EXPORT_SYMBOL(random32_get_bytes); + +/** * random32 - pseudo random number generator * * A 32 bit pseudo-random number is generated using a fast
Add functions to get the requested number of pseudo-random bytes. The difference from get_random_bytes() is that it uses pseudo-random numbers generated by random32. It is fast, suitable for generating random bytes for testing, and reproducible if prandom32 interface is used. Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Cc: "Theodore Ts'o" <tytso@mit.edu> Cc: Artem Bityutskiy <dedekind1@gmail.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: David Woodhouse <dwmw2@infradead.org> Cc: linux-mtd@lists.infradead.org --- include/linux/random.h | 2 ++ lib/random32.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+)