Mercurial > gnulib
changeset 39699:263fb4b417fe
random_r: do not crash if state is unaligned
Problem reported by Bruce Korb in:
https://lists.gnu.org/r/bug-gnulib/2018-06/msg00030.html
I reproduced the crash on 32-bit sparc with Oracle Studio 12.6
with 'cc -O2 -xmemalign=8s'.
* lib/random_r.c: Include string.h, for memcpy.
(get_int32, set_int32): New functions.
(__srandom_r, __initstate_r, __setstate_r, __random_r):
Use them to avoid assumption that state pointer is aligned.
(__random_r): Avoid integer overflow if INT_MAX == UINT32_MAX.
* tests/test-random_r.c (test_failed): New function.
(main): Use it, to test for alignment bugs.
author | Paul Eggert <eggert@cs.ucla.edu> |
---|---|
date | Thu, 21 Jun 2018 12:28:34 -0700 |
parents | fe94383b9591 |
children | 997df1305be8 |
files | ChangeLog lib/random_r.c tests/test-random_r.c |
diffstat | 3 files changed, 64 insertions(+), 24 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog Thu Jun 21 11:25:53 2018 -0700 +++ b/ChangeLog Thu Jun 21 12:28:34 2018 -0700 @@ -1,5 +1,18 @@ 2018-06-21 Paul Eggert <eggert@cs.ucla.edu> + random_r: do not crash if state is unaligned + Problem reported by Bruce Korb in: + https://lists.gnu.org/r/bug-gnulib/2018-06/msg00030.html + I reproduced the crash on 32-bit sparc with Oracle Studio 12.6 + with 'cc -O2 -xmemalign=8s'. + * lib/random_r.c: Include string.h, for memcpy. + (get_int32, set_int32): New functions. + (__srandom_r, __initstate_r, __setstate_r, __random_r): + Use them to avoid assumption that state pointer is aligned. + (__random_r): Avoid integer overflow if INT_MAX == UINT32_MAX. + * tests/test-random_r.c (test_failed): New function. + (main): Use it, to test for alignment bugs. + random_r: omit unnecessary include * lib/random_r.c: Do not include limits.h.
--- a/lib/random_r.c Thu Jun 21 11:25:53 2018 -0700 +++ b/lib/random_r.c Thu Jun 21 12:28:34 2018 -0700 @@ -69,6 +69,7 @@ #include <errno.h> #include <stddef.h> +#include <string.h> /* An improved random number generation package. In addition to the standard @@ -160,7 +161,19 @@ { DEG_0, DEG_1, DEG_2, DEG_3, DEG_4 } }; +static int32_t +get_int32 (void *p) +{ + int32_t v; + memcpy (&v, p, sizeof v); + return v; +} +static void +set_int32 (void *p, int32_t v) +{ + memcpy (p, &v, sizeof v); +} /* Initialize the random number generator based on the given seed. If the @@ -191,7 +204,7 @@ /* We must make sure the seed is not 0. Take arbitrarily 1 in this case. */ if (seed == 0) seed = 1; - state[0] = seed; + set_int32 (&state[0], seed); if (type == TYPE_0) goto done; @@ -208,7 +221,7 @@ word = 16807 * lo - 2836 * hi; if (word < 0) word += 2147483647; - *++dst = word; + set_int32 (++dst, word); } buf->fptr = &state[buf->rand_sep]; @@ -251,10 +264,10 @@ if (old_state != NULL) { int old_type = buf->rand_type; - if (old_type == TYPE_0) - old_state[-1] = TYPE_0; - else - old_state[-1] = (MAX_TYPES * (buf->rptr - old_state)) + old_type; + set_int32 (&old_state[-1], + (old_type == TYPE_0 + ? TYPE_0 + : (MAX_TYPES * (buf->rptr - old_state)) + old_type)); } int type; @@ -284,9 +297,8 @@ __srandom_r (seed, buf); - state[-1] = TYPE_0; - if (type != TYPE_0) - state[-1] = (buf->rptr - state) * MAX_TYPES + type; + set_int32 (&state[-1], + type == TYPE_0 ? TYPE_0 : (buf->rptr - state) * MAX_TYPES + type); return 0; @@ -320,12 +332,12 @@ old_type = buf->rand_type; old_state = buf->state; - if (old_type == TYPE_0) - old_state[-1] = TYPE_0; - else - old_state[-1] = (MAX_TYPES * (buf->rptr - old_state)) + old_type; + set_int32 (&old_state[-1], + (old_type == TYPE_0 + ? TYPE_0 + : (MAX_TYPES * (buf->rptr - old_state)) + old_type)); - type = new_state[-1] % MAX_TYPES; + type = get_int32 (&new_state[-1]) % MAX_TYPES; if (type < TYPE_0 || type > TYPE_4) goto fail; @@ -335,7 +347,7 @@ if (type != TYPE_0) { - int rear = new_state[-1] / MAX_TYPES; + int rear = get_int32 (&new_state[-1]) / MAX_TYPES; buf->rptr = &new_state[rear]; buf->fptr = &new_state[(rear + separation) % degree]; } @@ -375,8 +387,9 @@ if (buf->rand_type == TYPE_0) { - int32_t val = ((state[0] * 1103515245U) + 12345U) & 0x7fffffff; - state[0] = val; + int32_t val = (((get_int32 (&state[0]) * 1103515245U) + 12345U) + & 0x7fffffff); + set_int32 (&state[0], val); *result = val; } else @@ -384,9 +397,12 @@ int32_t *fptr = buf->fptr; int32_t *rptr = buf->rptr; int32_t *end_ptr = buf->end_ptr; - uint32_t val; - - val = *fptr += (uint32_t) *rptr; + /* F and R are unsigned int, not uint32_t, to avoid undefined + overflow behavior on platforms where INT_MAX == UINT32_MAX. */ + unsigned int f = get_int32 (fptr); + unsigned int r = get_int32 (rptr); + uint32_t val = f + r; + set_int32 (fptr, val); /* Chucking least random bit. */ *result = val >> 1; ++fptr;
--- a/tests/test-random_r.c Thu Jun 21 11:25:53 2018 -0700 +++ b/tests/test-random_r.c Thu Jun 21 12:28:34 2018 -0700 @@ -29,16 +29,17 @@ #include "macros.h" -int -main () +static int +test_failed (int alignment) { struct random_data rand_state; - char buf[128]; + char buf[128 + sizeof (int32_t)]; unsigned int i; unsigned int n_big = 0; rand_state.state = NULL; - if (initstate_r (time (NULL), buf, sizeof buf, &rand_state)) + if (initstate_r (time (NULL), buf + alignment, sizeof buf - alignment, + &rand_state)) return 1; for (i = 0; i < 1000; i++) { @@ -52,3 +53,13 @@ /* Fail if none of the numbers were larger than RAND_MAX / 2. */ return !n_big; } + +int +main () +{ + int alignment; + for (alignment = 0; alignment < sizeof (int32_t); alignment++) + if (test_failed (alignment)) + return 1; + return 0; +}