Don't mark heap memory as executable v3

Marking heap memory as executable using mprotect() is not allowed
by SELinux.  mprotect() calls that try to do this will fail on systems
with SELinux enabled.  This is also a security risk, so it should be
fixed even on systems that allow this.

Any memory we want to mark as executable must be allocated using mmap().
See https://www.akkadia.org/drepper/selinux-mem.html

The two places where we try to mark heap memory as executable both use
posix_memalign() to allocate the heap memory.  In both cases, the
alignment value passed into this function is always equal to PAGE_SIZE,
which means that they are safe to replace with mmap(), which guarantees
alignment to PAGE_SIZE.  In this case PAGE_SIZE has been set to
sysconf(_SC_PAGESIZE);

v2:
  - Use MAP_PRIVATE instead of MAP_SHARED.  This matches the behavior
    of memory allocated by posix_memalign()
  - Ignore alignment hints instead of returning error when we can't
    accommodate them.
  - Drop alignment parameter of allocate_exec_aligned_memory() since
    the only alignment supported is sysconf(_SC_PAGESIZE).
  - Remove extra parameter from fmm_release().
  - Add error path to fmm_allocate_host_cpu() for when mmap fails.

v3:
  - Avoid use after free.

Change-Id: I7d51279790d9700bc3fa761c44bfde1c1936019b
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>

src/fmm.c

@@ -1110,26 +1110,22 @@ void *fmm_allocate_doorbell(uint32_t gpu_id, uint64_t MemorySizeInBytes,
 static void *fmm_allocate_host_cpu(uint64_t MemorySizeInBytes,
 				HsaMemFlags flags)
 {
-	int err;
-	HSAuint64 page_size;
 	void *mem = NULL;
 	vm_object_t *vm_obj;
+	int mmap_prot = PROT_READ | PROT_WRITE;
 
-	page_size = PageSizeFromFlags(flags.ui32.PageSize);
-	err = posix_memalign(&mem, page_size, MemorySizeInBytes);
-	if (err != 0)
+	if (flags.ui32.ExecuteAccess)
+		mmap_prot |= PROT_EXEC;
+
+	/* mmap will return a pointer with alignment equal to
+	 * sysconf(_SC_PAGESIZE).
+	 */
+	mem = mmap(NULL, MemorySizeInBytes, mmap_prot,
+			MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+
+	if (mem == MAP_FAILED)
 		return NULL;
 
-	if (flags.ui32.ExecuteAccess) {
-		err = mprotect(mem, MemorySizeInBytes,
-				PROT_READ | PROT_WRITE | PROT_EXEC);
-
-		if (err != 0) {
-			free(mem);
-			return NULL;
-		}
-	}
-
 	pthread_mutex_lock(&cpuvm_aperture.fmm_mutex);
 	vm_obj = aperture_allocate_object(&cpuvm_aperture, mem, 0,
 				      MemorySizeInBytes, flags.Value);
@@ -1354,14 +1350,18 @@ void fmm_release(void *address)
 	 * refers to the system memory
 	 */
 	if (!found) {
+		uint64_t size = 0;
 		/* Release the vm object in CPUVM */
 		pthread_mutex_lock(&cpuvm_aperture.fmm_mutex);
 		object = vm_find_object_by_address(&cpuvm_aperture, address, 0);
-		if (object)
+		if (object) {
+			size = object->size;
 			vm_remove_object(&cpuvm_aperture, object);
+		}
 		pthread_mutex_unlock(&cpuvm_aperture.fmm_mutex);
 		/* Free the memory from the system */
-		free(address);
+		if (size)
+			munmap(address, size);
 	}
 }
 

src/queues.c

@@ -309,21 +309,20 @@ static HSAKMT_STATUS map_doorbell(HSAuint32 NodeId, HSAuint32 gpu_id,
 	return status;
 }
 
-static void *allocate_exec_aligned_memory_cpu(uint32_t size, uint32_t align)
+static void *allocate_exec_aligned_memory_cpu(uint32_t size)
 {
 	void *ptr;
-	int retval;
 
-	retval = posix_memalign(&ptr, align, size);
-	if (retval != 0)
-		return NULL;
+	/* mmap will return a pointer with alignment equal to
+	 * sysconf(_SC_PAGESIZE).
+	 *
+	 * MAP_ANONYMOUS initializes the memory to zero.
+	 */
+	ptr = mmap(NULL, size, PROT_READ | PROT_WRITE | PROT_EXEC,
+				MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
 
-	retval = mprotect(ptr, size, PROT_READ | PROT_WRITE | PROT_EXEC);
-	if (retval != 0) {
-		free(ptr);
+	if (ptr == MAP_FAILED)
 		return NULL;
-	}
-	memset(ptr, 0, size);
 	return ptr;
 }
 
@@ -395,15 +394,17 @@ void free_exec_aligned_memory_gpu(void *addr, uint32_t size, uint32_t align)
 		hsaKmtFreeMemory(addr, size);
 }
 
+/*
+ * Allocates memory aligned to sysconf(_SC_PAGESIZE)
+ */
 static void *allocate_exec_aligned_memory(uint32_t size,
-					  uint32_t align,
 					  enum asic_family_type type,
 					  uint32_t NodeId)
 {
 	if (IS_DGPU(type))
-		return allocate_exec_aligned_memory_gpu(size, align, NodeId,
+		return allocate_exec_aligned_memory_gpu(size, PAGE_SIZE, NodeId,
 							false);
-	return allocate_exec_aligned_memory_cpu(size, align);
+	return allocate_exec_aligned_memory_cpu(size);
 }
 
 static void free_exec_aligned_memory(void *addr, uint32_t size, uint32_t align,
@@ -412,7 +413,7 @@ static void free_exec_aligned_memory(void *addr, uint32_t size, uint32_t align,
 	if (IS_DGPU(type))
 		free_exec_aligned_memory_gpu(addr, size, align);
 	else
-		free(addr);
+		munmap(addr, size);
 }
 
 static void free_queue(struct queue *q)
@@ -439,7 +440,6 @@ static int handle_concrete_asic(struct queue *q,
 		if (dev_info->eop_buffer_size > 0) {
 			q->eop_buffer =
 					allocate_exec_aligned_memory(q->dev_info->eop_buffer_size,
-					PAGE_SIZE,
 					dev_info->asic_family,
 					NodeId);
 			if (!q->eop_buffer)
@@ -454,7 +454,6 @@ static int handle_concrete_asic(struct queue *q,
 			args->ctl_stack_size = q->ctl_stack_size;
 			q->ctx_save_restore =
 				allocate_exec_aligned_memory(q->ctx_save_restore_size,
-							     PAGE_SIZE,
 							     dev_info->asic_family,
 							     NodeId);
 			if (!q->ctx_save_restore)
@@ -506,7 +505,7 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtCreateQueue(HSAuint32 NodeId,
 	dev_info = get_device_info_by_dev_id(dev_id);
 
 	struct queue *q = allocate_exec_aligned_memory(sizeof(*q),
-			PAGE_SIZE, dev_info->asic_family,
+			dev_info->asic_family,
 			NodeId);
 	if (!q)
 		return HSAKMT_STATUS_NO_MEMORY;