From cc98a5e2fd16ae8589deea9dbd71be59b6f77305 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 24 Apr 2021 17:59:53 -0700 Subject: [PATCH] xmalloca, etc.: fix some xalloc-oversized issues MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * lib/malloca.h (nmalloca): * lib/xmalloca.h (xnmalloca): Convert S to ptrdiff_t to avoid arithmetic overflow if N and S are both narrower than ptrdiff_t. * lib/xalloc-oversized.h (xalloc_oversized): Don’t say that args must be ptrdiff_t or size_t or wider. The macro returns the correct answer even when that is not the case, and it’s the caller’s responsibility to avoid howlers like (xalloc_oversized (n, s) ? NULL : malloc (n * s)) when N and S are both narrower than ptrdiff_t and size_t. Add a comment to that effect. * lib/xmalloca.h: Include xalloc-oversized.h, since this file uses xalloc_oversized. Add comments about side effects and avoid unnecessary parens. * modules/xmalloca (Depends-on): Add xalloc-oversized. --- ChangeLog | 16 ++++++++++++++++ lib/malloca.h | 4 ++-- lib/xalloc-oversized.h | 18 +++++++++++++----- lib/xmalloca.h | 10 ++++++---- modules/xmalloca | 1 + 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index c198830bf5..63471166c4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,21 @@ 2021-04-24 Paul Eggert + xmalloca, etc.: fix some xalloc-oversized issues + * lib/malloca.h (nmalloca): + * lib/xmalloca.h (xnmalloca): Convert S to ptrdiff_t to avoid + arithmetic overflow if N and S are both narrower than ptrdiff_t. + * lib/xalloc-oversized.h (xalloc_oversized): + Don’t say that args must be ptrdiff_t or size_t or wider. + The macro returns the correct answer even when that is not + the case, and it’s the caller’s responsibility to avoid + howlers like (xalloc_oversized (n, s) ? NULL : malloc (n * s)) + when N and S are both narrower than ptrdiff_t and size_t. + Add a comment to that effect. + * lib/xmalloca.h: Include xalloc-oversized.h, since this file uses + xalloc_oversized. Add comments about side effects and avoid + unnecessary parens. + * modules/xmalloca (Depends-on): Add xalloc-oversized. + reallocarray: check for ptrdiff_t overflow * doc/glibc-functions/reallocarray.texi (reallocarray): Mention ptrdiff_t overflow. diff --git a/lib/malloca.h b/lib/malloca.h index 16a156ba20..f9b30880d0 100644 --- a/lib/malloca.h +++ b/lib/malloca.h @@ -77,9 +77,9 @@ extern void freea (void *p); /* nmalloca(N,S) is an overflow-safe variant of malloca (N * S). It allocates an array of N objects, each with S bytes of memory, on the stack. S must be positive and N must be nonnegative. - Either N or S should be of type ptrdiff_t or size_t or wider. The array must be freed using freea() before the function returns. */ -#define nmalloca(n, s) (xalloc_oversized (n, s) ? NULL : malloca ((n) * (s))) +#define nmalloca(n, s) \ + (xalloc_oversized (n, s) ? NULL : malloca ((n) * (ptrdiff_t) (s))) #ifdef __cplusplus diff --git a/lib/xalloc-oversized.h b/lib/xalloc-oversized.h index 6437a5d8b1..62f1ae94b8 100644 --- a/lib/xalloc-oversized.h +++ b/lib/xalloc-oversized.h @@ -30,11 +30,19 @@ #define __xalloc_oversized(n, s) \ ((size_t) (PTRDIFF_MAX < SIZE_MAX ? PTRDIFF_MAX : SIZE_MAX - 1) / (s) < (n)) -/* Return 1 if an array of N objects, each of size S, cannot exist reliably - because its total size in bytes exceeds MIN (PTRDIFF_MAX, SIZE_MAX - 1). - N must be nonnegative, S must be positive, and either N or S should be - of type ptrdiff_t or size_t or wider. This is a macro, not a function, - so that it works even if an argument exceeds MAX (PTRDIFF_MAX, SIZE_MAX). */ +/* Return 1 if and only if an array of N objects, each of size S, + cannot exist reliably because its total size in bytes would exceed + MIN (PTRDIFF_MAX, SIZE_MAX - 1). + + N must be nonnegative and S must be positive. + + Warning: (xalloc_oversized (N, S) ? NULL : malloc (N * S)) can + misbehave if N and S are both narrower than ptrdiff_t and size_t, + and can be rewritten as (xalloc_oversized (N, S) ? NULL : malloc + (N * (ptrdiff_t) S)) or similarly with size_t. + + This is a macro, not a function, so that it works even if an + argument exceeds MAX (PTRDIFF_MAX, SIZE_MAX). */ #if 7 <= __GNUC__ && !defined __clang__ && PTRDIFF_MAX < SIZE_MAX # define xalloc_oversized(n, s) \ __builtin_mul_overflow_p (n, s, (ptrdiff_t) 1) diff --git a/lib/xmalloca.h b/lib/xmalloca.h index a87a336b62..c7815f6171 100644 --- a/lib/xmalloca.h +++ b/lib/xmalloca.h @@ -20,6 +20,7 @@ #include "malloca.h" #include "xalloc.h" +#include "xalloc-oversized.h" #ifdef __cplusplus @@ -29,7 +30,8 @@ extern "C" { /* xmalloca(N) is a checking safe variant of alloca(N). It allocates N bytes of memory allocated on the stack, that must be freed using freea() before - the function returns. Upon failure, it exits with an error message. */ + the function returns. N should not have side effects. + Upon failure, it exits with an error message. */ #if HAVE_ALLOCA # define xmalloca(N) \ ((N) < 4032 - (2 * sa_alignment_max - 1) \ @@ -46,16 +48,16 @@ extern void * xmmalloca (size_t n); /* xnmalloca(N,S) is an overflow-safe variant of xmalloca (N * S). It allocates an array of N objects, each with S bytes of memory, on the stack. S must be positive and N must be nonnegative, - and at least one of N and S should be ptrdiff_t or size_t or wider. + and S and N should not have side effects. The array must be freed using freea() before the function returns. Upon failure, it exits with an error message. */ #if HAVE_ALLOCA /* Rely on xmalloca (SIZE_MAX) calling xalloc_die (). */ # define xnmalloca(n, s) \ - xmalloca (xalloc_oversized ((n), (s)) ? (size_t) (-1) : (n) * (s)) + xmalloca (xalloc_oversized (n, s) ? (size_t) (-1) : (n) * (ptrdiff_t) (s)) #else # define xnmalloca(n, s) \ - xnmalloc ((n), (s)) + xnmalloc (n, s) #endif diff --git a/modules/xmalloca b/modules/xmalloca index dac01f2d22..92b965b82a 100644 --- a/modules/xmalloca +++ b/modules/xmalloca @@ -8,6 +8,7 @@ lib/xmalloca.c Depends-on: malloca xalloc +xalloc-oversized configure.ac: -- 2.39.5