gmane.linux.kernel.cross-arch

Re: [PATCH 1/4] [-next] Makefile: Do not call cc-option before including the arch's Makefile

Michal Marek — 2012-05-25T12:29:40

Dne 20.5.2012 11:27, Sam Ravnborg napsal(a):

The first patch has to go to the tip tree, where the CONFIG_READABLE_ASM
option is introduced. The remaining patches can go to the individual
arch trees, I see Geert already pushed the m68k one.

Michal

[PATCH 1/1] openrisc: use generic strncpy_from_user

Jonas Bonn — 2012-05-25T06:24:49

As per commits 2922585b93294d47172a765115e0dbc1bfe1be19
("lib: Sparc's strncpy_from_user is generic enough, move under lib/"),
92ae03f2ef99fbc23bfa9080d6b58f25227bd7ef ("x86: merge 32/64-bit versions
of 'strncpy_from_user()' and speed it up"), and corresponding
discussion on linux-arch.

Signed-off-by: Jonas Bonn <jonas< at >southpole.se>
---

Hi Linus,

Here's a single patch update to the OpenRISC arch to use the new
strncpy_from_user implementation.  Hope it's ok to send it as a
single patch and not as a pull request; let me know otherwise.

Thanks,
Jonas

 arch/openrisc/Kconfig               |    1 +
 arch/openrisc/include/asm/uaccess.h |   10 ++-----
 arch/openrisc/lib/string.S          |   52 -----------------------------------
 3 files changed, 4 insertions(+), 59 deletions(-)

diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index da4ccd1..ee4d124 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
< at >< at > -19,6 +19,7 < at >< at > config OPENRISC
 select GENERIC_CPU_DEVICES
 select GENERIC_ATOMIC64
 select GENERIC_CLOCKEVENTS
+select GENERIC_STRNCPY_FROM_USER
 
 config MMU
 def_bool y
diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index f5abaa0..04b93de 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
< at >< at > -313,14 +313,10 < at >< at > clear_user(void *addr, unsigned long size)
 return size;
 }
 
-extern int __strncpy_from_user(char *dst, const char *src, long count);
+#define user_addr_max() \
+(segment_eq(get_fs(), USER_DS) ? TASK_SIZE : ~0UL)
 
