[haiku-commits] Change in haiku[master]: kernel/vm: Validate user addresses do not cross the kernel/user bound...

  • From: Gerrit <review@xxxxxxxxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 16 Mar 2020 03:35:35 +0000

From waddlesplash <waddlesplash@xxxxxxxxx>:

waddlesplash has uploaded this change for review. ( 
https://review.haiku-os.org/c/haiku/+/2361 ;)


Change subject: kernel/vm: Validate user addresses do not cross the kernel/user 
boundary.
......................................................................

kernel/vm: Validate user addresses do not cross the kernel/user boundary.

This protects against malicious programs trying to steal/overwrite
kernel memory by overflowing user buffers.

Note that this constitutes a behavioral change to user_strlcpy:
previously, address overflows on the "source" side would either
copy less data, or copy some data and fail anyway. Now, address
overflows on either side will always fail before any data is
copied at all.
---
M src/system/kernel/vm/vm.cpp
1 file changed, 22 insertions(+), 18 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/61/2361/1

diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp
index 05f1dac..3597f6f 100644
--- a/src/system/kernel/vm/vm.cpp
+++ b/src/system/kernel/vm/vm.cpp
@@ -5242,14 +5242,30 @@
 }


+static inline bool
+validate_user_range(const void* addr, size_t size)
+{
+       addr_t address = (addr_t)addr;
+
+       // Check for overflows on all addresses.
+       if ((address + size) < address)
+               return false;
+
+       // Validate that the address does not cross the kernel/user boundary.
+       if (IS_USER_ADDRESS(address))
+               return IS_USER_ADDRESS(address + size);
+       else
+               return !IS_USER_ADDRESS(address + size);
+}
+
+
 //     #pragma mark - kernel public API


 status_t
 user_memcpy(void* to, const void* from, size_t size)
 {
-       // don't allow address overflows
-       if ((addr_t)from + size < (addr_t)from || (addr_t)to + size < 
(addr_t)to)
+       if (!validate_user_range(to, size) || !validate_user_range(from, size))
                return B_BAD_ADDRESS;

        if (arch_cpu_user_memcpy(to, from, size) < B_OK)
@@ -5275,31 +5291,19 @@
                return B_BAD_VALUE;
        if (from == NULL)
                return B_BAD_ADDRESS;
-
-       // limit size to avoid address overflows
-       size_t maxSize = std::min((addr_t)size,
-               ~(addr_t)0 - std::max((addr_t)from, (addr_t)to) + 1);
-               // NOTE: Since arch_cpu_user_strlcpy() determines the length of 
\a from,
-               // the source address might still overflow.
-
-       ssize_t result = arch_cpu_user_strlcpy(to, from, maxSize);
-
-       // If we hit the address overflow boundary, fail.
-       if (result < 0 || (result >= 0 && (size_t)result >= maxSize
-                       && maxSize < size)) {
+       if (!validate_user_range(to, size) || !validate_user_range(from, size))
                return B_BAD_ADDRESS;
-       }

-       return result;
+       return arch_cpu_user_strlcpy(to, from, size);
 }


 status_t
 user_memset(void* s, char c, size_t count)
 {
-       // don't allow address overflows
-       if ((addr_t)s + count < (addr_t)s)
+       if (!validate_user_range(s, count))
                return B_BAD_ADDRESS;
+
        if (arch_cpu_user_memset(s, c, count) < B_OK)
                return B_BAD_ADDRESS;


--
To view, visit https://review.haiku-os.org/c/haiku/+/2361
To unsubscribe, or for help writing mail filters, visit 
https://review.haiku-os.org/settings

Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: I01d8b22672ab3758a9dd87b521af6fedd0487417
Gerrit-Change-Number: 2361
Gerrit-PatchSet: 1
Gerrit-Owner: waddlesplash <waddlesplash@xxxxxxxxx>
Gerrit-MessageType: newchange

Other related posts:

  • » [haiku-commits] Change in haiku[master]: kernel/vm: Validate user addresses do not cross the kernel/user bound... - Gerrit