[haiku-commits] haiku: hrev53988 - src/system/kernel/vm

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 16 Mar 2020 09:18:33 -0400 (EDT)

hrev53988 adds 1 changeset to branch 'master'
old head: 4f44282c3bbe2fb1885f632371aee1b3c9be16db
new head: 65920cacc0e84851485c8506499efd87c761e069
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=65920cacc0e8+%5E4f44282c3bbe

----------------------------------------------------------------------------

65920cacc0e8: 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.
  
  Change-Id: I01d8b22672ab3758a9dd87b521af6fedd0487417
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/2361
  Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxx>

                              [ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]

----------------------------------------------------------------------------

Revision:    hrev53988
Commit:      65920cacc0e84851485c8506499efd87c761e069
URL:         https://git.haiku-os.org/haiku/commit/?id=65920cacc0e8
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Mon Mar 16 03:35:00 2020 UTC
Committer:   waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Mon Mar 16 13:11:45 2020 UTC

----------------------------------------------------------------------------

1 file changed, 22 insertions(+), 18 deletions(-)
src/system/kernel/vm/vm.cpp | 40 ++++++++++++++++++++++------------------

----------------------------------------------------------------------------

diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp
index 05f1dace6b..3597f6f604 100644
--- a/src/system/kernel/vm/vm.cpp
+++ b/src/system/kernel/vm/vm.cpp
@@ -5242,14 +5242,30 @@ vm_debug_copy_page_memory(team_id teamID, void* 
unsafeMemory, void* buffer,
 }
 
 
+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 @@ user_strlcpy(char* to, const char* from, size_t size)
                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;
 


Other related posts: