Re: [PATCH] malloc: Fix for infinite loop in memalign/posix_memalign.

[Will Newton <will dot newton at linaro dot org>]

On 10 October 2013 13:26, Will Newton <will.newton@linaro.org> wrote:
>
> A very large alignment argument passed to mealign/posix_memalign
> causes _int_memalign to enter an infinite loop. Limit the maximum
> alignment value to the maximum representable power of two to
> prevent this from happening.
>
> Changelog:
>
> 2013-10-10  Will Newton  <will.newton@linaro.org>
>
>         [BZ #16038]
>         * malloc/hooks.c (memalign_check): Limit alignment to the
>         maximum representable power of two.
>         * malloc/malloc.c (__libc_memalign): Likewise.
>         * malloc/tst-memalign.c (do_test): Add test for very
>         large alignment values.
>         * malloc/tst-posix_memalign.c (do_test): Likewise.
> ---
>  malloc/hooks.c              |  8 ++++++++
>  malloc/malloc.c             |  8 ++++++++
>  malloc/tst-memalign.c       | 15 +++++++++++++++
>  malloc/tst-posix_memalign.c | 10 ++++++++++
>  4 files changed, 41 insertions(+)
>
> diff --git a/malloc/hooks.c b/malloc/hooks.c
> index 3f663bb..1dbe93f 100644
> --- a/malloc/hooks.c
> +++ b/malloc/hooks.c
> @@ -361,6 +361,14 @@ memalign_check(size_t alignment, size_t bytes, const void *caller)
>    if (alignment <= MALLOC_ALIGNMENT) return malloc_check(bytes, NULL);
>    if (alignment <  MINSIZE) alignment = MINSIZE;
>
> +  /* If the alignment is greater than SIZE_MAX / 2 + 1 it cannot be a
> +     power of 2 and will cause overflow in the check below.  */
> +  if (alignment > SIZE_MAX / 2 + 1)
> +    {
> +      __set_errno (EINVAL);
> +      return 0;
> +    }
> +
>    /* Check for overflow.  */
>    if (bytes > SIZE_MAX - alignment - MINSIZE)
>      {
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 2938234..f1949f0 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3026,6 +3026,14 @@ __libc_memalign(size_t alignment, size_t bytes)
>    /* Otherwise, ensure that it is at least a minimum chunk size */
>    if (alignment <  MINSIZE) alignment = MINSIZE;
>
> +  /* If the alignment is greater than SIZE_MAX / 2 + 1 it cannot be a
> +     power of 2 and will cause overflow in the check below.  */
> +  if (alignment > SIZE_MAX / 2 + 1)
> +    {
> +      __set_errno (EINVAL);
> +      return 0;
> +    }
> +
>    /* Check for overflow.  */
>    if (bytes > SIZE_MAX - alignment - MINSIZE)
>      {
> diff --git a/malloc/tst-memalign.c b/malloc/tst-memalign.c
> index 1c59752..b8eec9e 100644
> --- a/malloc/tst-memalign.c
> +++ b/malloc/tst-memalign.c
> @@ -70,6 +70,21 @@ do_test (void)
>
>    free (p);
>
> +  errno = 0;
> +
> +  /* Test to expose integer overflow in malloc internals from BZ #16038.  */
> +  p = memalign (-1, pagesize);
> +
> +  save = errno;
> +
> +  if (p != NULL)
> +    merror ("memalign (-1, pagesize) succeeded.");
> +
> +  if (p == NULL && save != EINVAL)
> +    merror ("memalign (-1, -pagesize) errno is not set correctly");
> +
> +  free (p);
> +
>    /* A zero-sized allocation should succeed with glibc, returning a
>       non-NULL value.  */
>    p = memalign (sizeof (void *), 0);
> diff --git a/malloc/tst-posix_memalign.c b/malloc/tst-posix_memalign.c
> index 27c0dd2..7f34e37 100644
> --- a/malloc/tst-posix_memalign.c
> +++ b/malloc/tst-posix_memalign.c
> @@ -65,6 +65,16 @@ do_test (void)
>
>    p = NULL;
>
> +  /* Test to expose integer overflow in malloc internals from BZ #16038.  */
> +  ret = posix_memalign (&p, -1, pagesize);
> +
> +  if (ret != EINVAL)
> +    merror ("posix_memalign (&p, -1, pagesize) succeeded.");
> +
> +  free (p);
> +
> +  p = NULL;
> +
>    /* A zero-sized allocation should succeed with glibc, returning zero
>       and setting p to a non-NULL value.  */
>    ret = posix_memalign (&p, sizeof (void *), 0);
> --
> 1.8.1.4
>

Ping?

-- 
Will Newton
Toolchain Working Group, Linaro