Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 19 Jun 2020 03:32:05 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r362361 - in head/sys: compat/linuxkpi/common/src vm
Message-ID:  <202006190332.05J3W53u006443@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: markj
Date: Fri Jun 19 03:32:04 2020
New Revision: 362361
URL: https://svnweb.freebsd.org/changeset/base/362361

Log:
  Add a helper function for validating VA ranges.
  
  Functions which take untrusted user ranges must validate against the
  bounds of the map, and also check for wraparound.  Instead of having the
  same logic duplicated in a number of places, add a function to check.
  
  Reviewed by:	dougm, kib
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D25328

Modified:
  head/sys/compat/linuxkpi/common/src/linux_page.c
  head/sys/vm/vm_fault.c
  head/sys/vm/vm_map.c
  head/sys/vm/vm_map.h
  head/sys/vm/vm_mmap.c

Modified: head/sys/compat/linuxkpi/common/src/linux_page.c
==============================================================================
--- head/sys/compat/linuxkpi/common/src/linux_page.c	Fri Jun 19 03:31:46 2020	(r362360)
+++ head/sys/compat/linuxkpi/common/src/linux_page.c	Fri Jun 19 03:32:04 2020	(r362361)
@@ -222,7 +222,7 @@ __get_user_pages_fast(unsigned long start, int nr_page
 	va = start;
 	map = &curthread->td_proc->p_vmspace->vm_map;
 	end = start + (((size_t)nr_pages) << PAGE_SHIFT);
-	if (start < vm_map_min(map) || end > vm_map_max(map))
+	if (!vm_map_range_valid(map, start, end))
 		return (-EINVAL);
 	prot = write ? (VM_PROT_READ | VM_PROT_WRITE) : VM_PROT_READ;
 	for (count = 0, mp = pages, va = start; va < end;

Modified: head/sys/vm/vm_fault.c
==============================================================================
--- head/sys/vm/vm_fault.c	Fri Jun 19 03:31:46 2020	(r362360)
+++ head/sys/vm/vm_fault.c	Fri Jun 19 03:32:04 2020	(r362361)
@@ -1713,10 +1713,7 @@ vm_fault_quick_hold_pages(vm_map_t map, vm_offset_t ad
 	end = round_page(addr + len);
 	addr = trunc_page(addr);
 
-	/*
-	 * Check for illegal addresses.
-	 */
-	if (addr < vm_map_min(map) || addr > end || end > vm_map_max(map))
+	if (!vm_map_range_valid(map, addr, end))
 		return (-1);
 
 	if (atop(end - addr) > max_count)

Modified: head/sys/vm/vm_map.c
==============================================================================
--- head/sys/vm/vm_map.c	Fri Jun 19 03:31:46 2020	(r362360)
+++ head/sys/vm/vm_map.c	Fri Jun 19 03:32:04 2020	(r362361)
@@ -1616,8 +1616,7 @@ vm_map_insert(vm_map_t map, vm_object_t object, vm_oof
 	/*
 	 * Check that the start and end points are not bogus.
 	 */
-	if (start < vm_map_min(map) || end > vm_map_max(map) ||
-	    start >= end)
+	if (!vm_map_range_valid(map, start, end))
 		return (KERN_INVALID_ADDRESS);
 
 	/*
@@ -2161,9 +2160,7 @@ again:
 			goto done;
 		}
 	} else if ((cow & MAP_REMAP) != 0) {
-		if (*addr < vm_map_min(map) ||
-		    *addr + length > vm_map_max(map) ||
-		    *addr + length <= length) {
+		if (!vm_map_range_valid(map, *addr, *addr + length)) {
 			rv = KERN_INVALID_ADDRESS;
 			goto done;
 		}
@@ -4324,9 +4321,8 @@ vm_map_stack_locked(vm_map_t map, vm_offset_t addrbos,
 	KASSERT(orient != (MAP_STACK_GROWS_DOWN | MAP_STACK_GROWS_UP),
 	    ("bi-dir stack"));
 
-	if (addrbos < vm_map_min(map) ||
-	    addrbos + max_ssize > vm_map_max(map) ||
-	    addrbos + max_ssize <= addrbos)
+	if (max_ssize == 0 ||
+	    !vm_map_range_valid(map, addrbos, addrbos + max_ssize))
 		return (KERN_INVALID_ADDRESS);
 	sgp = ((curproc->p_flag2 & P2_STKGAP_DISABLE) != 0 ||
 	    (curproc->p_fctl0 & NT_FREEBSD_FCTL_STKGAP_DISABLE) != 0) ? 0 :

Modified: head/sys/vm/vm_map.h
==============================================================================
--- head/sys/vm/vm_map.h	Fri Jun 19 03:31:46 2020	(r362360)
+++ head/sys/vm/vm_map.h	Fri Jun 19 03:32:04 2020	(r362361)
@@ -255,6 +255,17 @@ vm_map_modflags(vm_map_t map, vm_flags_t set, vm_flags
 {
 	map->flags = (map->flags | set) & ~clear;
 }
+
+static inline bool
+vm_map_range_valid(vm_map_t map, vm_offset_t start, vm_offset_t end)
+{
+	if (end < start)
+		return (false);
+	if (start < vm_map_min(map) || end > vm_map_max(map))
+		return (false);
+	return (true);
+}
+
 #endif	/* KLD_MODULE */
 #endif	/* _KERNEL */
 

Modified: head/sys/vm/vm_mmap.c
==============================================================================
--- head/sys/vm/vm_mmap.c	Fri Jun 19 03:31:46 2020	(r362360)
+++ head/sys/vm/vm_mmap.c	Fri Jun 19 03:32:04 2020	(r362361)
@@ -342,11 +342,8 @@ kern_mmap_req(struct thread *td, const struct mmap_req
 			return (EINVAL);
 
 		/* Address range must be all in user VM space. */
-		if (addr < vm_map_min(&vms->vm_map) ||
-		    addr + size > vm_map_max(&vms->vm_map))
+		if (!vm_map_range_valid(&vms->vm_map, addr, addr + size))
 			return (EINVAL);
-		if (addr + size < addr)
-			return (EINVAL);
 #ifdef MAP_32BIT
 		if (flags & MAP_32BIT && addr + size > MAP_32BIT_MAX_ADDR)
 			return (EINVAL);
@@ -577,7 +574,7 @@ kern_munmap(struct thread *td, uintptr_t addr0, size_t
 	vm_map_entry_t entry;
 	bool pmc_handled;
 #endif
-	vm_offset_t addr;
+	vm_offset_t addr, end;
 	vm_size_t pageoff;
 	vm_map_t map;
 
@@ -589,15 +586,11 @@ kern_munmap(struct thread *td, uintptr_t addr0, size_t
 	addr -= pageoff;
 	size += pageoff;
 	size = (vm_size_t) round_page(size);
-	if (addr + size < addr)
-		return (EINVAL);
-
-	/*
-	 * Check for illegal addresses.  Watch out for address wrap...
-	 */
+	end = addr + size;
 	map = &td->td_proc->p_vmspace->vm_map;
-	if (addr < vm_map_min(map) || addr + size > vm_map_max(map))
+	if (!vm_map_range_valid(map, addr, end))
 		return (EINVAL);
+
 	vm_map_lock(map);
 #ifdef HWPMC_HOOKS
 	pmc_handled = false;
@@ -609,7 +602,7 @@ kern_munmap(struct thread *td, uintptr_t addr0, size_t
 		 */
 		pkm.pm_address = (uintptr_t) NULL;
 		if (vm_map_lookup_entry(map, addr, &entry)) {
-			for (; entry->start < addr + size;
+			for (; entry->start < end;
 			    entry = vm_map_entry_succ(entry)) {
 				if (vm_map_check_protection(map, entry->start,
 					entry->end, VM_PROT_EXECUTE) == TRUE) {
@@ -621,7 +614,7 @@ kern_munmap(struct thread *td, uintptr_t addr0, size_t
 		}
 	}
 #endif
-	vm_map_delete(map, addr, addr + size);
+	vm_map_delete(map, addr, end);
 
 #ifdef HWPMC_HOOKS
 	if (__predict_false(pmc_handled)) {
@@ -772,9 +765,7 @@ kern_madvise(struct thread *td, uintptr_t addr0, size_
 	 */
 	map = &td->td_proc->p_vmspace->vm_map;
 	addr = addr0;
-	if (addr < vm_map_min(map) || addr + len > vm_map_max(map))
-		return (EINVAL);
-	if ((addr + len) < addr)
+	if (!vm_map_range_valid(map, addr, addr + len))
 		return (EINVAL);
 
 	/*

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202006190332.05J3W53u006443>