-static inline long strncpy_from_user(char *dst, const char *src, long count)
-{
-if (access_ok(VERIFY_READ, src, 1))
-return __strncpy_from_user(dst, src, count);
-return -EFAULT;
-}
+extern long strncpy_from_user(char *dest, const char __user *src, long count);
 
 /*
  * Return the size of a string (including the ending 0)
diff --git a/arch/openrisc/lib/string.S b/arch/openrisc/lib/string.S
index 465f04b..8ceb11e 100644
--- a/arch/openrisc/lib/string.S
+++ b/arch/openrisc/lib/string.S
< at >< at > -104,58 +104,6 < at >< at > __clear_user:
 .long 9b, 99b// write fault
 .previous
 
-/*
- * long strncpy_from_user(char *dst, const char *src, long count)
- *
- *
- */
-.global__strncpy_from_user
-__strncpy_from_user:
-l.addi  r1,r1,-16
-l.sw    0(r1),r6
-l.sw    4(r1),r5
-l.sw    8(r1),r4
-l.sw    12(r1),r3
-
-l.addi  r11,r5,0
-2:  l.sfeq  r5,r0
-l.bf    1f
-l.addi  r5,r5,-1
-8:    l.lbz   r6,0(r4)
-l.sfeq  r6,r0
-l.bf    1f
-9:    l.sb    0(r3),r6
-l.addi  r3,r3,1
-l.j     2b
-l.addi  r4,r4,1
-1:
-l.lwz   r6,0(r1)
-l.addi  r5,r5,1
-l.sub   r11,r11,r5              // r11 holds the return value
-
-l.lwz   r6,0(r1)
-l.lwz   r5,4(r1)
-l.lwz   r4,8(r1)
-l.lwz   r3,12(r1)
-l.jr    r9
-l.addi  r1,r1,16
-
-.section .fixup, "ax"
-99:
-l.movhi r11,hi(-EFAULT)
-l.ori   r11,r11,lo(-EFAULT)
-
-l.lwz   r6,0(r1)
-l.lwz   r5,4(r1)
-l.lwz   r4,8(r1)
-l.lwz   r3,12(r1)
-l.jrr9
-l.addi  r1,r1,16
-.previous
-
-.section __ex_table, "a"
-.long 8b, 99b// read fault
-.previous
 
 /*
  * extern int __strnlen_user(const char *str, long len, unsigned long top);

[PATCH 2/3 v2] kernel: Move REPEAT_BYTE definition into linux/kernel.h

David Miller — 2012-05-24T20:26:19

And make sure that everything using it explicitly includes
that header file.

Signed-off-by: David S. Miller <davem< at >davemloft.net>
---
 arch/sparc/lib/usercopy.c             |    3 +--
 arch/x86/include/asm/word-at-a-time.h |    4 ++--
 fs/namei.c                            |    1 +
 include/linux/kernel.h                |    2 ++
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/lib/usercopy.c b/arch/sparc/lib/usercopy.c
index f61ed82..0b12e91 100644
--- a/arch/sparc/lib/usercopy.c
+++ b/arch/sparc/lib/usercopy.c
< at >< at > -1,5 +1,6 < at >< at >
 #include <linux/module.h>
 #include <linux/uaccess.h>
+#include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/bug.h>
 
< at >< at > -11,8 +12,6 < at >< at > void copy_from_user_overflow(void)
 }
 EXPORT_SYMBOL(copy_from_user_overflow);
 
-#define REPEAT_BYTE(x)((~0ul / 0xff) * (x))
-
 static inline long find_zero(unsigned long mask)
 {
 long byte = 0;
diff --git a/arch/x86/include/asm/word-at-a-time.h b/arch/x86/include/asm/word-at-a-time.h
index e58f03b..ae03fac 100644
--- a/arch/x86/include/asm/word-at-a-time.h
+++ b/arch/x86/include/asm/word-at-a-time.h
< at >< at > -1,6 +1,8 < at >< at >
 #ifndef _ASM_WORD_AT_A_TIME_H
 #define _ASM_WORD_AT_A_TIME_H
 
+#include <linux/kernel.h>
+
 /*
  * This is largely generic for little-endian machines, but the
  * optimal byte mask counting is probably going to be something
< at >< at > -35,8 +37,6 < at >< at > static inline long count_masked_bytes(long mask)
 
 #endif
 
-#define REPEAT_BYTE(x)((~0ul / 0xff) * (x))
-
 /* Return the high bit set in the first byte that is a zero */
 static inline unsigned long has_zero(unsigned long a)
 {
diff --git a/fs/namei.c b/fs/namei.c
index f9e883c..8d2ba42 100644
--- a/fs/namei.c
+++ b/fs/namei.c
< at >< at > -16,6 +16,7 < at >< at >
 
 #include <linux/init.h>
 #include <linux/export.h>
+#include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/fs.h>
 #include <linux/namei.h>
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 645231c..fbe9bfa 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
< at >< at > -38,6 +38,8 < at >< at >
 
 #define STACK_MAGIC0xdeadbeef
 
+#define REPEAT_BYTE(x)((~0ul / 0xff) * (x))
+
 #define ALIGN(x, a)__ALIGN_KERNEL((x), (a))
 #define __ALIGN_MASK(x, mask)__ALIGN_KERNEL_MASK((x), (mask))
 #define PTR_ALIGN(p, a)((typeof(p))ALIGN((unsigned long)(p), (a)))

Re: [PATCH 1/3] sparc: Increase portability of strncpy_from_user() implementation.

David Miller — 2012-05-24T20:04:36

From: Linus Torvalds <torvalds< at >linux-foundation.org>
Date: Thu, 24 May 2012 09:04:27 -0700


Ok, I'll make this adjustment.

Re: [PATCH 3/3] lib: Sparc's strncpy_from_user is generic enough, move under lib/

David Miller — 2012-05-24T19:53:22

From: David Howells <dhowells< at >redhat.com>
Date: Thu, 24 May 2012 17:10:48 +0100


Thanks I'll fix that up.

Re: [PATCH 3/3] lib: Sparc's strncpy_from_user is generic enough, move under lib/

David Howells — 2012-05-24T17:17:52


Contingent on removing the extraneous semicolon:

Acked-by: David Howells <dhowells< at >redhat.com>

Re: Arch maintainers Ahoy!

David Howells — 2012-05-24T17:16:46


No, I didn't phrase the paragraph above very well, so let me rephrase: I think
Dave's patch is _good_[*] and that BSF/BSR would _not_ be usable for it.

I was thinking that reordering the bytes on LE might help, but no, it doesn't
as Dave's algorithm works equally well, no matter on the order the bytes are
in.

David

[*] Barring an extraneous semicolon already pointed out

Re: Arch maintainers Ahoy!

Linus Torvalds — 2012-05-24T16:56:34

David, I don't think you've followed this saga very well.

That word-at-a-time algorithm *comes* from x86. Literally. The code is
90% pure copies from arch/x86/lib/usercopy.c.

We *know* how the code should be written on little-endian, AND IT IS
BOTH BETTER AND FASTER there.

Seriously. Little-endian is *superior* for string handling. No
questions. In fact, anybody who thinks that big-endian is better for
*anything* is seriously deluded these days.

Also, BSF is too damn slow. It's slow as hell on most older x86 chips,
and it's pointless. You can do clever tricks (again - only on
little-endian) that do it portably without it. Grep for
count_masked_bytes in the current tree, which does need a fast
multiplier on 64-bit, but fast multipliers are way more common than
the fast bit scan instructions.

