10880 posix_memalign() should avoid clobbering return parameter on failure

Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Gergő Doma <domag02@gmail.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Dan McDonald <danmcd@joyent.com>

5 files changed, 116 insertions, 13 deletions

diff --git a/usr/src/lib/libc/port/gen/posix_memalign.c b/usr/src/lib/libc/port/gen/posix_memalign.c
index d05ac2d9ea..131e0d01b8 100644
--- a/usr/src/lib/libc/port/gen/posix_memalign.c
+++ b/usr/src/lib/libc/port/gen/posix_memalign.c
@@ -24,9 +24,10 @@
  * Use is subject to license terms.
  */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include "lint.h"
+
+#include <sys/sysmacros.h>
+
 #include <stdlib.h>
 #include <errno.h>
 
@@ -49,16 +50,19 @@ int
 posix_memalign(void **memptr, size_t alignment, size_t size)
 {
 	void *ptr = NULL;
-	int error = 0;
-
-	if (alignment == 0 ||
-	    (alignment & (sizeof (void *) - 1)) != 0 ||
-	    (alignment & (alignment - 1)) != 0)
-		error = EINVAL;
-	else if (size != 0 &&
-	    (ptr = memalign(alignment, size)) == NULL)
-		error = ENOMEM;
 
-	*memptr = ptr;
-	return (error);
+	if ((alignment == 0) || !ISP2(alignment) ||
+	    (alignment & (sizeof (void *) - 1)) != 0) {
+		return (EINVAL);
+	} else if (size == 0) {
+		*memptr = NULL;
+		return (0);
+	} else {
+		if ((ptr = memalign(alignment, size)) == NULL) {
+			return (ENOMEM);
+		} else {
+			*memptr = ptr;
+			return (0);
+		}
+	}
 }
diff --git a/usr/src/pkg/manifests/system-test-libctest.mf b/usr/src/pkg/manifests/system-test-libctest.mf
index f01471e5bf..e322646bcb 100644
--- a/usr/src/pkg/manifests/system-test-libctest.mf
+++ b/usr/src/pkg/manifests/system-test-libctest.mf
@@ -100,6 +100,8 @@ file path=opt/libc-tests/tests/newlocale_test.$(ARCH64) mode=0555
 file path=opt/libc-tests/tests/nl_langinfo_test mode=0555
 file path=opt/libc-tests/tests/nl_langinfo_test.$(ARCH) mode=0555
 file path=opt/libc-tests/tests/nl_langinfo_test.$(ARCH64) mode=0555
+file path=opt/libc-tests/tests/posix_memalign.32 mode=0555
+file path=opt/libc-tests/tests/posix_memalign.64 mode=0555
 file path=opt/libc-tests/tests/printf-6961.64 mode=0555
 file path=opt/libc-tests/tests/printf-9511.32 mode=0555
 file path=opt/libc-tests/tests/printf-9511.64 mode=0555
diff --git a/usr/src/test/libc-tests/runfiles/default.run b/usr/src/test/libc-tests/runfiles/default.run
index cde5a5f8c8..fbc8be8d40 100644
--- a/usr/src/test/libc-tests/runfiles/default.run
+++ b/usr/src/test/libc-tests/runfiles/default.run
@@ -79,6 +79,8 @@ timeout = 600
 [/opt/libc-tests/tests/fnmatch.64]
 [/opt/libc-tests/tests/memset_s.32]
 [/opt/libc-tests/tests/memset_s.64]
+[/opt/libc-tests/tests/posix_memalign.32]
+[/opt/libc-tests/tests/posix_memalign.64]
 [/opt/libc-tests/tests/printf-6961.64]
 [/opt/libc-tests/tests/printf-9511.32]
 [/opt/libc-tests/tests/printf-9511.64]
diff --git a/usr/src/test/libc-tests/tests/Makefile b/usr/src/test/libc-tests/tests/Makefile
index c2a566e69c..d22d33f613 100644
--- a/usr/src/test/libc-tests/tests/Makefile
+++ b/usr/src/test/libc-tests/tests/Makefile
@@ -39,6 +39,7 @@ PROGS = \
 	env-7076 \
 	fnmatch \
 	memset_s \
+	posix_memalign \
 	printf-9511 \
 	psignal-5097 \
 	quick_exit_order \
