Migrated user memory copy APIs to use the new exception table guards
authorDavide Libenzi <dlibenzi@google.com>
Sat, 31 Oct 2015 01:31:15 +0000 (18:31 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 16 Nov 2015 23:03:23 +0000 (15:03 -0800)
Migrated user memory copy APIs to use the new exception table guards.
Added string copy APIs as well.

Signed-off-by: Davide Libenzi <dlibenzi@google.com>
Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/arch/x86/uaccess.h
kern/include/umem.h
kern/src/ktest/pb_ktests.c
kern/src/syscall.c
kern/src/umem.c

index 90ffea8..539a59e 100644 (file)
@@ -77,6 +77,34 @@ struct extable_ip_fixup {
                                 : "=r"(err)                                                                                    \
                                 : "D" (dst), "S" (src), "c" (count), "i" (errret), "0" (err))
 
+static inline int __put_user(void *dst, const void *src, unsigned int count)
+{
+       int err = 0;
+
+       switch (count) {
+       case 1:
+               __put_user_asm(*(const uint8_t *) src, (uint8_t *) dst, err, "b",
+                                          "b", "iq", -EFAULT);
+               break;
+       case 2:
+               __put_user_asm(*(const uint16_t *) src, (uint16_t *) dst, err, "w",
+                                          "w", "ir", -EFAULT);
+               break;
+       case 4:
+               __put_user_asm(*(const uint32_t *) src, (uint32_t *) dst, err, "l",
+                                          "k", "ir", -EFAULT);
+               break;
+       case 8:
+               __put_user_asm(*(const uint64_t *) src, (uint64_t *) dst, err, "q",
+                                          "", "er", -EFAULT);
+               break;
+       default:
+               __user_memcpy(dst, src, count, err, -EFAULT);
+       }
+
+       return err;
+}
+
 static inline int copy_to_user(void *dst, const void *src, unsigned int count)
 {
        int err = 0;
@@ -86,26 +114,35 @@ static inline int copy_to_user(void *dst, const void *src, unsigned int count)
        } else if (!__builtin_constant_p(count)) {
                __user_memcpy(dst, src, count, err, -EFAULT);
        } else {
-               switch (count) {
-               case 1:
-                       __put_user_asm(*(const uint8_t *) src, (uint8_t *) dst, err, "b",
-                                                  "b", "iq", -EFAULT);
-                       break;
-               case 2:
-                       __put_user_asm(*(const uint16_t *) src, (uint16_t *) dst, err, "w",
-                                                  "w", "ir", -EFAULT);
-                       break;
-               case 4:
-                       __put_user_asm(*(const uint32_t *) src, (uint32_t *) dst, err, "l",
-                                                  "k", "ir", -EFAULT);
-                       break;
-               case 8:
-                       __put_user_asm(*(const uint64_t *) src, (uint64_t *) dst, err, "q",
-                                                  "", "er", -EFAULT);
-                       break;
-               default:
-                       __user_memcpy(dst, src, count, err, -EFAULT);
-               }
+               err = __put_user(dst, src, count);
+       }
+
+       return err;
+}
+
+static inline int __get_user(void *dst, const void *src, unsigned int count)
+{
+       int err = 0;
+
+       switch (count) {
+       case 1:
+               __get_user_asm(*(uint8_t *) dst, (const uint8_t *) src, err, "b",
+                                          "b", "=q", -EFAULT);
+               break;
+       case 2:
+               __get_user_asm(*(uint16_t *) dst, (const uint16_t *) src, err, "w",
+                                          "w", "=r", -EFAULT);
+               break;
+       case 4:
+               __get_user_asm(*(uint32_t *) dst, (const uint32_t *) src, err, "l",
+                                          "k", "=r", -EFAULT);
+               break;
+       case 8:
+               __get_user_asm(*(uint64_t *) dst, (const uint64_t *) src, err, "q",
+                                          "", "=r", -EFAULT);
+               break;
+       default:
+               __user_memcpy(dst, src, count, err, -EFAULT);
        }
 
        return err;
@@ -121,26 +158,7 @@ static inline int copy_from_user(void *dst, const void *src,
        } else if (!__builtin_constant_p(count)) {
                __user_memcpy(dst, src, count, err, -EFAULT);
        } else {
-               switch (count) {
-               case 1:
-                       __get_user_asm(*(uint8_t *) dst, (const uint8_t *) src, err, "b",
-                                                  "b", "=q", -EFAULT);
-                       break;
-               case 2:
-                       __get_user_asm(*(uint16_t *) dst, (const uint16_t *) src, err, "w",
-                                                  "w", "=r", -EFAULT);
-                       break;
-               case 4:
-                       __get_user_asm(*(uint32_t *) dst, (const uint32_t *) src, err, "l",
-                                                  "k", "=r", -EFAULT);
-                       break;
-               case 8:
-                       __get_user_asm(*(uint64_t *) dst, (const uint64_t *) src, err, "q",
-                                                  "", "=r", -EFAULT);
-                       break;
-               default:
-                       __user_memcpy(dst, src, count, err, -EFAULT);
-               }
+               err = __get_user(dst, src, count);
        }
 
        return err;
index f75d95b..cec5ad8 100644 (file)
@@ -17,13 +17,39 @@ static inline bool is_user_rwaddr(const void *addr, size_t len);
 /* Same deal, but read-only */
 static inline bool is_user_raddr(const void *addr, size_t len);
 
-/* Copy from proc p into the kernel's dest from src */
-int memcpy_from_user(struct proc *p, void *dest, const void *va,
-                     size_t len);
+int strcpy_from_user(struct proc *p, char *dst, const char *src);
+int strcpy_to_user(struct proc *p, char *dst, const char *src);
+
+/**
+ * @brief Copies data from a user buffer to a kernel buffer.
+ *
+ * @param p    the process associated with the user program
+ *             from which the buffer is being copied
+ * @param dest the destination address of the kernel buffer
+ * @param va   the address of the userspace buffer from which we are copying
+ * @param len  the length of the userspace buffer
+ *
+ * @return ESUCCESS on success
+ * @return -EFAULT  the page assocaited with 'va' is not present, the user
+ *                  lacks the proper permissions, or there was an invalid 'va'
+ */
+int memcpy_from_user(struct proc *p, void *dest, const void *va, size_t len);
+
+/**
+ * @brief Copies data to a user buffer from a kernel buffer.
+ *
+ * @param p    the process associated with the user program
+ *             to which the buffer is being copied
+ * @param dest the destination address of the user buffer
+ * @param src  the address of the kernel buffer from which we are copying
+ * @param len  the length of the user buffer
+ *
+ * @return ESUCCESS on success
+ * @return -EFAULT  the page assocaited with 'va' is not present, the user
+ *                  lacks the proper permissions, or there was an invalid 'va'
+ */
+int memcpy_to_user(struct proc *p, void *dest, const void *src, size_t len);
 
-/* Copy to proc p into va from the kernel's src */
-int memcpy_to_user(struct proc *p, void *a, const void *src,
-                   size_t len);
 /* Same as above, but sets errno */
 int memcpy_from_user_errno(struct proc *p, void *dst, const void *src, int len);
 int memcpy_to_user_errno(struct proc *p, void *dst, const void *src, int len);
@@ -70,6 +96,16 @@ static inline bool __is_user_addr(const void *addr, size_t len, uintptr_t lim)
        return TRUE;
 }
 
