OS-7651 umem_genasm needs to be stricter about segments

OS-7650 Clean up umem smatch and cerrwarn
Reviewed by: Cody Peter Mello <melloc@joyent.com>
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Approved by: Patrick Mooney <patrick.mooney@joyent.com>

9 files changed, 199 insertions, 59 deletions

diff --git a/usr/src/lib/libumem/Makefile.com b/usr/src/lib/libumem/Makefile.com
index b98977ed88..70bbb164a7 100644
--- a/usr/src/lib/libumem/Makefile.com
+++ b/usr/src/lib/libumem/Makefile.com
@@ -22,7 +22,7 @@
 # Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
 # Use is subject to license terms.
 #
-# Copyright (c) 2018, Joyent, Inc.
+# Copyright (c) 2019, Joyent, Inc.
 #
 
 #
@@ -51,11 +51,15 @@ OBJECTS_library = \
 	umem_agent_support.o \
 	umem_fail.o \
 	umem_fork.o \
+	umem_genasm.o \
 	umem_update_thread.o \
 	vmem_mmap.o \
 	vmem_sbrk.o
 
-SRCS_library = $(OBJECTS_library:%.o=../common/%.c)
+SRCS_common_library = \
+	$(ISASRCDIR)/umem_genasm.c
+
+SRCS_library = $(OBJECTS_library:%.o=../common/%.c) $(SRC_common_library)
 
 # Files specific to the standalone version of libumem
 OBJECTS_standalone = \
@@ -67,12 +71,10 @@ SRCS_standalone = $(OBJECTS_standalone:%.o=../common/%.c)
 
 # Architecture-dependent files common to both versions of libumem
 OBJECTS_common_isadep = \
-	asm_subr.o \
-	umem_genasm.o	
+	asm_subr.o
 
 SRCS_common_isadep = \
-	$(ISASRCDIR)/asm_subr.s \
-	$(ISASRCDIR)/umem_genasm.c
+	$(ISASRCDIR)/asm_subr.s
 
 # Architecture-independent files common to both versions  of libumem
 OBJECTS_common_common = \
@@ -96,7 +98,6 @@ include ../../Makefile.rootfs
 
 SRCS = \
 	$(SRCS_$(CURTYPE)) \
-	$(SRCS_common_isadep) \
 	$(SRCS_common_common)
 
 SRCDIR = ../common
@@ -127,11 +128,6 @@ ASFLAGS_standalone = -DUMEM_STANDALONE
 ASFLAGS_library =
 ASFLAGS += -P $(ASFLAGS_$(CURTYPE)) -D_ASM
 
-CERRWARN += -_gcc=-Wno-switch
-CERRWARN += -_gcc=-Wno-uninitialized
-
-SMOFF += deref_check
-
 $(LINTLIB) := SRCS = ../common/$(LINTSRC)
 
 # We want the thread-specific errno in the library, but we don't want it in
diff --git a/usr/src/lib/libumem/amd64/umem_genasm.c b/usr/src/lib/libumem/amd64/umem_genasm.c
index ba68cb2d37..6f0c72ab34 100644
--- a/usr/src/lib/libumem/amd64/umem_genasm.c
+++ b/usr/src/lib/libumem/amd64/umem_genasm.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright (c) 2013 Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2019 Joyent, Inc.  All rights reserved.
  */
 
 /*
@@ -62,12 +62,18 @@
  * Each block of assembly has psuedocode that describes its purpose.
  */
 
-#include <atomic.h>
+/*
+ * umem_base must be first.
+ */
+#include "umem_base.h"
+
 #include <inttypes.h>
-#include <sys/types.h>
 #include <strings.h>
 #include <umem_impl.h>
-#include "umem_base.h"
+#include <atomic.h>
+#include <sys/mman.h>
+#include <errno.h>
+
 
 const int umem_genasm_supported = 1;
 static uintptr_t umem_genasm_mptr = (uintptr_t)&_malloc;
@@ -543,20 +549,57 @@ genasm_free(void *base, size_t len, int nents, int *umem_alloc_sizes)
 	return (0);
 }
 