I'd love to use a population count instruction, but efficient popc
instructions are simply not widely enough available. And the bsf
instruction is very slow on old x86 microarchitectures.

                  Linus

Re: Arch maintainers Ahoy!

David Howells — 2012-05-24T16:45:29


I didn't suggest it was free, but it might be cheaper.  Besides x86/x86_64 has
BSF/BSR instructions - though having played with Dave's algorithm some, I
don't think they're usable for this.

My suggestion assumes that there would be zeros beyond the terminal NUL, which
isn't something you can rely on.

David

Re: [PATCH 1/3] sparc: Increase portability of strncpy_from_user() implementation.

Paul Mundt — 2012-05-24T16:25:00

SH-4A processors fall under this. All of them support efficient 32-bit
unaligned loads but no stores at all or loads of any other size. Having
said that, the compiler isn't exactly brilliant at figuring out when to
emit them, so I'd probably still have to do it by hand.

Re: [PATCH 3/3] lib: Sparc's strncpy_from_user is generic enough, move under lib/

David Howells — 2012-05-24T16:10:48


You've an extra semicolon there.

David

Re: [PATCH 1/3] sparc: Increase portability of strncpy_from_user() implementation.

Linus Torvalds — 2012-05-24T16:04:27

Btw, this would have been much cleaner with something like

   #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
      #define IS_UNALIGNED(src,dst) 0
   #else
      #define IS_UNALIGNED(src,dst) (((long)(src) | (long)(dst)) &
(sizeof(long) - 1))
   #endif

and then just

     if (IS_UNALIGNED(src,dst))
        goto byte_at_a_time;

in the source code. Those #ifdef's in the middle of the code really
are horribly ugly, and are unnecessary.

Besides, some architecture may actually have ok unaligned loads, and
just horrible unaligned stores, and it would be easier with that kind
of abstraction.