@@ -66,6 +67,8 @@ PROGS64 = \
 
 aligned_alloc.32 :=	LDLIBS += -lproc
 aligned_alloc.64 :=	LDLIBS64 += -lproc
+posix_memalign.32 :=	LDLIBS += -lproc
+posix_memalign.64 :=	LDLIBS64 += -lproc
 
 memset_s.32 :=	CPPFLAGS += -D__STDC_WANT_LIB_EXT1__=1
 memset_s.64 :=	CPPFLAGS += -D__STDC_WANT_LIB_EXT1__=1
diff --git a/usr/src/test/libc-tests/tests/posix_memalign.c b/usr/src/test/libc-tests/tests/posix_memalign.c
new file mode 100644
index 0000000000..b35607c2bd
--- /dev/null
+++ b/usr/src/test/libc-tests/tests/posix_memalign.c
@@ -0,0 +1,92 @@
+/*
+ * This file and its contents are supplied under the terms of the
+ * Common Development and Distribution License ("CDDL"), version 1.0.
+ * You may only use this file in accordance with the terms of version
+ * 1.0 of the CDDL.
+ *
+ * A full copy of the text of the CDDL should have accompanied this
+ * source.  A copy of the CDDL is also available via the Internet at
+ * http://www.illumos.org/license/CDDL.
+ */
+
+/*
+ * Copyright 2016 Joyent, Inc.
+ */
+
+/*
+ * Basic tests for posix_memalign(3C). Note that we test ENOMEM failure by
+ * relying on the implementation of the current libc malloc. Specifically we go
+ * through and add a mapping so we can't expand the heap and then use it up. If
+ * the memory allocator is ever changed, this test will start failing, at which
+ * point, it may not be worth the cost of keeping it around.
+ */
+
+#include <stdlib.h>
+#include <errno.h>
+#include <libproc.h>
+#include <sys/sysmacros.h>
+#include <sys/mman.h>
+#include <sys/debug.h>
+
+int
+main(void)
+{
+	pstatus_t status;
+	/*
+	 * We use a non-NULL value, so we can verify that failure does not
+	 * change the value of 'buf'
+	 */
+	void *sentinel = (void *)0xbad00000;
+	void *buf = sentinel;
+	int err = 0;
+
+	/*
+	 * Alignment must be sizeof (void *) (word) aligned.
+	 */
+	err = posix_memalign(&buf, sizeof (void *) - 1, 16);
+	VERIFY3S(err, ==, EINVAL);
+	VERIFY3P(buf, ==, sentinel);
+
+	err = posix_memalign(&buf, sizeof (void *) + 1, 16);
+	VERIFY3S(err, ==, EINVAL);
+	VERIFY3P(buf, ==, sentinel);
+
+	err = posix_memalign(&buf, 23, 16);
+	VERIFY3S(err, ==, EINVAL);
+	VERIFY3P(buf, ==, sentinel);
+
+	err = posix_memalign(&buf, sizeof (void *), 16);
+	VERIFY3S(err, ==, 0);
+	VERIFY3B(IS_P2ALIGNED(buf, sizeof (void *)), ==, B_TRUE);
+	VERIFY3P(buf, !=, sentinel);
+	VERIFY3P(buf, !=, NULL);
+	free(buf);
+	buf = sentinel;
+
+	/*
+	 * Cause ENOMEM
+	 */
+	VERIFY0(proc_get_status(getpid(), &status));
+	VERIFY3P(mmap((caddr_t)P2ROUNDUP(status.pr_brkbase +
+	    status.pr_brksize, 0x1000), 0x1000,
+	    PROT_READ, MAP_ANON | MAP_FIXED | MAP_PRIVATE, -1, 0),
+	    !=, (void *)-1);
+
+	for (;;) {
+		if (malloc(16) == NULL)
+			break;
+	}
+
+
+	for (;;) {
+		if (posix_memalign(&buf, sizeof (void *), 16) == ENOMEM)
+			break;
+	}
+
+	buf = sentinel;
+	err = posix_memalign(&buf, sizeof (void *), 16);
+	VERIFY3S(err, ==, ENOMEM);
+	VERIFY3P(buf, ==, sentinel);
+
+	return (0);
+}