summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Coopersmith <alan.coopersmith@oracle.com>2023-03-03 14:55:19 -0800
committerAlan Coopersmith <alan.coopersmith@oracle.com>2023-03-07 00:06:24 +0000
commit4ece1c842a08c11c1f84b95355801d41cd8435b1 (patch)
treed6f8fb17a745b9635a837c78046ac325b0505413
parent392eb1cd5f2bdb186f0ff7f51abc4dd05ec13709 (diff)
Add XtReallocArray() for overflow checking of multiplied args
Uses reallocarray() if available, otherwise checks for overflow itself, if overflow is possible (i.e. in ILP32 & ILP64 environments, but not LP64 with 32-bit ints). Includes unit tests and XtMallocArray() helper macro. Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
-rw-r--r--COPYING2
-rw-r--r--configure.ac10
-rw-r--r--include/X11/Intrinsic.h20
-rw-r--r--man/XtMalloc.man25
-rw-r--r--src/Alloc.c52
-rw-r--r--test/Alloc.c167
-rw-r--r--test/Makefile.am3
7 files changed, 264 insertions, 15 deletions
diff --git a/COPYING b/COPYING
index 30ef492..c341e9d 100644
--- a/COPYING
+++ b/COPYING
@@ -39,7 +39,7 @@ DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
PERFORMANCE OF THIS SOFTWARE.
-Copyright (c) 1993, 2011, Oracle and/or its affiliates.
+Copyright (c) 1993, 2023, Oracle and/or its affiliates.
Permission is hereby granted, free of charge, to any person obtaining a
copy of this software and associated documentation files (the "Software"),
diff --git a/configure.ac b/configure.ac
index e0dbaa4..1da5532 100644
--- a/configure.ac
+++ b/configure.ac
@@ -22,13 +22,18 @@
#
# Initialize Autoconf
-AC_PREREQ([2.60])
+AC_PREREQ([2.70])
AC_INIT([libXt], [1.2.1],
[https://gitlab.freedesktop.org/xorg/lib/libXt/issues], [libXt])
AC_CONFIG_SRCDIR([Makefile.am])
AC_CONFIG_HEADERS([config.h])
AC_CONFIG_MACRO_DIR([m4])
+# Set common system defines for POSIX extensions, such as _GNU_SOURCE
+# Must be called before any macros that run the compiler (like AC_PROG_LIBTOOL)
+# to avoid autoconf errors.
+AC_USE_SYSTEM_EXTENSIONS
+
# Initialize Automake
AM_INIT_AUTOMAKE([foreign dist-xz])
@@ -53,6 +58,9 @@ XORG_WITH_PERL
# Checks for header files.
AC_CHECK_HEADER([alloca.h], AC_DEFINE(INCLUDE_ALLOCA_H, 1, [Define to 1 if Xalloca.h should include <alloca.h>]))
+# Checks for library functions.
+AC_CHECK_FUNCS([reallocarray])
+
# Obtain compiler/linker options for dependencies
PKG_CHECK_MODULES(XT, sm ice x11 xproto kbproto)
diff --git a/include/X11/Intrinsic.h b/include/X11/Intrinsic.h
index 668b7d5..71199e7 100644
--- a/include/X11/Intrinsic.h
+++ b/include/X11/Intrinsic.h
@@ -1864,6 +1864,12 @@ extern char *XtRealloc(
Cardinal /* num */
);
+extern void *XtReallocArray(
+ void * /* ptr */,
+ Cardinal /* num */,
+ Cardinal /* size */
+);
+
extern void XtFree(
char* /* ptr */
);
@@ -1892,6 +1898,14 @@ extern char *_XtRealloc( /* implementation-private */
int /* line */
);
+extern char *_XtReallocArray( /* implementation-private */
+ char * /* ptr */,
+ Cardinal /* num */,
+ Cardinal /* size */,
+ const char */* file */,
+ int /* line */
+);
+
extern char *_XtCalloc( /* implementation-private */
Cardinal /* num */,
Cardinal /* size */,
@@ -1911,9 +1925,15 @@ extern void _XtPrintMemory( /* implementation-private */
#define XtMalloc(size) _XtMalloc(size, __FILE__, __LINE__)
#define XtRealloc(ptr,size) _XtRealloc(ptr, size, __FILE__, __LINE__)
+#define XtMallocArray(num,size) _XtReallocArray(NULL, num, size, __FILE__, __LINE__)
+#define XtReallocArray(ptr,num,size) _XtReallocArray(ptr, num, size, __FILE__, __LINE__)
#define XtCalloc(num,size) _XtCalloc(num, size, __FILE__, __LINE__)
#define XtFree(ptr) _XtFree(ptr)
+#else
+
+#define XtMallocArray(num,size) XtReallocArray(NULL, num, size)
+
#endif /* ifdef XTTRACEMEMORY */
#define XtNew(type) ((type *) XtMalloc((unsigned) sizeof(type)))
diff --git a/man/XtMalloc.man b/man/XtMalloc.man
index 2fa1d57..bdf580b 100644
--- a/man/XtMalloc.man
+++ b/man/XtMalloc.man
@@ -36,7 +36,7 @@
.na
.TH XtMalloc __libmansuffix__ __xorgversion__ "XT FUNCTIONS"
.SH NAME
-XtMalloc, XtCalloc, XtRealloc, XtFree, XtNew, XtNewString \- memory management functions
+XtMalloc, XtCalloc, XtRealloc, XtReallocArray, XtFree, XtNew, XtNewString \- memory management functions
.SH SYNTAX
#include <X11/Intrinsic.h>
.HP
@@ -44,7 +44,9 @@ char *XtMalloc(Cardinal \fIsize\fP);
.HP
char *XtCalloc(Cardinal \fInum\fP, Cardinal \fIsize\fP);
.HP
-char *XtRealloc(char *\fIptr\fP, Cardinal \fInum\fP);
+char *XtRealloc(char *\fIptr\fP, Cardinal \fIsize\fP);
+.HP
+void *XtReallocArray(void *\fIptr\fP, Cardinal \fInum\fP, Cardinal \fIsize\fP);
.HP
void XtFree(char *\fIptr\fP);
.HP
@@ -55,7 +57,7 @@ String XtNewString(String \fIstring\fP);
Cardinal XtAsprintf(char **\fInew_string\fP, const char *\fIformat\fP, ...);
.SH ARGUMENTS
.IP \fInum\fP 1i
-Specifies the number of bytes or array elements.
+Specifies the number of array elements.
.IP \fIptr\fP 1i
Specifies a pointer to the old storage or to the block of storage that is to be freed.
.IP \fIsize\fP 1i
@@ -90,17 +92,18 @@ calls
.LP
The
.B XtRealloc
-function changes the size of a block of storage (possibly moving it).
-Then, it copies the old contents (or as much as will fit) into the new block
-and frees the old block.
+and
+.B XtReallocArray
+functions change the size of a block of storage (possibly moving it).
+Then, they copy the old contents (or as much as will fit) into the new block
+and free the old block.
If there is insufficient memory to allocate the new block,
-.B XtRealloc
-calls
+or the calculations for the size of the new block would cause an
+integer overflow, these functions call
.BR XtErrorMsg .
If ptr is NULL,
-.B XtRealloc
-allocates the new storage without copying the old contents;
-that is, it simply calls
+these functions allocate the new storage without copying the old contents;
+that is, they simply call
.BR XtMalloc .
.LP
The
diff --git a/src/Alloc.c b/src/Alloc.c
index ce0594c..5bb27ce 100644
--- a/src/Alloc.c
+++ b/src/Alloc.c
@@ -1,5 +1,5 @@
/***********************************************************
-Copyright (c) 1993, 2011, Oracle and/or its affiliates.
+Copyright (c) 1993, 2023, Oracle and/or its affiliates.
Permission is hereby granted, free of charge, to any person obtaining a
copy of this software and associated documentation files (the "Software"),
@@ -84,6 +84,7 @@ in this Software without prior written authorization from The Open Group.
#include <stdlib.h>
#include <stdio.h>
#include <stdarg.h>
+#include <limits.h>
#define Xmalloc(size) malloc((size))
#define Xrealloc(ptr, size) realloc((ptr), (size))
@@ -197,6 +198,38 @@ XtRealloc(char *ptr, unsigned size)
return (ptr);
}
+void *
+XtReallocArray(void *ptr, unsigned num, unsigned size)
+{
+ if (ptr == NULL) {
+#ifdef MALLOC_0_RETURNS_NULL
+ if ((num == 0) || (size == 0))
+ num = size = 1;
+#endif
+#if (SIZE_MAX / UINT_MAX) <= UINT_MAX
+ if ((num > 0) && (SIZE_MAX / num) < size)
+ _XtAllocError("reallocarray: overflow detected");
+#endif
+ return (XtMalloc(num * size));
+ }
+ else {
+ void *new;
+
+#ifdef HAVE_REALLOCARRAY
+ new = reallocarray(ptr, size, num);
+#else
+# if (SIZE_MAX / UINT_MAX) <= UINT_MAX
+ if ((num > 0) && ((SIZE_MAX / num) < size))
+ _XtAllocError("reallocarray: overflow detected");
+# endif
+ new = Xrealloc(ptr, size * num);
+#endif
+ if ((new == NULL) && (num != 0) && (size != 0))
+ _XtAllocError("reallocarray");
+ return (new);
+ }
+}
+
char *
XtCalloc(unsigned num, unsigned size)
{
@@ -402,6 +435,23 @@ XtRealloc(char *ptr, unsigned size)
return _XtRealloc(ptr, size, (char *) NULL, 0);
}
+void *
+_XtReallocArray(void *ptr, unsigned num, unsigned size, const char *file, int line)
+{
+#if (SIZE_MAX / UINT_MAX) <= UINT_MAX
+ if ((num > 0) && (SIZE_MAX / num) < size)
+ _XtAllocError("reallocarray: overflow");
+#endif
+
+ return _XtRealloc(ptr, (num * size), file, line);
+}
+
+void *
+XtReallocArray(void *ptr, unsigned num, unsigned size)
+{
+ return _XtReallocArray(ptr, num, size, (char *) NULL, 0);
+}
+
char *
_XtCalloc(unsigned num, unsigned size, const char *file, int line)
{
diff --git a/test/Alloc.c b/test/Alloc.c
index 3120f0f..03f44a8 100644
--- a/test/Alloc.c
+++ b/test/Alloc.c
@@ -569,6 +569,166 @@ static void test_XtRealloc_overflow(void)
}
+/* Make sure XtReallocArray() works for a non-zero amount of memory */
+static void test_XtReallocArray_normal(void)
+{
+ void *p, *p2;
+ char *p3;
+
+ errno = 0;
+
+ /* Make sure reallocarray with a NULL pointer acts as malloc */
+ p = XtReallocArray(NULL, 8, 14);
+ g_assert_nonnull(p);
+ CHECK_ALIGNMENT(p);
+ CHECK_SIZE(p, 8 * 14);
+
+ /* make sure we can write to all the allocated memory */
+ memset(p, 'A', 8 * 14);
+
+ /* create another block after the first */
+ p2 = XtMalloc(73);
+ g_assert_nonnull(p2);
+
+ /* now resize the first */
+ p3 = XtReallocArray(p, 73, 14);
+ g_assert_nonnull(p3);
+ CHECK_ALIGNMENT(p3);
+ CHECK_SIZE(p3, 73 * 14);
+ /* verify previous values are still present */
+ for (int i = 0; i < (8 * 14); i++) {
+ g_assert_cmpint(p3[i], ==, 'A');
+ }
+
+ XtFree(p3);
+ XtFree(p2);
+ g_assert_cmpint(errno, ==, 0);
+}
+
+/* Make sure XtReallocArray(0) returns a valid pointer as expected */
+static void test_XtReallocArray_zero(void)
+{
+ void *p, *p2;
+
+ errno = 0;
+
+ p = XtReallocArray(NULL, 0, 0);
+ g_assert_nonnull(p);
+
+ p2 = XtReallocArray(p, 0, 0);
+#ifdef MALLOC_0_RETURNS_NULL
+ g_assert_null(p);
+#else
+ g_assert_nonnull(p);
+#endif
+
+ XtFree(p2);
+ g_assert_cmpint(errno, ==, 0);
+}
+
+/* Make sure sizes larger than the limit we set in main() fail */
+static void test_XtReallocArray_oversize(void)
+{
+ void *p, *p2;
+
+ /* Pick a number of elements between 1 & 16K */
+ guint32 num = g_test_rand_int_range(1, (16 * 1024));
+ /* Pick a size between 1 & 16K */
+ guint32 size = g_test_rand_int_range(1, (16 * 1024));
+
+ p = XtReallocArray(NULL, num, size);
+ g_assert_nonnull(p);
+ CHECK_ALIGNMENT(p);
+ CHECK_SIZE(p, num * size);
+
+ if (setjmp(jmp_env) == 0) {
+ p2 = XtReallocArray(p, 2, ALLOC_LIMIT);
+ g_assert_null(p2);
+ } else {
+ /*
+ * We jumped here from error handler as expected.
+ * We cannot verify errno here, as the Xt error handler makes
+ * calls that override errno, when trying to load error message
+ * files from different locations.
+ */
+ }
+
+ errno = 0;
+ XtFree(p);
+ g_assert_cmpint(errno, ==, 0);
+}
+
+/* Make sure XtReallocArray catches integer overflow if possible, by requesting
+ * sizes that are so large that they cause overflows when either adding the
+ * reallocarray data block overhead or aligning.
+ *
+ * Testing integer overflow cases is limited by the fact that XtReallocArray
+ * only takes unsigned arguments (typically 32-bit), and relies on
+ * the underlying libc reallocarray to catch overflow, which can't occur if
+ * 32-bit arguments are passed to a function taking 64-bit args.
+ */
+static void test_XtReallocArray_overflow(void)
+{
+#if UINT_MAX < SIZE_MAX
+ g_test_skip("overflow not possible in this config");
+#else
+ void *p, *p2;
+
+ /* Pick a number of elements between 1 & 16K */
+ guint32 num = g_test_rand_int_range(1, (16 * 1024));
+ /* Pick a size between 1 & 16K */
+ guint32 size = g_test_rand_int_range(1, (16 * 1024));
+
+ p = XtReallocArray(NULL, num, size);
+ g_assert_nonnull(p);
+ CHECK_ALIGNMENT(p);
+ CHECK_SIZE(p, num * size);
+
+ if (setjmp(jmp_env) == 0) {
+ p2 = XtReallocArray(p, 1, SIZE_MAX);
+ g_assert_null(p2);
+ } else {
+ /*
+ * We jumped here from error handler as expected.
+ * We cannot verify errno here, as the Xt error handler makes
+ * calls that override errno, when trying to load error message
+ * files from different locations.
+ */
+ }
+
+ if (setjmp(jmp_env) == 0) {
+ /* SQRT_SIZE_MAX * SQRT_SIZE_MAX == 0 due to overflow */
+ p2 = XtReallocArray(p, SQRT_SIZE_MAX, SQRT_SIZE_MAX);
+ g_assert_null(p2);
+ } else {
+ /*
+ * We jumped here from error handler as expected.
+ * We cannot verify errno here, as the Xt error handler makes
+ * calls that override errno, when trying to load error message
+ * files from different locations.
+ */
+ }
+
+ if (setjmp(jmp_env) == 0) {
+ /* Overflows to a small positive number */
+ p2 = XtReallocArray(p, SQRT_SIZE_MAX + 1, SQRT_SIZE_MAX);
+ g_assert_null(p2);
+ } else {
+ /*
+ * We jumped here from error handler as expected.
+ * We cannot verify errno here, as the Xt error handler makes
+ * calls that override errno, when trying to load error message
+ * files from different locations.
+ */
+ }
+
+ errno = 0;
+ XtFree(p);
+ g_assert_cmpint(errno, ==, 0);
+#endif
+}
+
+
int main(int argc, char** argv)
{
struct rlimit lim;
@@ -606,5 +766,12 @@ int main(int argc, char** argv)
g_test_add_func("/Alloc/XtRealloc/oversize", test_XtRealloc_oversize);
g_test_add_func("/Alloc/XtRealloc/overflow", test_XtRealloc_overflow);
+ g_test_add_func("/Alloc/XtReallocArray/normal", test_XtReallocArray_normal);
+ g_test_add_func("/Alloc/XtReallocArray/zero", test_XtReallocArray_zero);
+ g_test_add_func("/Alloc/XtReallocArray/oversize",
+ test_XtReallocArray_oversize);
+ g_test_add_func("/Alloc/XtReallocArray/overflow",
+ test_XtReallocArray_overflow);
+
return g_test_run();
}
diff --git a/test/Makefile.am b/test/Makefile.am
index adc7060..00229fb 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -5,7 +5,8 @@ check_PROGRAMS = Alloc Converters Event
TESTS=$(check_PROGRAMS)
AM_CFLAGS = $(CWARNFLAGS) $(XT_CFLAGS) $(GLIB_CFLAGS)
-AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include
+AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include \
+ -I$(top_builddir)/include/X11
LDADD= $(top_builddir)/src/libXt.la $(GLIB_LIBS)
TESTS_ENVIRONMENT = $(MALLOC_DEBUG_ENV)