x86: merge 32/64-bit versions of 'strncpy_from_user()' and speed it up
authorLinus Torvalds <torvalds@linux-foundation.org>
Fri, 6 Apr 2012 21:32:32 +0000 (14:32 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Wed, 11 Apr 2012 16:41:28 +0000 (09:41 -0700)
This merges the 32- and 64-bit versions of the x86 strncpy_from_user()
by just rewriting it in C rather than the ancient inline asm versions
that used lodsb/stosb and had been duplicated for (trivial) differences
between the 32-bit and 64-bit versions.

While doing that, it also speeds them up by doing the accesses a word at
a time.  Finally, the new routines also properly handle the case of
hitting the end of the address space, which we have never done correctly
before (fs/namei.c has a hack around it for that reason).

Despite all these improvements, it actually removes more lines than it
adds, due to the de-duplication.  Also, we no longer export (or define)
the legacy __strncpy_from_user() function (that was defined to not do
the user permission checks), since it's not actually used anywhere, and
the user address space checks are built in to the new code.

Other architecture maintainers have been notified that the old hack in
fs/namei.c will be going away in the 3.5 merge window, in case they
copied the x86 approach of being a bit cavalier about the end of the
address space.

Cc: linux-arch@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Anvin" <hpa@zytor.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
arch/x86/include/asm/uaccess.h
arch/x86/include/asm/uaccess_32.h
arch/x86/include/asm/uaccess_64.h
arch/x86/lib/usercopy.c
arch/x86/lib/usercopy_32.c
arch/x86/lib/usercopy_64.c

index 8be5f54..e054459 100644 (file)
@@ -557,6 +557,8 @@ struct __large_struct { unsigned long buf[100]; };
 
 extern unsigned long
 copy_from_user_nmi(void *to, const void __user *from, unsigned long n);
+extern __must_check long
+strncpy_from_user(char *dst, const char __user *src, long count);
 
 /*
  * movsl can be slow when source and dest are not both 8-byte aligned
index 566e803..8084bc7 100644 (file)
@@ -213,11 +213,6 @@ static inline unsigned long __must_check copy_from_user(void *to,
        return n;
 }
 
-long __must_check strncpy_from_user(char *dst, const char __user *src,
-                                   long count);
-long __must_check __strncpy_from_user(char *dst,
-                                     const char __user *src, long count);
-
 /**
  * strlen_user: - Get the size of a string in user space.
  * @str: The string to measure.
index 1c66d30..fcd4b6f 100644 (file)
@@ -208,10 +208,6 @@ int __copy_in_user(void __user *dst, const void __user *src, unsigned size)
        }
 }
 
-__must_check long
-strncpy_from_user(char *dst, const char __user *src, long count);
-__must_check long
-__strncpy_from_user(char *dst, const char __user *src, long count);
 __must_check long strnlen_user(const char __user *str, long n);
 __must_check long __strnlen_user(const char __user *str, long n);
 __must_check long strlen_user(const char __user *str);
index 97be9cb..57252c9 100644 (file)
@@ -7,6 +7,8 @@
 #include <linux/highmem.h>
 #include <linux/module.h>
 
+#include <asm/word-at-a-time.h>
+
 /*
  * best effort, GUP based copy_from_user() that is NMI-safe
  */
@@ -41,3 +43,104 @@ copy_from_user_nmi(void *to, const void __user *from, unsigned long n)
        return len;
 }
 EXPORT_SYMBOL_GPL(copy_from_user_nmi);