Not a big deal, but I thought I'd point it out when I shuddered when I
looked at it.

                      Linus

Re: Arch maintainers Ahoy!

Linus Torvalds — 2012-05-24T15:53:20

First off, the *last* thing you want to do is go to big-endian mode.
All the bit counting gets *much* more complicated, and your argument
that it's "free" on some architectures is pointless, since it is only
free on the architectures that have the *least* users.

Secondly, it's not "likely a single instruction" on LE, neither is
ffs. It can be, but it's often one of the slower instructions.

Many architectures will have - but only in their most recent uarch
versions - popcount or similar, and if you're little-endian, that
would be what you want. Except we already figured out faster versions
for little-endian based on multiplication or a few add/shift
operations.

                        Linus

Re: Arch maintainers Ahoy!

David Howells — 2012-05-24T09:40:16

Linus Torvalds <torvalds< at >linux-foundation.org> wrote:



Could you use cpu_to_be32/64() and then ffs()?  That ought to work for both
variants of endianness.  The cpu_to_beXX() should be a noop on BE and is
likely to be a single instruction on LE.  The meat of ffs() is usually a
single instruction, though it may have to have zero-detect logic added.

David

Re: Arch maintainers Ahoy!

David Miller — 2012-05-24T05:56:40

From: Paul Mackerras <paulus< at >samba.org>
Date: Thu, 24 May 2012 15:25:45 +1000


I know, guess whose glibc strlen asm I got these formulas from?
:-)


I'm sure a powerpc person can cons up a patch that does that once
my stuff goes in, and test it too.

Re: Arch maintainers Ahoy!

Paul Mackerras — 2012-05-24T05:25:45


Sounds good, thanks for doing this.  On PowerPC we have the "count
leading zeroes" instructions we could use to find the position of the
leftmost zero byte, so if you can conveniently make the find_zero()
function overridable, that would be good.

Paul.

[PATCH 3/3] lib: Sparc's strncpy_from_user is generic enough, move under lib/

David Miller — 2012-05-24T03:34:40

To use this, an architecture simply needs to:

1) Provide a user_addr_max() implementation via asm/uaccess.h

2) Add "select GENERIC_STRNCPY_FROM_USER" to their arch Kcnfig

3) Remove the existing strncpy_from_user() implementation and symbol
   exports their architecture had.

Signed-off-by: David S. Miller <davem< at >davemloft.net>
---
 arch/sparc/Kconfig        |    1 +
 arch/sparc/lib/usercopy.c |  138 --------------------------------------------
 lib/Kconfig               |    3 +
 lib/Makefile              |    2 +
 lib/strncpy_from_user.c   |  140 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 146 insertions(+), 138 deletions(-)
 create mode 100644 lib/strncpy_from_user.c

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 051af37..2247423 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
< at >< at > -32,6 +32,7 < at >< at > config SPARC
 select HAVE_NMI_WATCHDOG if SPARC64
 select HAVE_BPF_JIT
 select GENERIC_SMP_IDLE_THREAD
+select GENERIC_STRNCPY_FROM_USER
 
 config SPARC32
 def_bool !64BIT
diff --git a/arch/sparc/lib/usercopy.c b/arch/sparc/lib/usercopy.c
index 3d3bef9..5c4284c 100644
--- a/arch/sparc/lib/usercopy.c
+++ b/arch/sparc/lib/usercopy.c
< at >< at > -1,147 +1,9 < at >< at >
 #include <linux/module.h>
-#include <linux/uaccess.h>
 #include <linux/kernel.h>
-#include <linux/errno.h>
 #include <linux/bug.h>
 
-#include <asm/byteorder.h>
-
 void copy_from_user_overflow(void)
 {
 WARN(1, "Buffer overflow detected!\n");
 }
 EXPORT_SYMBOL(copy_from_user_overflow);