-/*ARGSUSED*/
-int
+boolean_t
 umem_genasm(int *cp, umem_cache_t **caches, int nc)
 {
 	int nents, i;
 	uint8_t *mptr;
 	uint8_t *fptr;
 	uint64_t v, *vptr;
+	size_t mplen, fplen;
+	uintptr_t mpbase, fpbase;
+	boolean_t ret = B_FALSE;
 
 	mptr = (void *)((uintptr_t)umem_genasm_mptr + 5);
 	fptr = (void *)((uintptr_t)umem_genasm_fptr + 5);
 	if (umem_genasm_mptr == 0 || umem_genasm_msize == 0 ||
-	    umem_genasm_fptr == 0 || umem_genasm_fsize == 0)
-		return (1);
+	    umem_genasm_fptr == 0 || umem_genasm_fsize == 0) {
+		return (B_FALSE);
+	}
+
+	mplen = P2ROUNDUP(umem_genasm_msize, pagesize);
+	mpbase = P2ALIGN((uintptr_t)umem_genasm_mptr, pagesize);
+	fplen = P2ROUNDUP(umem_genasm_fsize, pagesize);
+	fpbase = P2ALIGN((uintptr_t)umem_genasm_mptr, pagesize);
+
+	/*
+	 * If the values straddle a page boundary, then we might need to
+	 * actually remap two pages.
+	 */
+	if (P2ALIGN(umem_genasm_msize + (uintptr_t)umem_genasm_mptr,
+	    pagesize) != mpbase) {
+		mplen += pagesize;
+	}
+
+	if (P2ALIGN(umem_genasm_fsize + (uintptr_t)umem_genasm_fptr,
+	    pagesize) != fpbase) {
+		fplen += pagesize;
+	}
+
+	if (mprotect((void *)mpbase, mplen, PROT_READ | PROT_WRITE |
+	    PROT_EXEC) != 0) {
+		return (B_FALSE);
+	}
+
+	if (mprotect((void *)fpbase, fplen, PROT_READ | PROT_WRITE |
+	    PROT_EXEC) != 0) {
+		if (mprotect((void *)mpbase, mplen, PROT_READ | PROT_EXEC) !=
+		    0) {
+			umem_panic("genasm failed to restore memory "
+			    "protection: %d", errno);
+		}
+		return (B_FALSE);
+	}
 
 	/*
 	 * The total number of caches that we can service is the minimum of:
@@ -573,17 +616,24 @@ umem_genasm(int *cp, umem_cache_t **caches, int nc)
 	if (nc < nents)
 		nents = nc;
 
-	/* Based on our constraints, this is not an error */
-	if (nents == 0 || umem_ptc_size == 0)
-		return (0);
+	/*
+	 * If the number of per-thread caches has been set to zero or the
+	 * per-thread cache size has been set to zero, don't bother trying to
+	 * write any assembly and just use the default malloc and free. When we
+	 * return, indicate that there is no PTC support.
+	 */
+	if (nents == 0 || umem_ptc_size == 0) {
+		goto out;
+	}
 
 	/* Take into account the jump */
-	if (genasm_malloc(mptr, umem_genasm_msize, nents, cp) != 0)
-		return (1);
-
-	if (genasm_free(fptr, umem_genasm_fsize, nents, cp) != 0)
-		return (1);
+	if (genasm_malloc(mptr, umem_genasm_msize, nents, cp) != 0) {
+		goto out;
+	}
 
+	if (genasm_free(fptr, umem_genasm_fsize, nents, cp) != 0) {
+		goto out;
+	}
 
 	/* nop out the jump with a multibyte jump */
 	vptr = (void *)umem_genasm_mptr;
@@ -598,5 +648,17 @@ umem_genasm(int *cp, umem_cache_t **caches, int nc)
 	for (i = 0; i < nents; i++)
 		caches[i]->cache_flags |= UMF_PTC;
 
-	return (0);
+	ret = B_TRUE;
+out:
+	if (mprotect((void *)mpbase, mplen, PROT_READ | PROT_EXEC) != 0) {
+		umem_panic("genasm failed to restore memory protection: %d",
+		    errno);
+	}
+
+	if (mprotect((void *)fpbase, fplen, PROT_READ | PROT_EXEC) != 0) {
+		umem_panic("genasm failed to restore memory protection: %d",
+		    errno);
+	}
+
+	return (ret);
 }
diff --git a/usr/src/lib/libumem/common/envvar.c b/usr/src/lib/libumem/common/envvar.c
index 6c57d9553e..25758aa2e7 100644
--- a/usr/src/lib/libumem/common/envvar.c
+++ b/usr/src/lib/libumem/common/envvar.c
@@ -25,7 +25,7 @@
  */
 
 /*
- * Copyright (c) 2012 Joyent, Inc. All rights reserved.
+ * Copyright (c) 2019 Joyent, Inc.
  * Copyright (c) 2015 by Delphix. All rights reserved.
  */
 