+
+static inline unsigned long count_bytes(unsigned long mask)
+{
+       mask = (mask - 1) & ~mask;
+       mask >>= 7;
+       return count_masked_bytes(mask);
+}
+
+/*
+ * Do a strncpy, return length of string without final '\0'.
+ * 'count' is the user-supplied count (return 'count' if we
+ * hit it), 'max' is the address space maximum (and we return
+ * -EFAULT if we hit it).
+ */
+static inline long do_strncpy_from_user(char *dst, const char __user *src, long count, long max)
+{
+       long res = 0;
+
+       /*
+        * Truncate 'max' to the user-specified limit, so that
+        * we only have one limit we need to check in the loop
+        */
+       if (max > count)
+               max = count;
+
+       while (max >= sizeof(unsigned long)) {
+               unsigned long c;
+
+               /* Fall back to byte-at-a-time if we get a page fault */
+               if (unlikely(__get_user(c,(unsigned long __user *)(src+res))))
+                       break;
+               /* This can write a few bytes past the NUL character, but that's ok */
+               *(unsigned long *)(dst+res) = c;
+               c = has_zero(c);
+               if (c)
+                       return res + count_bytes(c);
+               res += sizeof(unsigned long);
+               max -= sizeof(unsigned long);
+       }
+
+       while (max) {
+               char c;
+
+               if (unlikely(__get_user(c,src+res)))
+                       return -EFAULT;
+               dst[res] = c;
+               if (!c)
+                       return res;
+               res++;
+               max--;
+       }
+
+       /*
+        * Uhhuh. We hit 'max'. But was that the user-specified maximum
+        * too? If so, that's ok - we got as much as the user asked for.
+        */
+       if (res >= count)
+               return count;
+
+       /*
+        * Nope: we hit the address space limit, and we still had more
+        * characters the caller would have wanted. That's an EFAULT.
+        */
+       return -EFAULT;
+}
+
+/**
+ * strncpy_from_user: - Copy a NUL terminated string from userspace.
+ * @dst:   Destination address, in kernel space.  This buffer must be at
+ *         least @count bytes long.
+ * @src:   Source address, in user space.
+ * @count: Maximum number of bytes to copy, including the trailing NUL.
+ *
+ * Copies a NUL-terminated string from userspace to kernel space.
+ *
+ * On success, returns the length of the string (not including the trailing
+ * NUL).
+ *
+ * If access to userspace fails, returns -EFAULT (some data may have been
+ * copied).
+ *
+ * If @count is smaller than the length of the string, copies @count bytes
+ * and returns @count.
+ */
+long
+strncpy_from_user(char *dst, const char __user *src, long count)
+{
+       unsigned long max_addr, src_addr;
+
+       if (unlikely(count <= 0))
+               return 0;
+
+       max_addr = current_thread_info()->addr_limit.seg;
+       src_addr = (unsigned long)src;
+       if (likely(src_addr < max_addr)) {
+               unsigned long max = max_addr - src_addr;
+               return do_strncpy_from_user(dst, src, count, max);
+       }
+       return -EFAULT;
+}
+EXPORT_SYMBOL(strncpy_from_user);
index d9b094c..ef2a6a5 100644 (file)
@@ -32,93 +32,6 @@ static inline int __movsl_is_ok(unsigned long a1, unsigned long a2, unsigned lon
 #define movsl_is_ok(a1, a2, n) \
        __movsl_is_ok((unsigned long)(a1), (unsigned long)(a2), (n))
 
-/*
- * Copy a null terminated string from userspace.
- */
-
-#define __do_strncpy_from_user(dst, src, count, res)                      \
-do {                                                                      \
-       int __d0, __d1, __d2;                                              \
-       might_fault();                                                     \
-       __asm__ __volatile__(                                              \
-               "       testl %1,%1\n"                                     \
-               "       jz 2f\n"                                           \
-               "0:     lodsb\n"                                           \
-               "       stosb\n"                                           \
-               "       testb %%al,%%al\n"                                 \
-               "       jz 1f\n"                                           \
-               "       decl %1\n"                                         \
-               "       jnz 0b\n"                                          \
-               "1:     subl %1,%0\n"                                      \
-               "2:\n"                                                     \
-               ".section .fixup,\"ax\"\n"                                 \
-               "3:     movl %5,%0\n"                                      \
-               "       jmp 2b\n"                                          \
-               ".previous\n"                                              \
-               _ASM_EXTABLE(0b,3b)                                        \
-               : "=&d"(res), "=&c"(count), "=&a" (__d0), "=&S" (__d1),    \
-                 "=&D" (__d2)                                             \
-               : "i"(-EFAULT), "0"(count), "1"(count), "3"(src), "4"(dst) \
-               : "memory");                                               \
-} while (0)
-
-/**
- * __strncpy_from_user: - Copy a NUL terminated string from userspace, with less checking.
- * @dst:   Destination address, in kernel space.  This buffer must be at
- *         least @count bytes long.
- * @src:   Source address, in user space.
- * @count: Maximum number of bytes to copy, including the trailing NUL.
- *
- * Copies a NUL-terminated string from userspace to kernel space.
- * Caller must check the specified block with access_ok() before calling
- * this function.
- *
- * On success, returns the length of the string (not including the trailing
- * NUL).
- *
- * If access to userspace fails, returns -EFAULT (some data may have been
- * copied).
- *
- * If @count is smaller than the length of the string, copies @count bytes
- * and returns @count.
- */
-long
-__strncpy_from_user(char *dst, const char __user *src, long count)
-{
-       long res;
-       __do_strncpy_from_user(dst, src, count, res);
-       return res;
-}
-EXPORT_SYMBOL(__strncpy_from_user);
-
-/**
- * strncpy_from_user: - Copy a NUL terminated string from userspace.
- * @dst:   Destination address, in kernel space.  This buffer must be at
- *         least @count bytes long.
- * @src:   Source address, in user space.
- * @count: Maximum number of bytes to copy, including the trailing NUL.
- *
- * Copies a NUL-terminated string from userspace to kernel space.
- *
- * On success, returns the length of the string (not including the trailing
- * NUL).
- *
- * If access to userspace fails, returns -EFAULT (some data may have been
- * copied).
- *
- * If @count is smaller than the length of the string, copies @count bytes
- * and returns @count.
- */
-long
-strncpy_from_user(char *dst, const char __user *src, long count)
-{
-       long res = -EFAULT;
-       if (access_ok(VERIFY_READ, src, 1))
-               __do_strncpy_from_user(dst, src, count, res);
-       return res;
-}
-EXPORT_SYMBOL(strncpy_from_user);
-
 /*
  * Zero Userspace
  */
index b7c2849..0d0326f 100644 (file)
@@ -8,55 +8,6 @@
 #include <linux/module.h>
 #include <asm/uaccess.h>
 
-/*
- * Copy a null terminated string from userspace.
- */
-
-#define __do_strncpy_from_user(dst,src,count,res)                         \
-do {                                                                      \
-       long __d0, __d1, __d2;                                             \
-       might_fault();                                                     \
-       __asm__ __volatile__(                                              \
-               "       testq %1,%1\n"                                     \
-               "       jz 2f\n"                                           \
-               "0:     lodsb\n"                                           \
-               "       stosb\n"                                           \
-               "       testb %%al,%%al\n"                                 \
-               "       jz 1f\n"                                           \
-               "       decq %1\n"                                         \
-               "       jnz 0b\n"                                          \
-               "1:     subq %1,%0\n"                                      \
-               "2:\n"                                                     \
-               ".section .fixup,\"ax\"\n"                                 \
-               "3:     movq %5,%0\n"                                      \
-               "       jmp 2b\n"                                          \
-               ".previous\n"                                              \
-               _ASM_EXTABLE(0b,3b)                                        \
-               : "=&r"(res), "=&c"(count), "=&a" (__d0), "=&S" (__d1),    \
-                 "=&D" (__d2)                                             \
-               : "i"(-EFAULT), "0"(count), "1"(count), "3"(src), "4"(dst) \
-               : "memory");                                               \
-} while (0)
-
-long
-__strncpy_from_user(char *dst, const char __user *src, long count)
-{
-       long res;
-       __do_strncpy_from_user(dst, src, count, res);
-       return res;
-}
-EXPORT_SYMBOL(__strncpy_from_user);
-
-long
-strncpy_from_user(char *dst, const char __user *src, long count)
-{
-       long res = -EFAULT;
-       if (access_ok(VERIFY_READ, src, 1))
-               return __strncpy_from_user(dst, src, count);
-       return res;
-}
-EXPORT_SYMBOL(strncpy_from_user);
-
 /*
  * Zero Userspace
  */