-
-static inline long find_zero(unsigned long mask)
-{
-long byte = 0;
-
-#ifdef __BIG_ENDIAN
-#ifdef CONFIG_64BIT
-if (mask >> 32)
-mask >>= 32;
-else
-byte = 4;
-#endif
-if (mask >> 16)
-mask >>= 16;
-else
-byte += 2;
-return (mask >> 8) ? byte : byte + 1;
-#else
-#ifdef CONFIG_64BIT
-if (!((unsigned int) mask)) {
-mask >>= 32;
-byte = 4;
-}
-#endif
-if (!(mask & 0xffff)) {
-mask >>= 16;
-byte += 2;
-}
-return (mask & 0xff) ? byte : byte + 1;
-#endif
-}
-
-/*
- * 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, unsigned long max)
-{
-const unsigned long high_bits = REPEAT_BYTE(0xfe) + 1;
-const unsigned long low_bits = REPEAT_BYTE(0x7f);
-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;
-#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-if (((long) dst | (long) src) & (sizeof(long) - 1))
-goto byte_at_a_time;
-#endif
-while (max >= sizeof(unsigned long)) {
-unsigned long c, v, rhs;
-
-/* Fall back to byte-at-a-time if we get a page fault */
-if (unlikely(__get_user(c,(unsigned long __user *)(src+res))))
-break;
-rhs = c | low_bits;
-v = (c + high_bits) & ~rhs;
-*(unsigned long *)(dst+res) = c;
-if (v) {
-v = (c & low_bits) + low_bits;;
-v = ~(v | rhs);
-return res + find_zero(v);
-}
-res += sizeof(unsigned long);
-max -= sizeof(unsigned long);
-}
-#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-byte_at_a_time:
-#endif
-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 res;
-
-/*
- * 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.
- * < at >dst:   Destination address, in kernel space.  This buffer must be at
- *         least < at >count bytes long.
- * < at >src:   Source address, in user space.
- * < at >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 < at >count is smaller than the length of the string, copies < at >count bytes
- * and returns < at >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 = user_addr_max();
-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);
diff --git a/lib/Kconfig b/lib/Kconfig
index 4a8aba2..49cb463 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
< at >< at > -16,6 +16,9 < at >< at > config BITREVERSE
 config RATIONAL
 boolean
 
+config GENERIC_STRNCPY_FROM_USER
+bool
+
 config GENERIC_FIND_FIRST_BIT
 bool
 
diff --git a/lib/Makefile b/lib/Makefile
index 18515f0..57538b4 100644
--- a/lib/Makefile
+++ b/lib/Makefile
< at >< at > -123,6 +123,8 < at >< at > obj-$(CONFIG_SIGNATURE) += digsig.o
 
 obj-$(CONFIG_CLZ_TAB) += clz_tab.o
 
+obj-$(CONFIG_GENERIC_STRNCPY_FROM_USER) += strncpy_from_user.o
+
 hostprogs-y:= gen_crc32table
 clean-files:= crc32table.h
 
diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
new file mode 100644
index 0000000..278de4a
--- /dev/null
+++ b/lib/strncpy_from_user.c
< at >< at > -0,0 +1,140 < at >< at >
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+
+#include <asm/byteorder.h>
+
+static inline long find_zero(unsigned long mask)
+{
+long byte = 0;
+
+#ifdef __BIG_ENDIAN
+#ifdef CONFIG_64BIT
+if (mask >> 32)
+mask >>= 32;
+else
+byte = 4;
+#endif
+if (mask >> 16)
+mask >>= 16;
+else
+byte += 2;
+return (mask >> 8) ? byte : byte + 1;
+#else
+#ifdef CONFIG_64BIT
+if (!((unsigned int) mask)) {
+mask >>= 32;
+byte = 4;
+}
+#endif
+if (!(mask & 0xffff)) {
+mask >>= 16;
+byte += 2;
+}
+return (mask & 0xff) ? byte : byte + 1;
+#endif
+}
+
+/*
+ * 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, unsigned long max)
+{
+const unsigned long high_bits = REPEAT_BYTE(0xfe) + 1;
+const unsigned long low_bits = REPEAT_BYTE(0x7f);
+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;
+#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+if (((long) dst | (long) src) & (sizeof(long) - 1))
+goto byte_at_a_time;
+#endif
+while (max >= sizeof(unsigned long)) {
+unsigned long c, v, rhs;
+
+/* Fall back to byte-at-a-time if we get a page fault */
+if (unlikely(__get_user(c,(unsigned long __user *)(src+res))))
+break;
+rhs = c | low_bits;
+v = (c + high_bits) & ~rhs;
+*(unsigned long *)(dst+res) = c;
+if (v) {
+v = (c & low_bits) + low_bits;;
+v = ~(v | rhs);
+return res + find_zero(v);
+}
+res += sizeof(unsigned long);
+max -= sizeof(unsigned long);
+}
+#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+byte_at_a_time:
+#endif
+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 res;
+
+/*
+ * 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.
+ * < at >dst:   Destination address, in kernel space.  This buffer must be at
+ *         least < at >count bytes long.
+ * < at >src:   Source address, in user space.
+ * < at >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 < at >count is smaller than the length of the string, copies < at >count bytes
+ * and returns < at >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 = user_addr_max();
+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);

[PATCH 1/3] sparc: Increase portability of strncpy_from_user() implementation.

David Miller — 2012-05-24T03:34:30

Hide details of maximum user address calculation in a new
asm/uaccess.h interface named user_addr_max().

Provide little-endian implementation in find_zero(), which should work
but can probably be improved.

Guard alignment check using CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.

Signed-off-by: David S. Miller <davem< at >davemloft.net>
---
 arch/sparc/include/asm/uaccess.h |    3 +++
 arch/sparc/lib/usercopy.c        |   28 ++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/arch/sparc/include/asm/uaccess.h b/arch/sparc/include/asm/uaccess.h
index 42a28cf..20c2acb 100644
--- a/arch/sparc/include/asm/uaccess.h
+++ b/arch/sparc/include/asm/uaccess.h
< at >< at > -6,6 +6,9 < at >< at >
 #include <asm/uaccess_32.h>
 #endif
 
+#define user_addr_max() \
+(segment_eq(get_fs(), USER_DS) ? STACK_TOP : ~0UL)
+
 extern long strncpy_from_user(char *dest, const char __user *src, long count);
 
 #endif
diff --git a/arch/sparc/lib/usercopy.c b/arch/sparc/lib/usercopy.c
index 87f9645..9ba59ef 100644
--- a/arch/sparc/lib/usercopy.c
+++ b/arch/sparc/lib/usercopy.c
< at >< at > -3,6 +3,8 < at >< at >
 #include <linux/errno.h>
 #include <linux/bug.h>
 
+#include <asm/byteorder.h>
+
 void copy_from_user_overflow(void)
 {
 WARN(1, "Buffer overflow detected!\n");
< at >< at > -14,6 +16,8 < at >< at > EXPORT_SYMBOL(copy_from_user_overflow);
 static inline long find_zero(unsigned long mask)
 {
 long byte = 0;
+
+#ifdef __BIG_ENDIAN
 #ifdef CONFIG_64BIT
 if (mask >> 32)
 mask >>= 32;
< at >< at > -25,6 +29,19 < at >< at > static inline long find_zero(unsigned long mask)
 else
 byte += 2;
 return (mask >> 8) ? byte : byte + 1;
+#else
+#ifdef CONFIG_64BIT
+if (!((unsigned int) mask)) {
+mask >>= 32;
+byte = 4;
+}
+#endif
+if (!(mask & 0xffff)) {
+mask >>= 16;
+byte += 2;
+}
+return (mask & 0xff) ? byte : byte + 1;
+#endif
 }
 
 /*
< at >< at > -45,10 +62,10 < at >< at > static inline long do_strncpy_from_user(char *dst, const char __user *src, long
  */
 if (max > count)
 max = count;