+static inline size_t __valid_user_bytes_from(const void *addr, uintptr_t lim)
+{
+       if (unlikely((uintptr_t) addr < MMAP_LOWEST_VA))
+               return 0;
+       if (unlikely((uintptr_t) addr >= lim))
+               return 0;
+
+       return (size_t) (lim - (uintptr_t) addr);
+}
+
 /* UWLIM is defined as virtual address below which a process can write */
 static inline bool is_user_rwaddr(const void *addr, size_t len)
 {
@@ -81,3 +117,13 @@ static inline bool is_user_raddr(const void *addr, size_t len)
 {
        return __is_user_addr(addr, len, ULIM);
 }
+
+static inline size_t valid_user_rwbytes_from(const void *addr)
+{
+       return __valid_user_bytes_from(addr, UWLIM);
+}
+
+static inline size_t valid_user_rbytes_from(const void *addr)
+{
+       return __valid_user_bytes_from(addr, ULIM);
+}
index a0f72c6..5274df5 100644 (file)
@@ -2118,6 +2118,13 @@ bool test_uaccess(void)
                                copy_from_user(buf, (const void *) UDATA, sizeof(buf)) ==
                                -EFAULT);
 
+       KT_ASSERT_M(
+               "String copy to user to not mapped UDATA address should fail",
+               strcpy_to_user(NULL, (char *) UDATA, "Akaros") == -EFAULT);
+       KT_ASSERT_M(
+               "String copy from user to not mapped UDATA address should fail",
+               strcpy_from_user(NULL, buf, (const char *) UDATA) == -EFAULT);
+
        KT_ASSERT_M("Copy from user with kernel side source pointer should fail",
                                copy_from_user(buf, buf2, sizeof(buf)) == -EFAULT);
        KT_ASSERT_M("Copy to user with kernel side source pointer should fail",
@@ -2164,6 +2171,15 @@ bool test_uaccess(void)
                        "Copy from user (mem) to mapped address should not fail",
                        copy_from_user(buf, addr, sizeof(buf)) == 0);
 
