[PATCH v2] stdlib: random_r: fix unaligned access in initstate and initstate_r [BZ #30584]
Sam James
sam@gentoo.org
Mon Dec 30 16:34:01 GMT 2024
Sam James <sam@gentoo.org> writes:
> Sam James <sam@gentoo.org> writes:
>
>> The initstate{,_r} interfaces are documented in BSD as needing an aligned
>> array of 32-bit values, but neither POSIX nor glibc's own documentation
>> require it to be aligned. glibc's documentation says it "should" be a power
>> of 2, but not must.
>>
>> Use memcpy to read and write to `state` to handle such an unaligned
>> argument.
>>
>> Co-authored-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> ---
>> v2: Use _Alignas in test to ensure we construct a (mis)aligned pointer.
>> Fix indentation.
>> Fix comment style.
>>
>> OK? I have some more changes to make but they're a little bit more invasive
>> and need to be posted and tested separately.
>
> Florian, is this one OK? Thanks.
Ping. Sorry, I'd just really like to get this one in for 2.41 and be
able to move on to the refactoring for this code.
>
>>
>> stdlib/Makefile | 1 +
>> stdlib/random_r.c | 40 +++++++++++++++++++++++++++----------
>> stdlib/tst-random-bz30584.c | 38 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 68 insertions(+), 11 deletions(-)
>> create mode 100644 stdlib/tst-random-bz30584.c
>>
>> diff --git a/stdlib/Makefile b/stdlib/Makefile
>> index 370cfa57aa..715446970f 100644
>> --- a/stdlib/Makefile
>> +++ b/stdlib/Makefile
>> @@ -299,6 +299,7 @@ tests := \
>> tst-rand48-2 \
>> tst-random \
>> tst-random2 \
>> + tst-random-bz30584 \
>> tst-realpath \
>> tst-realpath-toolong \
>> tst-secure-getenv \
>> diff --git a/stdlib/random_r.c b/stdlib/random_r.c
>> index b6297fe099..044d72b2c3 100644
>> --- a/stdlib/random_r.c
>> +++ b/stdlib/random_r.c
>> @@ -55,6 +55,7 @@
>> #include <limits.h>
>> #include <stddef.h>
>> #include <stdlib.h>
>> +#include <string.h>
>>
>>
>> /* An improved random number generation package. In addition to the standard
>> @@ -146,7 +147,19 @@ static const struct random_poly_info random_poly_info =
>> { DEG_0, DEG_1, DEG_2, DEG_3, DEG_4 }
>> };
>>
>> +static inline int32_t
>> +read_state (int32_t *b, size_t idx)
>> +{
>> + int32_t r;
>> + memcpy (&r, &b[idx], sizeof (int32_t));
>> + return r;
>> +}
>>
>> +static inline void
>> +write_state (int32_t *b, size_t idx, int32_t v)
>> +{
>> + memcpy (&b[idx], &v, sizeof (int32_t));
>> +}
>>
>>
>> /* Initialize the random number generator based on the given seed. If the
>> @@ -177,7 +190,7 @@ __srandom_r (unsigned int seed, struct random_data *buf)
>> /* We must make sure the seed is not 0. Take arbitrarily 1 in this case. */
>> if (seed == 0)
>> seed = 1;
>> - state[0] = seed;
>> + write_state (state, 0, seed);
>> if (type == TYPE_0)
>> goto done;
>>
>> @@ -194,7 +207,7 @@ __srandom_r (unsigned int seed, struct random_data *buf)
>> word = 16807 * lo - 2836 * hi;
>> if (word < 0)
>> word += 2147483647;
>> - *++dst = word;
>> + write_state (++dst, 0, word);
>> }
>>
>> buf->fptr = &state[buf->rand_sep];
>> @@ -238,9 +251,10 @@ __initstate_r (unsigned int seed, char *arg_state, size_t n,
>> {
>> int old_type = buf->rand_type;
>> if (old_type == TYPE_0)
>> - old_state[-1] = TYPE_0;
>> + write_state (old_state, -1, TYPE_0);
>> else
>> - old_state[-1] = (MAX_TYPES * (buf->rptr - old_state)) + old_type;
>> + write_state (old_state, -1, (MAX_TYPES * (buf->rptr - old_state))
>> + + old_type);
>> }
>>
>> int type;
>> @@ -270,9 +284,9 @@ __initstate_r (unsigned int seed, char *arg_state, size_t n,
>>
>> __srandom_r (seed, buf);
>>
>> - state[-1] = TYPE_0;
>> + write_state (state, -1, TYPE_0);
>> if (type != TYPE_0)
>> - state[-1] = (buf->rptr - state) * MAX_TYPES + type;
>> + write_state (state, -1, (buf->rptr - state) * MAX_TYPES + type);
>>
>> return 0;
>>
>> @@ -307,9 +321,10 @@ __setstate_r (char *arg_state, struct random_data *buf)
>> old_type = buf->rand_type;
>> old_state = buf->state;
>> if (old_type == TYPE_0)
>> - old_state[-1] = TYPE_0;
>> + write_state (old_state, -1, TYPE_0);
>> else
>> - old_state[-1] = (MAX_TYPES * (buf->rptr - old_state)) + old_type;
>> + write_state (old_state, -1, (MAX_TYPES * (buf->rptr - old_state))
>> + + old_type);
>>
>> type = new_state[-1] % MAX_TYPES;
>> if (type < TYPE_0 || type > TYPE_4)
>> @@ -361,8 +376,9 @@ __random_r (struct random_data *buf, int32_t *result)
>>
>> if (buf->rand_type == TYPE_0)
>> {
>> - int32_t val = ((state[0] * 1103515245U) + 12345U) & 0x7fffffff;
>> - state[0] = val;
>> + int32_t val = ((read_state(state, 0) * 1103515245U) + 12345U)
>> + & 0x7fffffff;
>> + write_state (state, 0, val);
>> *result = val;
>> }
>> else
>> @@ -372,7 +388,9 @@ __random_r (struct random_data *buf, int32_t *result)
>> int32_t *end_ptr = buf->end_ptr;
>> uint32_t val;
>>
>> - val = *fptr += (uint32_t) *rptr;
>> + val = read_state (rptr, 0);
>> + int32_t t = read_state (fptr, 0);
>> + write_state (fptr, 0, t + val);
>> /* Chucking least random bit. */
>> *result = val >> 1;
>> ++fptr;
>> diff --git a/stdlib/tst-random-bz30584.c b/stdlib/tst-random-bz30584.c
>> new file mode 100644
>> index 0000000000..0a9b013f26
>> --- /dev/null
>> +++ b/stdlib/tst-random-bz30584.c
>> @@ -0,0 +1,38 @@
>> +/* Test program for initstate(), initstate_r() for BZ #30584.
>> + Copyright (C) 2024 Free Software Foundation, Inc.
>> + This file is part of the GNU C Library.
>> +
>> + The GNU C Library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public License as
>> + published by the Free Software Foundation; either version 2.1 of the
>> + License, or (at your option) any later version.
>> +
>> + The GNU C Library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with the GNU C Library; see the file COPYING.LIB. If
>> + not, see <https://www.gnu.org/licenses/>. */
>> +
>> +#include <stdlib.h>
>> +#include <time.h>
>> +
>> +static int
>> +do_test (void)
>> +{
>> + struct random_data rand_state = { .state = NULL };
>> + _Alignas (double) char buf[128 + sizeof (int32_t)];
>> +
>> + /* Test initstate_r with an unaligned `state` array. */
>> + initstate_r (time (NULL), buf + 1, sizeof buf, &rand_state);
>> +
>> + /* Ditto initstate. */
>> + initstate (time (NULL), buf + 1, sizeof buf);
>> +
>> + return 0;
>> +}
>> +
>> +#define TEST_FUNCTION do_test ()
>> +#include "../test-skeleton.c"
More information about the Libc-alpha
mailing list