This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v6] Add reallocarray function.
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Tue, 30 May 2017 12:17:07 -0300
- Subject: Re: [PATCH v6] Add reallocarray function.
- Authentication-results: sourceware.org; auth=none
- References: <CAKCAbMj46tfbXaVitKisjf_27=N4qwnkuV08a-qMfpzo45Qgxg@mail.gmail.com> <20170522202515.19374-1-denniswoelfing@gmx.de>
On 22/05/2017 17:25, Dennis Wölfing wrote:
> The reallocarray function is an extension from OpenBSD. It is an
> integer-overflow-safe replacement for realloc(p, X*Y) and
> malloc(X*Y) (realloc(NULL, X*Y)). It can therefore help in preventing
> certain security issues in code.
>
> This is an updated version of a patch originally submitted by Rüdiger
> Sonderfeld in May 2014.
> See <https://sourceware.org/ml/libc-alpha/2014-05/msg00481.html>.
>
> Tested on i686 and x86_64.
>
> 2017-05-22 Dennis Wölfing <denniswoelfing@gmx.de>
> Rüdiger Sonderfeld <ruediger@c-plusplus.de>
>
> * include/stdlib.h (__libc_reallocarray): New declaration.
> * malloc/Makefile (routines): Add reallocarray.
> (tests): Add tst-reallocarray.c.
> * malloc/Versions: Add reallocarray and __libc_reallocarray.
> * malloc/malloc-internal.h (check_mul_overflow_size_t): New inline
> function.
> * malloc/malloc.h (reallocarray): New declaration.
> * stdlib/stdlib.h (reallocarray): Likewise.
> * malloc/reallocarray.c: New file.
> * malloc/tst-reallocarray.c: New test file.
> * manual/memory.texi: Document reallocarray.
> * sysdeps/unix/sysv/linux/aarch64/libc.abilist: Add reallocarray.
> * sysdeps/unix/sysv/linux/alpha/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/arm/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/hppa/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/i386/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/ia64/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/microblaze/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/nios2/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist:
> Likewise.
> * sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist:
> Likewise.
> * sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist: Likewise.
> * sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/sh/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/tilepro/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/x86_64/64/libc.abilist: Likewise.
> * sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist: Likewise.
LGTM with some nits below.
> diff --git a/NEWS b/NEWS
> index b4ecd6201d..e70bff01db 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -66,6 +66,8 @@ Version 2.26
> * The port to Native Client running on ARMv7-A (--host=arm-nacl) has been
> removed.
>
> +* The reallocarray function has been added to libc.
> +
> Security related changes:
I would extend it a bit by describing what reallocarray is intended to
(maybe something as 'It is a realloc replacement with a check for integer
overflow when calculating total allocation size').
> diff --git a/malloc/tst-reallocarray.c b/malloc/tst-reallocarray.c
> new file mode 100644
> index 0000000000..e914e2938b
> --- /dev/null
> +++ b/malloc/tst-reallocarray.c
> @@ -0,0 +1,119 @@
> +/* Test for reallocarray.
> + Copyright (C) 2014-2017 Free Software Foundation, Inc.
It is a new file, so use 2017 instead.
> + 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; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <malloc.h>
> +#include <string.h>
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +
Extra line.
> diff --git a/manual/memory.texi b/manual/memory.texi
> index a256ca07b2..fb6b594ef1 100644
> --- a/manual/memory.texi
> +++ b/manual/memory.texi
> @@ -751,8 +751,8 @@ be a buffer that you use to hold a line being read from a file; no matter
> how long you make the buffer initially, you may encounter a line that is
> longer.
>
> -You can make the block longer by calling @code{realloc}. This function
> -is declared in @file{stdlib.h}.
> +You can make the block longer by calling @code{realloc} or
> +@code{reallocarray}. These functions are declared in @file{stdlib.h}.
> @pindex stdlib.h
>
> @comment malloc.h stdlib.h
> @@ -816,9 +816,29 @@ behavior, and will probably crash when @code{realloc} is passed a null
> pointer.
> @end deftypefun
>
> -Like @code{malloc}, @code{realloc} may return a null pointer if no
> -memory space is available to make the block bigger. When this happens,
> -the original block is untouched; it has not been modified or relocated.
> +@comment malloc.h stdlib.h
> +@comment BSD
> +@deftypefun {void *} reallocarray (void *@var{ptr}, size_t @var{nmemb}, size_t @var{size})
> +@safety{@prelim{}@mtsafe{}@asunsafe{@asulock{}}@acunsafe{@aculock{} @acsfd{} @acsmem{}}}
> +
> +The @code{reallocarray} function changes the size of the block whose address
> +is @var{ptr} to be long enough to contain a vector of @var{nmemb} elements,
> +each of size @var{size}. It is equivalent to @samp{realloc (@var{ptr},
> +@var{nmemb} * @var{size})}, except that @code{reallocarray} fails safely if
> +the multiplication overflows, by setting @code{errno} to @code{ENOMEM},
> +returning a null pointer, and leaving the original block unchanged.
> +
> +@code{reallocarray} should be used instead of @code{realloc} when the new size
> +of the allocated block is the result of a multiplication that might overflow.
> +
> +@strong{Portability Note:} This function is not part of any standard. It was
> +first introduced in OpenBSD 5.6.
> +@end deftypefun
I think it is worth to add FreeBSD 11.0 also supports it.