-
+#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
 if (((long) dst | (long) src) & (sizeof(long) - 1))
 goto byte_at_a_time;
-
+#endif
 while (max >= sizeof(unsigned long)) {
 unsigned long c, v, rhs;
 
< at >< at > -66,8 +83,9 < at >< at > static inline long do_strncpy_from_user(char *dst, const char __user *src, long
 res += sizeof(unsigned long);
 max -= sizeof(unsigned long);
 }
-
+#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
 byte_at_a_time:
+#endif
 while (max) {
 char c;
 
< at >< at > -119,9 +137,7 < at >< at > long strncpy_from_user(char *dst, const char __user *src, long count)
 if (unlikely(count <= 0))
 return 0;
 
-max_addr = ~0UL;
-if (likely(segment_eq(get_fs(), USER_DS)))
-max_addr = STACK_TOP;
+max_addr = user_addr_max();
 src_addr = (unsigned long)src;
 if (likely(src_addr < max_addr)) {
 unsigned long max = max_addr - src_addr;

[PATCH 2/3] kernel: Move REPEAT_BYTE definition into linux/kernel.h

David Miller — 2012-05-24T03:34:35

And make sure that everything using it explicitly includes
that header file.

Signed-off-by: David S. Miller <davem< at >davemloft.net>
---
 arch/sparc/lib/usercopy.c             |    3 +--
 arch/x86/include/asm/word-at-a-time.h |    4 ++--
 fs/namei.c                            |    1 +
 include/linux/kernel.h                |    2 ++
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/lib/usercopy.c b/arch/sparc/lib/usercopy.c
index 9ba59ef..3d3bef9 100644
--- a/arch/sparc/lib/usercopy.c
+++ b/arch/sparc/lib/usercopy.c
< at >< at > -1,5 +1,6 < at >< at >
 #include <linux/module.h>
 #include <linux/uaccess.h>
+#include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/bug.h>
 
< at >< at > -11,8 +12,6 < at >< at > void copy_from_user_overflow(void)
 }
 EXPORT_SYMBOL(copy_from_user_overflow);
 
-#define REPEAT_BYTE(x)((~0ul / 0xff) * (x))
-
 static inline long find_zero(unsigned long mask)
 {
 long byte = 0;
diff --git a/arch/x86/include/asm/word-at-a-time.h b/arch/x86/include/asm/word-at-a-time.h
index e58f03b..ae03fac 100644
--- a/arch/x86/include/asm/word-at-a-time.h
+++ b/arch/x86/include/asm/word-at-a-time.h
< at >< at > -1,6 +1,8 < at >< at >
 #ifndef _ASM_WORD_AT_A_TIME_H
 #define _ASM_WORD_AT_A_TIME_H
 
+#include <linux/kernel.h>
+
 /*
  * This is largely generic for little-endian machines, but the
  * optimal byte mask counting is probably going to be something
< at >< at > -35,8 +37,6 < at >< at > static inline long count_masked_bytes(long mask)
 
 #endif
 
-#define REPEAT_BYTE(x)((~0ul / 0xff) * (x))
-
 /* Return the high bit set in the first byte that is a zero */
 static inline unsigned long has_zero(unsigned long a)
 {
diff --git a/fs/namei.c b/fs/namei.c
index f9e883c..8d2ba42 100644
--- a/fs/namei.c
+++ b/fs/namei.c
< at >< at > -16,6 +16,7 < at >< at >
 
 #include <linux/init.h>
 #include <linux/export.h>
+#include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/fs.h>
 #include <linux/namei.h>
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 645231c..fbe9bfa 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
< at >< at > -38,6 +38,8 < at >< at >
 
 #define STACK_MAGIC0xdeadbeef
 
+#define REPEAT_BYTE(x)((~0ul / 0xff) * (x))
+
 #define ALIGN(x, a)__ALIGN_KERNEL((x), (a))
 #define __ALIGN_MASK(x, mask)__ALIGN_KERNEL_MASK((x), (mask))
 #define PTR_ALIGN(p, a)((typeof(p))ALIGN((unsigned long)(p), (a)))

[PATCH 0/3] Make sparc's strncpy_from_user() generic.

David Miller — 2012-05-24T03:34:18

Ok, this is against my current sparc GIT tree at:

git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git

After these changes it should definitely be usable by any
architecture.

If there are no major objections I can push this out to my sparc tree
and ask Linus to pull all of this work in.

Afterwards we can tweak it to use the x86 word-at-a-time tricks if we
want.

Thanks.

Re: Arch maintainers Ahoy!

David Miller — 2012-05-24T02:11:50

From: Linus Torvalds <torvalds< at >linux-foundation.org>
Date: Wed, 23 May 2012 14:01:26 -0700


Ok, now confirmed, the following patch generates the best code I've
been able to get out of GCC thus far, and it's tested too.

Therefore, I'll push this out to my sparc tree.

Next, I'll try to add the abstractions et al. necessary to put this
under the top-level lib/ so others can use it too.

diff --git a/arch/sparc/lib/usercopy.c b/arch/sparc/lib/usercopy.c
index 851cb75..78c363f 100644
--- a/arch/sparc/lib/usercopy.c
+++ b/arch/sparc/lib/usercopy.c
< at >< at > -17,29 +17,20 < at >< at > static inline unsigned long has_zero(unsigned long a)
 return ((a - REPEAT_BYTE(0x01)) & ~a) & REPEAT_BYTE(0x80);
 }
 
-static inline long find_zero(unsigned long c)
+static inline long find_zero(unsigned long mask)
 {
+long byte = 0;
 #ifdef CONFIG_64BIT
-if (!(c & 0xff00000000000000UL))
-return 0;
-if (!(c & 0x00ff000000000000UL))
-return 1;
-if (!(c & 0x0000ff0000000000UL))
-return 2;
-if (!(c & 0x000000ff00000000UL))
-return 3;
-#define __OFF 4
-#else
-#define __OFF 0
+if (mask >> 32)
+mask >>= 32;
+else
+byte = 4;
 #endif
-if (!(c & 0xff000000))
-return __OFF + 0;
-if (!(c & 0x00ff0000))
-return __OFF + 1;
-if (!(c & 0x0000ff00))
-return __OFF + 2;
-return __OFF + 3;
-#undef __OFF
+if (mask >> 16)
+mask >>= 16;
+else
+byte += 2;
+return (mask >> 8) ? byte : byte + 1;
 }
 
 /*
< at >< at > -50,6 +41,8 < at >< at > static inline long find_zero(unsigned long c)
  */
 static inline long do_strncpy_from_user(char *dst, const char __user *src, long count, unsigned long max)
 {
+const unsigned long high_bits = REPEAT_BYTE(0xfe) + 1;
+const unsigned long low_bits = REPEAT_BYTE(0x7f);
 long res = 0;
 
 /*
< at >< at > -63,14 +56,19 < at >< at > static inline long do_strncpy_from_user(char *dst, const char __user *src, long
 goto byte_at_a_time;
 
 while (max >= sizeof(unsigned long)) {
-unsigned long c;
+unsigned long c, v, rhs;
 
 /* Fall back to byte-at-a-time if we get a page fault */
 if (unlikely(__get_user(c,(unsigned long __user *)(src+res))))
 break;
+rhs = c | low_bits;
+v = (c + high_bits) & ~rhs;
 *(unsigned long *)(dst+res) = c;
-if (has_zero(c))
-return res + find_zero(c);
+if (v) {
+v = (c & low_bits) + low_bits;;
+v = ~(v | rhs);
+return res + find_zero(v);
+}
 res += sizeof(unsigned long);
 max -= sizeof(unsigned long);
 }