@@ -539,6 +539,13 @@ process_item(const umem_env_item_t *item, const char *item_arg)
 	case ITEM_SIZE:
 		arg_required = 1;
 		break;
+
+	default:
+		/*
+		 * These are flags that aren't supported, so they'll error out
+		 * below.
+		 */
+		break;
 	}
 
 	switch (item->item_type) {
diff --git a/usr/src/lib/libumem/common/mapfile-vers b/usr/src/lib/libumem/common/mapfile-vers
index 888a1570f2..e95e666885 100644
--- a/usr/src/lib/libumem/common/mapfile-vers
+++ b/usr/src/lib/libumem/common/mapfile-vers
@@ -20,7 +20,7 @@
 #
 #
 # Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
-# Copyright (c) 2012, Joyent, Inc.  All rights reserved.
+# Copyright (c) 2019, Joyent, Inc.
 #
 
 #
@@ -41,7 +41,7 @@ $mapfile_version 2
 
 $if _x86
 LOAD_SEGMENT umem {
-	FLAGS = READ WRITE EXECUTE;
+	FLAGS = READ EXECUTE;
 	ASSIGN_SECTION {
 		IS_NAME = .text;
 		FILE_BASENAME = asm_subr.o
diff --git a/usr/src/lib/libumem/common/umem.c b/usr/src/lib/libumem/common/umem.c
index dbc738a049..7f31529852 100644
--- a/usr/src/lib/libumem/common/umem.c
+++ b/usr/src/lib/libumem/common/umem.c
@@ -25,7 +25,7 @@
  */
 
 /*
- * Copyright (c) 2014 Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2019 Joyent, Inc.
  * Copyright (c) 2015 by Delphix. All rights reserved.
  */
 
@@ -1338,12 +1338,13 @@ static void *
 umem_log_enter(umem_log_header_t *lhp, void *data, size_t size)
 {
 	void *logspace;
-	umem_cpu_log_header_t *clhp =
-	    &lhp->lh_cpu[CPU(umem_cpu_mask)->cpu_number];
+	umem_cpu_log_header_t *clhp;
 
 	if (lhp == NULL || umem_logging == 0)
 		return (NULL);
 
+	clhp = &lhp->lh_cpu[CPU(umem_cpu_mask)->cpu_number];
+
 	(void) mutex_lock(&clhp->clh_lock);
 	clhp->clh_hits++;
 	if (size > clhp->clh_avail) {
@@ -2843,8 +2844,9 @@ umem_cache_create(
 		}
 		ASSERT(!(cp->cache_flags & UMF_AUDIT));
 	} else {
-		size_t chunks, bestfit, waste, slabsize;
+		size_t chunks, waste, slabsize;
 		size_t minwaste = LONG_MAX;
+		size_t bestfit = SIZE_MAX;
 
 		for (chunks = 1; chunks <= UMEM_VOID_FRACTION; chunks++) {
 			slabsize = P2ROUNDUP(chunksize * chunks,
@@ -2865,6 +2867,10 @@ umem_cache_create(
 		}
 		if (cflags & UMC_QCACHE)
 			bestfit = MAX(1 << highbit(3 * vmp->vm_qcache_max), 64);
+		if (bestfit == SIZE_MAX) {
+			errno = ENOMEM;
+			goto fail;
+		}
 		cp->cache_slabsize = bestfit;
 		cp->cache_mincolor = 0;
 		cp->cache_maxcolor = bestfit % chunksize;
@@ -3215,12 +3221,16 @@ umem_cache_init(void)
 	umem_tmem_off = _tmem_get_base();
 	_tmem_set_cleanup(umem_cache_tmem_cleanup);
 
+#ifndef	UMEM_STANDALONE
 	if (umem_genasm_supported && !(umem_flags & UMF_DEBUG) &&
 	    !(umem_flags & UMF_NOMAGAZINE) &&
 	    umem_ptc_size > 0) {
 		umem_ptc_enabled = umem_genasm(umem_alloc_sizes,
-		    umem_alloc_caches, i) == 0 ? 1 : 0;
+		    umem_alloc_caches, i) ? 1 : 0;
 	}
+#else
+	umem_ptc_enabled = 0;
+#endif
 
 	/*
 	 * Initialization cannot fail at this point.  Make the caches
diff --git a/usr/src/lib/libumem/common/umem_base.h b/usr/src/lib/libumem/common/umem_base.h
index c845331fbc..d8f6f4fd4a 100644
--- a/usr/src/lib/libumem/common/umem_base.h
+++ b/usr/src/lib/libumem/common/umem_base.h
@@ -23,7 +23,7 @@
  * Use is subject to license terms.
  */
 /*
- * Copyright (c) 2012, Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2019, Joyent, Inc.
  */
 
 #ifndef	_UMEM_BASE_H
@@ -147,7 +147,7 @@ void umem_process_envvars(void);
  * umem_genasm.c: private interfaces
  */
 extern const int umem_genasm_supported;
-extern int umem_genasm(int *, umem_cache_t **, int);
+extern boolean_t umem_genasm(int *, umem_cache_t **, int);
 
 /*
  * malloc.c: traditional malloc/free interface for genasm
diff --git a/usr/src/lib/libumem/common/vmem.c b/usr/src/lib/libumem/common/vmem.c
index f66cccc698..a04a538e69 100644
--- a/usr/src/lib/libumem/common/vmem.c
+++ b/usr/src/lib/libumem/common/vmem.c
@@ -22,7 +22,7 @@
 /*
  * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
- * Copyright 2012 Joyent, Inc. All rights reserved.
+ * Copyright 2019, Joyent, Inc.
  * Copyright (c) 2017 by Delphix. All rights reserved.
  */
 
@@ -819,7 +819,7 @@ vmem_xalloc(vmem_t *vmp, size_t size, size_t align, size_t phase,
 {
 	vmem_seg_t *vsp;
 	vmem_seg_t *vbest = NULL;
-	uintptr_t addr, taddr, start, end;
+	uintptr_t addr = 0, taddr, start, end;
 	void *vaddr;
 	int hb, flist, resv;
 	uint32_t mtbf;
@@ -981,6 +981,9 @@ vmem_xalloc(vmem_t *vmp, size_t size, size_t align, size_t phase,
 		(void) pthread_setcancelstate(cancel_state, NULL);
 	}
 	if (vbest != NULL) {
+		if (addr == 0) {
+			umem_panic("vmem_xalloc(): addr == 0");
+		}
 		ASSERT(vbest->vs_type == VMEM_FREE);
 		ASSERT(vbest->vs_knext != vbest);
 		(void) vmem_seg_alloc(vmp, vbest, addr, size);
diff --git a/usr/src/lib/libumem/i386/umem_genasm.c b/usr/src/lib/libumem/i386/umem_genasm.c
index 530a83e486..7c0753bf81 100644
--- a/usr/src/lib/libumem/i386/umem_genasm.c
+++ b/usr/src/lib/libumem/i386/umem_genasm.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright (c) 2014 Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2019 Joyent, Inc.  All rights reserved.
  */
 
 /*
@@ -60,12 +60,17 @@
  * Each block of assembly has psuedocode that describes its purpose.
  */
 
+/*
+ * umem_base must be first.
+ */
+#include "umem_base.h"
+
 #include <inttypes.h>
 #include <strings.h>
 #include <umem_impl.h>
-#include "umem_base.h"
-
 #include <atomic.h>
+#include <sys/mman.h>
+#include <errno.h>
 
 const int umem_genasm_supported = 1;
 static uintptr_t umem_genasm_mptr = (uintptr_t)&_malloc;
@@ -536,19 +541,57 @@ genasm_free(void *base, size_t len, int nents, int *umem_alloc_sizes)
 	return (0);
 }
 
-int
+boolean_t
 umem_genasm(int *alloc_sizes, umem_cache_t **caches, int ncaches)
 {
 	int nents, i;
 	uint8_t *mptr;
 	uint8_t *fptr;
 	uint64_t v, *vptr;
+	size_t mplen, fplen;
+	uintptr_t mpbase, fpbase;
+	boolean_t ret = B_FALSE;
 
 	mptr = (void *)((uintptr_t)umem_genasm_mptr + 5);
 	fptr = (void *)((uintptr_t)umem_genasm_fptr + 5);
 	if (umem_genasm_mptr == 0 || umem_genasm_msize == 0 ||
-	    umem_genasm_fptr == 0 || umem_genasm_fsize == 0)
-		return (1);
+	    umem_genasm_fptr == 0 || umem_genasm_fsize == 0) {
+		return (B_FALSE);
+	}
+
+	mplen = P2ROUNDUP(umem_genasm_msize, pagesize);
+	mpbase = P2ALIGN((uintptr_t)umem_genasm_mptr, pagesize);
+	fplen = P2ROUNDUP(umem_genasm_fsize, pagesize);
+	fpbase = P2ALIGN((uintptr_t)umem_genasm_mptr, pagesize);
+
+	/*
+	 * If the values straddle a page boundary, then we might need to
+	 * actually remap two pages.
+	 */
+	if (P2ALIGN(umem_genasm_msize + (uintptr_t)umem_genasm_mptr,
+	    pagesize) != mpbase) {
+		mplen += pagesize;
+	}
+
+	if (P2ALIGN(umem_genasm_fsize + (uintptr_t)umem_genasm_fptr,
+	    pagesize) != fpbase) {
+		fplen += pagesize;
+	}
+
+	if (mprotect((void *)mpbase, mplen, PROT_READ | PROT_WRITE |
+	    PROT_EXEC) != 0) {
+		return (B_FALSE);
+	}
+
+	if (mprotect((void *)fpbase, fplen, PROT_READ | PROT_WRITE |
+	    PROT_EXEC) != 0) {
+		if (mprotect((void *)mpbase, mplen, PROT_READ | PROT_EXEC) !=
+		    0) {
+			umem_panic("genasm failed to restore memory "
+			    "protection: %d", errno);
+		}
+		return (B_FALSE);
+	}
 
 	/*
 	 * The total number of caches that we can service is the minimum of:
@@ -565,18 +608,26 @@ umem_genasm(int *alloc_sizes, umem_cache_t **caches, int ncaches)
 	if (ncaches < nents)
 		nents = ncaches;
 
-	/* Based on our constraints, this is not an error */
-	if (nents == 0 || umem_ptc_size == 0)
-		return (0);
+	/*
+	 * If the number of per-thread caches has been set to zero or the
+	 * per-thread cache size has been set to zero, don't bother trying to
+	 * write any assembly and just use the default malloc and free. When we
+	 * return, indicate that there is no PTC support.
+	 */
+	if (nents == 0 || umem_ptc_size == 0) {
+		goto out;
+	}
 
 	/* Take into account the jump */
 	if (genasm_malloc(mptr, umem_genasm_msize, nents,
-	    alloc_sizes) != 0)
-		return (1);
+	    alloc_sizes) != 0) {
+		goto out;
+	}
 
 	if (genasm_free(fptr, umem_genasm_fsize, nents,
-	    alloc_sizes) != 0)
-		return (1);
+	    alloc_sizes) != 0) {
+		goto out;
+	}
 
 	/* nop out the jump with a multibyte jump */
 	vptr = (void *)umem_genasm_mptr;
@@ -591,5 +642,17 @@ umem_genasm(int *alloc_sizes, umem_cache_t **caches, int ncaches)
 	for (i = 0; i < nents; i++)
 		caches[i]->cache_flags |= UMF_PTC;
 
-	return (0);
+	ret = B_TRUE;
+out:
+	if (mprotect((void *)mpbase, mplen, PROT_READ | PROT_EXEC) != 0) {
+		umem_panic("genasm failed to restore memory protection: %d",
+		    errno);
+	}
+
+	if (mprotect((void *)fpbase, fplen, PROT_READ | PROT_EXEC) != 0) {
+		umem_panic("genasm failed to restore memory protection: %d",
+		    errno);
+	}
+
+	return (ret);
 }
diff --git a/usr/src/lib/libumem/sparc/umem_genasm.c b/usr/src/lib/libumem/sparc/umem_genasm.c
index 202d642b0b..95a38a0a78 100644
--- a/usr/src/lib/libumem/sparc/umem_genasm.c
+++ b/usr/src/lib/libumem/sparc/umem_genasm.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright (c) 2014 Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2019 Joyent, Inc.
  */
 
 /*
@@ -35,9 +35,8 @@
 
 const int umem_genasm_supported = 0;
 
-/*ARGSUSED*/
-int
+boolean_t
 umem_genasm(int *alloc_sizes, umem_cache_t **caches, int ncaches)
 {
-	return (1);
+	return (B_FALSE);
 }