+               KT_ASSERT_M(
+                       "String copy to user to mapped address should not fail",
+                       strcpy_to_user(current, addr, "Akaros") == 0);
+               KT_ASSERT_M(
+                       "String copy from user to mapped address should not fail",
+                       strcpy_from_user(current, buf, addr) == 0);
+               KT_ASSERT_M("The copied string content should be matching",
+                                       memcmp(buf, "Akaros", 7) == 0);
+
                munmap(tmp, (uintptr_t) addr, mmap_size);
        }
 
index 22a9841..fe6eed4 100644 (file)
@@ -330,14 +330,14 @@ static char *copy_in_path(struct proc *p, const char *path, size_t path_l)
        struct systrace_record *t = pcpui->cur_kthread->trace;
        char *t_path;
        /* PATH_MAX includes the \0 */
-       if (path_l > PATH_MAX) {
+       if (unlikely(path_l > PATH_MAX)) {
                set_errno(ENAMETOOLONG);
                return 0;
        }
        t_path = user_strdup_errno(p, path, path_l);
-       if (!t_path)
+       if (unlikely(!t_path))
                return 0;
-       if (t) {
+       if (unlikely(t)) {
                t->datalen = MIN(sizeof(t->data), path_l);
                memcpy(t->data, t_path, t->datalen);
        }
index 87f4e3a..70865d0 100644 (file)
@@ -7,6 +7,7 @@
  * to involve some form of pinning (TODO), and that global static needs to go. */
 
 #include <ros/common.h>
+#include <arch/uaccess.h>
 #include <umem.h>
 #include <process.h>
 #include <error.h>
 #include <pmap.h>
 #include <smp.h>
 
-/**
- * @brief Copies data from a user buffer to a kernel buffer.
- * 
- * @param p    the process associated with the user program
- *             from which the buffer is being copied
- * @param dest the destination address of the kernel buffer
- * @param va   the address of the userspace buffer from which we are copying
- * @param len  the length of the userspace buffer
- *
- * @return ESUCCESS on success
- * @return -EFAULT  the page assocaited with 'va' is not present, the user 
- *                  lacks the proper permissions, or there was an invalid 'va'
- */
-int memcpy_from_user(struct proc *p, void *dest, const void *va,
-                     size_t len)
-{
-       const void *start, *end;
-       size_t num_pages, i;
-       pte_t pte;
-       uintptr_t perm = PTE_USER_RO;
-       size_t bytes_copied = 0;
-
-       static_assert(ULIM % PGSIZE == 0 && ULIM != 0); // prevent wrap-around
-
-       start = (void*)ROUNDDOWN((uintptr_t)va, PGSIZE);
-       end = (void*)ROUNDUP((uintptr_t)va + len, PGSIZE);
-
-       if (start >= (void*)ULIM || end > (void*)ULIM)
-               return -EFAULT;
-
-       num_pages = LA2PPN(end - start);
-       for (i = 0; i < num_pages; i++) {
-               pte = pgdir_walk(p->env_pgdir, start + i * PGSIZE, 0);
-               if (!pte_walk_okay(pte))
-                       return -EFAULT;
-               if (pte_is_present(pte) && !pte_has_perm_ur(pte))
+static int string_copy_from_user(char *dst, const char *src)
+{
+       int error;
+       const char *top = src + valid_user_rbytes_from(src);
+
+       for (;; dst++, src++) {
+               if (unlikely(src >= top))
                        return -EFAULT;
-               if (!pte_is_present(pte))
-                       if (handle_page_fault(p, (uintptr_t)start + i * PGSIZE, PROT_READ))
-                               return -EFAULT;
-
-               void *kpage = KADDR(pte_get_paddr(pte));
-               const void *src_start = i > 0 ? kpage : kpage + (va - start);
-               void *dst_start = dest + bytes_copied;
-               size_t copy_len = PGSIZE;
-               if (i == 0)
-                       copy_len -= va - start;
-               if (i == num_pages-1)
-                       copy_len -= end - (va + len);
-
-               memcpy(dst_start, src_start, copy_len);
-               bytes_copied += copy_len;
+               error = __get_user(dst, src, 1);
+               if (unlikely(error))
+                       return error;
+               if (unlikely(!*dst))
+                       break;
        }
-       assert(bytes_copied == len);
+
        return 0;
 }
 
-/* Same as above, but sets errno */
-int memcpy_from_user_errno(struct proc *p, void *dst, const void *src, int len)
+static int string_copy_to_user(char *dst, const char *src)
 {
-       if (memcpy_from_user(p, dst, src, len)) {
-               set_errno(EINVAL);
-               return -1;
-       }
-       return 0;
-}
+       int error;
+       char *top = dst + valid_user_rwbytes_from(dst);
 
-/**
- * @brief Copies data to a user buffer from a kernel buffer.
- * 
- * @param p    the process associated with the user program
- *             to which the buffer is being copied
- * @param dest the destination address of the user buffer
- * @param va   the address of the kernel buffer from which we are copying
- * @param len  the length of the user buffer
- *
- * @return ESUCCESS on success
- * @return -EFAULT  the page assocaited with 'va' is not present, the user 
- *                  lacks the proper permissions, or there was an invalid 'va'
- */
-int memcpy_to_user(struct proc *p, void *va, const void *src, size_t len)
-{
-       const void *start, *end;
-       size_t num_pages, i;
-       pte_t pte;
-       uintptr_t perm = PTE_USER_RW;
-       size_t bytes_copied = 0;
-
-       static_assert(ULIM % PGSIZE == 0 && ULIM != 0); // prevent wrap-around
-
-       start = (void*)ROUNDDOWN((uintptr_t)va, PGSIZE);
-       end = (void*)ROUNDUP((uintptr_t)va + len, PGSIZE);
-
-       if (start >= (void*)ULIM || end > (void*)ULIM)
-               return -EFAULT;
-
-       num_pages = LA2PPN(end - start);
-       for (i = 0; i < num_pages; i++) {
-               pte = pgdir_walk(p->env_pgdir, start + i * PGSIZE, 0);
-               if (!pte_walk_okay(pte))
+       for (;; dst++, src++) {
+               if (unlikely(dst >= top))
                        return -EFAULT;
-               if (pte_is_present(pte) && !pte_has_perm_urw(pte))
-                       return -EFAULT;
-               if (!pte_is_present(pte))
-                       if (handle_page_fault(p, (uintptr_t)start + i * PGSIZE, PROT_WRITE))
-                               return -EFAULT;
-               void *kpage = KADDR(pte_get_paddr(pte));
-               void *dst_start = i > 0 ? kpage : kpage + (va - start);
-               const void *src_start = src + bytes_copied;
-               size_t copy_len = PGSIZE;
-               if (i == 0)
-                       copy_len -= va - start;
-               if (i == num_pages - 1)
-                       copy_len -= end - (va + len);
-               memcpy(dst_start, src_start, copy_len);
-               bytes_copied += copy_len;
+               error = __put_user(dst, src, 1);
+               if (unlikely(error))
+                       return error;
+               if (unlikely(!*src))
+                       break;
        }
-       assert(bytes_copied == len);
+
        return 0;
 }
 
+int strcpy_from_user(struct proc *p, char *dst, const char *src)
+{
+       struct proc *prev = switch_to(p);
+       int error = string_copy_from_user(dst, src);
+
+       switch_back(p, prev);
+
+       return error;
+}
+
+int strcpy_to_user(struct proc *p, char *dst, const char *src)
+{
+       struct proc *prev = switch_to(p);
+       int error = string_copy_to_user(dst, src);
+
+       switch_back(p, prev);
+
+       return error;
+}
+
+int memcpy_from_user(struct proc *p, void *dest, const void *va, size_t len)
+{
+       struct proc *prev = switch_to(p);
+       int error = copy_from_user(dest, va, len);
+
+       switch_back(p, prev);
+
+       return error;
+}
+
+int memcpy_to_user(struct proc *p, void *dest, const void *src, size_t len)
+{
+       struct proc *prev = switch_to(p);
+       int error = copy_to_user(dest, src, len);
+
+       switch_back(p, prev);
+
+       return error;
+}
+
+/* Same as above, but sets errno */
+int memcpy_from_user_errno(struct proc *p, void *dst, const void *src, int len)
+{
+       int error = memcpy_from_user(p, dst, src, len);
+
+       if (unlikely(error < 0))
+               set_errno(-error);
+
+       return error;
+}
+
 /* Same as above, but sets errno */
 int memcpy_to_user_errno(struct proc *p, void *dst, const void *src, int len)
 {
-       if (memcpy_to_user(p, dst, src, len)) {
-               set_errno(EFAULT);
-               return -1;
-       }
-       return 0;
+       int error = memcpy_to_user(p, dst, src, len);
+
+       if (unlikely(error < 0))
+               set_errno(-error);
+
+       return error;
 }
 
 /* Creates a buffer (kmalloc) and safely copies into it from va.  Can return an