Skip to contentSkip to conversation

asm-generic: strncpy_from_user/strnlen_user cleanup

PATCH Superseded Arnd Bergmann 9 participants 31 messages Jul 22, 2021 – Aug 14, 2021

Subsystems (22)
Arm (32-bit) x86 PowerPC Library code +18 more subsystems
lore
Share & tools
lore
#0 lore raw

v3 cover asm-generic: strncpy_from_user/strnlen_user cleanup

From: Arnd Bergmann <[email protected]>

I had run into some regressions for the previous version of this series, the new version is based on v5.14-rc2 instead.

These two functions appear to be unnecessarily different between architectures, and the asm-generic version is a bit questionable, even for NOMMU architectures.

Clean this up to just use the generic library version for anything that uses the generic version today. I've expanded on the patch descriptions a little, as suggested by Christoph Hellwig, but I suspect a more detailed review would uncover additional problems with the custom versions that are getting removed.

I ended up adding patches for csky and microblaze as they had the same implementation that I removed elsewhere, these are now gone as well.

If I hear no objections from architecture maintainers or new build regressions, I'll queue these up in the asm-generic tree for 5.15.

Arnd

Arnd Bergmann (9):
asm-generic/uaccess.h: remove __strncpy_from_user/__strnlen_user
h8300: remove stale strncpy_from_user
hexagon: use generic strncpy/strnlen from_user
arc: use generic strncpy/strnlen from_user
csky: use generic strncpy/strnlen from_user
microblaze: use generic strncpy/strnlen from_user
asm-generic: uaccess: remove inline strncpy_from_user/strnlen_user
asm-generic: remove extra strn{cpy_from,len}_user declarations
asm-generic: reverse GENERIC_{STRNCPY_FROM,STRNLEN}_USER symbols

 arch/alpha/Kconfig                        |   2 -
 arch/arc/include/asm/uaccess.h            |  72 -------------
 arch/arc/mm/extable.c                     |  12 ---
 arch/arm/Kconfig                          |   2 -
 arch/arm64/Kconfig                        |   2 -
 arch/csky/include/asm/uaccess.h           |   6 --
 arch/csky/lib/usercopy.c                  | 102 ------------------
 arch/h8300/kernel/h8300_ksyms.c           |   2 -
 arch/h8300/lib/Makefile                   |   2 +-
 arch/h8300/lib/strncpy.S                  |  35 ------
 arch/hexagon/include/asm/uaccess.h        |  31 ------
 arch/hexagon/kernel/hexagon_ksyms.c       |   1 -
 arch/hexagon/mm/Makefile                  |   2 +-
 arch/hexagon/mm/strnlen_user.S            | 126 ----------------------
 arch/ia64/Kconfig                         |   2 +
 arch/m68k/Kconfig                         |   2 -
 arch/microblaze/include/asm/uaccess.h     |  19 +---
 arch/microblaze/kernel/microblaze_ksyms.c |   3 -
 arch/microblaze/lib/uaccess_old.S         |  90 ----------------
 arch/mips/Kconfig                         |   2 +
 arch/nds32/Kconfig                        |   2 -
 arch/nios2/Kconfig                        |   2 -
 arch/openrisc/Kconfig                     |   2 -
 arch/parisc/Kconfig                       |   2 +-
 arch/powerpc/Kconfig                      |   2 -
 arch/riscv/Kconfig                        |   2 -
 arch/s390/Kconfig                         |   2 +
 arch/sh/Kconfig                           |   2 -
 arch/sparc/Kconfig                        |   2 -
 arch/um/Kconfig                           |   2 +
 arch/um/include/asm/uaccess.h             |   5 +-
 arch/um/kernel/skas/uaccess.c             |  14 ++-
 arch/x86/Kconfig                          |   2 -
 arch/xtensa/Kconfig                       |   3 +-
 arch/xtensa/include/asm/uaccess.h         |   3 +-
 include/asm-generic/uaccess.h             |  52 ++-------
 lib/Kconfig                               |  10 +-
 37 files changed, 43 insertions(+), 581 deletions(-)
 delete mode 100644 arch/h8300/lib/strncpy.S
 delete mode 100644 arch/hexagon/mm/strnlen_user.S
Signature · 33 lines
2.29.2

Cc: "James E.J. Bottomley" <[email protected]> 
Cc: Al Viro <[email protected]> 
Cc: Anton Ivanov <[email protected]> 
Cc: Brian Cain <[email protected]> 
Cc: Chris Zankel <[email protected]> 
Cc: Christian Borntraeger <[email protected]> 
Cc: Christoph Hellwig <[email protected]>
Cc: Guo Ren <[email protected]> 
Cc: Heiko Carstens <[email protected]> 
Cc: Helge Deller <[email protected]> 
Cc: Jeff Dike <[email protected]> 
Cc: Linus Walleij <[email protected]> 
Cc: Max Filippov <[email protected]> 
Cc: Michal Simek <[email protected]> 
Cc: Richard Weinberger <[email protected]> 
Cc: Thomas Bogendoerfer <[email protected]> 
Cc: Vasily Gorbik <[email protected]> 
Cc: Vineet Gupta <[email protected]> 
Cc: Yoshinori Sato <[email protected]> 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
Cc: [email protected] 
#1 lore raw

PATCH v3 1/9 asm-generic/uaccess.h: remove __strncpy_from_user/__strnlen_user

From: Arnd Bergmann <[email protected]>

This is a preparation for changing over architectures to the generic implementation one at a time. As there are no callers of either __strncpy_from_user() or __strnlen_user(), fold these into the strncpy_from_user() and strnlen_user() functions to make each implementation independent of the others.

Many of these implementations have known bugs, but the intention here is to not change behavior at all and stay compatible with those bugs for the moment.

---
 arch/arc/include/asm/uaccess.h     | 14 ++++++++++----
 arch/csky/include/asm/uaccess.h    |  8 ++++----
 arch/csky/lib/usercopy.c           | 14 ++++++++++----
 arch/hexagon/include/asm/uaccess.h | 22 +++++++++++++---------
 arch/um/include/asm/uaccess.h      |  8 ++++----
 arch/um/kernel/skas/uaccess.c      | 14 ++++++++++----
 include/asm-generic/uaccess.h      | 28 +++++++++++-----------------
 7 files changed, 62 insertions(+), 46 deletions(-)
arch/arc/include/asm/uaccess.h +10 -4
index ea40ec7f6cae..3476348f361e 100644
@@ -661,6 +661,9 @@ __arc_strncpy_from_user(char *dst, const char __user *src, long count)
 	long res = 0;
 	char val;
 
+	if (!access_ok(src, 1))
+		return -EFAULT;
+
 	if (count == 0)
 		return 0;
 
@@ -693,6 +696,9 @@ static inline long __arc_strnlen_user(const char __user *s, long n)
 	long res, tmp1, cnt;
 	char val;
 
+	if (!access_ok(s, 1))
+		return 0;
+
 	__asm__ __volatile__(
 	"	mov %2, %1			\n"
 	"1:	ldb.ab  %3, [%0, 1]		\n"
@@ -724,8 +730,8 @@ static inline long __arc_strnlen_user(const char __user *s, long n)
 #define INLINE_COPY_FROM_USER
 
 #define __clear_user(d, n)		__arc_clear_user(d, n)
-#define __strncpy_from_user(d, s, n)	__arc_strncpy_from_user(d, s, n)
-#define __strnlen_user(s, n)		__arc_strnlen_user(s, n)
+#define strncpy_from_user(d, s, n)	__arc_strncpy_from_user(d, s, n)
+#define strnlen_user(s, n)		__arc_strnlen_user(s, n)
 #else
 extern unsigned long arc_clear_user_noinline(void __user *to,
 		unsigned long n);
@@ -734,8 +740,8 @@ extern long arc_strncpy_from_user_noinline (char *dst, const char __user *src,
 extern long arc_strnlen_user_noinline(const char __user *src, long n);
 
 #define __clear_user(d, n)		arc_clear_user_noinline(d, n)
-#define __strncpy_from_user(d, s, n)	arc_strncpy_from_user_noinline(d, s, n)
-#define __strnlen_user(s, n)		arc_strnlen_user_noinline(s, n)
+#define strncpy_from_user(d, s, n)	arc_strncpy_from_user_noinline(d, s, n)
+#define strnlen_user(s, n)		arc_strnlen_user_noinline(s, n)
 
 #endif
 
arch/csky/include/asm/uaccess.h +4 -4
index ac83823fc437..e17c02a6709f 100644
@@ -209,11 +209,11 @@ unsigned long raw_copy_to_user(void *to, const void *from, unsigned long n);
 unsigned long __clear_user(void __user *to, unsigned long n);
 #define __clear_user __clear_user
 
-long __strncpy_from_user(char *dst, const char *src, long count);
-#define __strncpy_from_user __strncpy_from_user
+long strncpy_from_user(char *dst, const char *src, long count);
+#define strncpy_from_user strncpy_from_user
 
-long __strnlen_user(const char *s, long n);
-#define __strnlen_user __strnlen_user
+long strnlen_user(const char *s, long n);
+#define strnlen_user strnlen_user
 
 #include <asm/segment.h>
 #include <asm-generic/uaccess.h>
arch/csky/lib/usercopy.c +10 -4
index c5d394a0ae78..938b750d2fb1 100644
@@ -163,11 +163,14 @@ EXPORT_SYMBOL(raw_copy_to_user);
  * If @count is smaller than the length of the string, copies @count bytes
  * and returns @count.
  */
-long __strncpy_from_user(char *dst, const char *src, long count)
+long strncpy_from_user(char *dst, const char *src, long count)
 {
 	long res, faultres;
 	int tmp;
 
+	if (!access_ok(src, 1))
+		return -EFAULT;
+
 	__asm__ __volatile__(
 	"       cmpnei  %3, 0           \n"
 	"       bf      4f              \n"
@@ -198,7 +201,7 @@ long __strncpy_from_user(char *dst, const char *src, long count)
 
 	return res;
 }
-EXPORT_SYMBOL(__strncpy_from_user);
+EXPORT_SYMBOL(strncpy_from_user);
 
 /*
  * strnlen_user: - Get the size of a string in user space.
@@ -211,10 +214,13 @@ EXPORT_SYMBOL(__strncpy_from_user);
  * On exception, returns 0.
  * If the string is too long, returns a value greater than @n.
  */
-long __strnlen_user(const char *s, long n)
+long strnlen_user(const char *s, long n)
 {
 	unsigned long res, tmp;
 
+	if (!access_ok(src, 1))
+		return -EFAULT;
+
 	__asm__ __volatile__(
 	"       cmpnei  %1, 0           \n"
 	"       bf      3f              \n"
@@ -242,7 +248,7 @@ long __strnlen_user(const char *s, long n)
 
 	return res;
 }
-EXPORT_SYMBOL(__strnlen_user);
+EXPORT_SYMBOL(strnlen_user);
 
 /*
  * __clear_user: - Zero a block of memory in user space, with less checking.
arch/hexagon/include/asm/uaccess.h +13 -9
index c1019a736ff1..59aa3a50744f 100644
@@ -57,23 +57,27 @@ unsigned long raw_copy_to_user(void __user *to, const void *from,
 __kernel_size_t __clear_user_hexagon(void __user *dest, unsigned long count);
 #define __clear_user(a, s) __clear_user_hexagon((a), (s))
 
-#define __strncpy_from_user(dst, src, n) hexagon_strncpy_from_user(dst, src, n)
+extern long __strnlen_user(const char __user *src, long n);
 
-/*  get around the ifndef in asm-generic/uaccess.h  */
-#define __strnlen_user __strnlen_user
+static inline strnlen_user(const char __user *src, long n)
+{
+        if (!access_ok(src, 1))
+		return 0;
 
-extern long __strnlen_user(const char __user *src, long n);
+	return __strnlen_user(src, n);
+}
+/*  get around the ifndef in asm-generic/uaccess.h  */
+#define strnlen_user strnlen_user
 
-static inline long hexagon_strncpy_from_user(char *dst, const char __user *src,
-					     long n);
+static inline long strncpy_from_user(char *dst, const char __user *src, long n);
+#define strncpy_from_user strncpy_from_user
 
 #include <asm-generic/uaccess.h>
 
 /*  Todo:  an actual accelerated version of this.  */
-static inline long hexagon_strncpy_from_user(char *dst, const char __user *src,
-					     long n)
+static inline long strncpy_from_user(char *dst, const char __user *src, long n)
 {
-	long res = __strnlen_user(src, n);
+	long res = strnlen_user(src, n);
 
 	if (unlikely(!res))
 		return -EFAULT;
arch/um/include/asm/uaccess.h +4 -4
index fe66d659acad..3bf209f683f8 100644
@@ -23,16 +23,16 @@
 
 extern unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n);
 extern unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n);
-extern long __strncpy_from_user(char *dst, const char __user *src, long count);
-extern long __strnlen_user(const void __user *str, long len);
+extern long strncpy_from_user(char *dst, const char __user *src, long count);
+extern long strnlen_user(const void __user *str, long len);
 extern unsigned long __clear_user(void __user *mem, unsigned long len);
 static inline int __access_ok(unsigned long addr, unsigned long size);
 
 /* Teach asm-generic/uaccess.h that we have C functions for these. */
 #define __access_ok __access_ok
 #define __clear_user __clear_user
-#define __strnlen_user __strnlen_user
-#define __strncpy_from_user __strncpy_from_user
+#define strnlen_user strnlen_user
+#define strncpy_from_user strncpy_from_user
 #define INLINE_COPY_FROM_USER
 #define INLINE_COPY_TO_USER
 
arch/um/kernel/skas/uaccess.c +10 -4
index 6c76df96e858..fc2c7e961e72 100644
@@ -189,11 +189,14 @@ static int strncpy_chunk_from_user(unsigned long from, int len, void *arg)
 	return 0;
 }
 
-long __strncpy_from_user(char *dst, const char __user *src, long count)
+long strncpy_from_user(char *dst, const char __user *src, long count)
 {
 	long n;
 	char *ptr = dst;
 
+	if (!access_ok(src, 1))
+		return -EFAULT;
+
 	if (uaccess_kernel()) {
 		strncpy(dst, (__force void *) src, count);
 		return strnlen(dst, count);
@@ -205,7 +208,7 @@ long __strncpy_from_user(char *dst, const char __user *src, long count)
 		return -EFAULT;
 	return strnlen(dst, count);
 }
-EXPORT_SYMBOL(__strncpy_from_user);
+EXPORT_SYMBOL(strncpy_from_user);
 
 static int clear_chunk(unsigned long addr, int len, void *unused)
 {
@@ -236,10 +239,13 @@ static int strnlen_chunk(unsigned long str, int len, void *arg)
 	return 0;
 }
 
-long __strnlen_user(const void __user *str, long len)
+long strnlen_user(const void __user *str, long len)
 {
 	int count = 0, n;
 
+	if (!access_ok(src, 1))
+		return -EFAULT;
+
 	if (uaccess_kernel())
 		return strnlen((__force char*)str, len) + 1;
 
@@ -248,7 +254,7 @@ long __strnlen_user(const void __user *str, long len)
 		return count + 1;
 	return 0;
 }
-EXPORT_SYMBOL(__strnlen_user);
+EXPORT_SYMBOL(strnlen_user);
 
 /**
  * arch_futex_atomic_op_inuser() - Atomic arithmetic operation with constant
include/asm-generic/uaccess.h +11 -17
index a0b2f270dddc..2f8a5d3bbd57 100644
@@ -246,11 +246,15 @@ extern int __get_user_bad(void) __attribute__((noreturn));
 /*
  * Copy a null terminated string from userspace.
  */
-#ifndef __strncpy_from_user
+#ifndef strncpy_from_user
 static inline long
-__strncpy_from_user(char *dst, const char __user *src, long count)
+strncpy_from_user(char *dst, const char __user *src, long count)
 {
 	char *tmp;
+
+	if (!access_ok(src, 1))
+		return -EFAULT;
+
 	strncpy(dst, (const char __force *)src, count);
 	for (tmp = dst; *tmp && count > 0; tmp++, count--)
 		;
@@ -258,24 +262,12 @@ __strncpy_from_user(char *dst, const char __user *src, long count)
 }
 #endif
 
-static inline long
-strncpy_from_user(char *dst, const char __user *src, long count)
-{
-	if (!access_ok(src, 1))
-		return -EFAULT;
-	return __strncpy_from_user(dst, src, count);
-}
-
+#ifndef strnlen_user
 /*
  * Return the size of a string (including the ending 0)
  *
  * Return 0 on exception, a value greater than N if too long
- */
-#ifndef __strnlen_user
-#define __strnlen_user(s, n) (strnlen((s), (n)) + 1)
-#endif
-
-/*
+ *
  * Unlike strnlen, strnlen_user includes the nul terminator in
  * its returned count. Callers should check for a returned value
  * greater than N as an indication the string is too long.
@@ -284,8 +276,10 @@ static inline long strnlen_user(const char __user *src, long n)
 {
 	if (!access_ok(src, 1))
 		return 0;
-	return __strnlen_user(src, n);
+
+	return strnlen(src, n) + 1;
 }
+#endif
 
 /*
  * Zero Userspace
Signature · 1 line
2.29.2
Reviewed-by on v3 1/9
#2 lore raw

Re: [PATCH v3 1/9] asm-generic/uaccess.h: remove __strncpy_from_user/__strnlen_user

Looks good,

Note that the uml version has a minor conflict with my pending set_fs() removal for uml, but that should be easy to resolve.

#3 lore raw

PATCH v3 2/9 h8300: remove stale strncpy_from_user

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

This function is never called because h8300 uses the asm-generic inline function version.

---
 arch/h8300/kernel/h8300_ksyms.c |  2 --
 arch/h8300/lib/Makefile         |  2 +-
 arch/h8300/lib/strncpy.S        | 35 ---------------------------------
 3 files changed, 1 insertion(+), 38 deletions(-)
 delete mode 100644 arch/h8300/lib/strncpy.S
arch/h8300/kernel/h8300_ksyms.c +0 -2
index 1c6f902e82a5..853d6e886477 100644
@@ -19,7 +19,6 @@ asmlinkage long __mulsi3(long, long);
 asmlinkage long __udivsi3(long, long);
 asmlinkage void *memcpy(void *, const void *, size_t);
 asmlinkage void *memset(void *, int, size_t);
-asmlinkage long strncpy_from_user(void *to, void *from, size_t n);
 
 	/* gcc lib functions */
 EXPORT_SYMBOL(__ucmpdi2);
@@ -34,4 +33,3 @@ EXPORT_SYMBOL(__mulsi3);
 EXPORT_SYMBOL(__udivsi3);
 EXPORT_SYMBOL(memcpy);
 EXPORT_SYMBOL(memset);
-EXPORT_SYMBOL(strncpy_from_user);
arch/h8300/lib/Makefile +1 -1
index 685fa837c1f7..5911c1fa856d 100644
@@ -3,7 +3,7 @@
 # Makefile for H8/300-specific library files..
 #
 
-lib-y  = memcpy.o memset.o abs.o strncpy.o \
+lib-y  = memcpy.o memset.o abs.o \
 	 mulsi3.o udivsi3.o muldi3.o moddivsi3.o \
 	 ashldi3.o lshrdi3.o ashrdi3.o ucmpdi2.o \
 	 delay.o
arch/h8300/lib/strncpy.S deleted +0 -35
deleted file mode 100644
index 8b65d7c4727b..000000000000
@@ -1,35 +0,0 @@
-;;; SPDX-License-Identifier: GPL-2.0
-;;; strncpy.S
-
-#include <asm/linkage.h>
-
-	.text
-.global strncpy_from_user
-
-;;; long strncpy_from_user(void *to, void *from, size_t n)
-strncpy_from_user:
-	mov.l	er2,er2
-	bne	1f
-	sub.l	er0,er0
-	rts
-1:
-	mov.l	er4,@-sp
-	sub.l	er3,er3
-2:
-	mov.b	@er1+,r4l
-	mov.b	r4l,@er0
-	adds	#1,er0
-	beq	3f
-	inc.l	#1,er3
-	dec.l	#1,er2
-	bne	2b
-3:
-	dec.l	#1,er2
-4:
-	mov.b	r4l,@er0
-	adds	#1,er0
-	dec.l	#1,er2
-	bne	4b
-	mov.l	er3,er0
-	mov.l	@sp+,er4
-	rts
Signature · 1 line
2.29.2
#5 lore raw

PATCH v3 3/9 hexagon: use generic strncpy/strnlen from_user

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the hexagon implementation of strncpy/strnlen and instead use the generic version. The hexagon version reads the data twice for strncpy() by doing an extra strnlen(), and it apparently lacks a check for user_addr_max().

---
 arch/hexagon/Kconfig                |   2 +
 arch/hexagon/include/asm/uaccess.h  |  33 +-------
 arch/hexagon/kernel/hexagon_ksyms.c |   1 -
 arch/hexagon/mm/Makefile            |   2 +-
 arch/hexagon/mm/strnlen_user.S      | 126 ----------------------------
 5 files changed, 5 insertions(+), 159 deletions(-)
 delete mode 100644 arch/hexagon/mm/strnlen_user.S
arch/hexagon/Kconfig +2 -0
index e5a852080730..3bf4845fed4b 100644
@@ -19,6 +19,8 @@ config HEXAGON
 	# GENERIC_ALLOCATOR is used by dma_alloc_coherent()
 	select GENERIC_ALLOCATOR
 	select GENERIC_IRQ_SHOW
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_TRACEHOOK
 	select NEED_SG_DMA_LENGTH
arch/hexagon/include/asm/uaccess.h +2 -31
index 59aa3a50744f..d950df12d8c5 100644
@@ -57,42 +57,13 @@ unsigned long raw_copy_to_user(void __user *to, const void *from,
 __kernel_size_t __clear_user_hexagon(void __user *dest, unsigned long count);
 #define __clear_user(a, s) __clear_user_hexagon((a), (s))
 
-extern long __strnlen_user(const char __user *src, long n);
-
-static inline strnlen_user(const char __user *src, long n)
-{
-        if (!access_ok(src, 1))
-		return 0;
-
-	return __strnlen_user(src, n);
-}
-/*  get around the ifndef in asm-generic/uaccess.h  */
+extern long strnlen_user(const char __user *src, long n);
 #define strnlen_user strnlen_user
 
-static inline long strncpy_from_user(char *dst, const char __user *src, long n);
+extern long strncpy_from_user(char *dst, const char __user *src, long n)
 #define strncpy_from_user strncpy_from_user
 
 #include <asm-generic/uaccess.h>
 
-/*  Todo:  an actual accelerated version of this.  */
-static inline long strncpy_from_user(char *dst, const char __user *src, long n)
-{
-	long res = strnlen_user(src, n);
-
-	if (unlikely(!res))
-		return -EFAULT;
-
-	if (res > n) {
-		long left = raw_copy_from_user(dst, src, n);
-		if (unlikely(left))
-			memset(dst + (n - left), 0, left);
-		return n;
-	} else {
-		long left = raw_copy_from_user(dst, src, res);
-		if (unlikely(left))
-			memset(dst + (res - left), 0, left);
-		return res-1;
-	}
-}
 
 #endif
arch/hexagon/kernel/hexagon_ksyms.c +0 -1
index 35545a7386a0..ec56ce2d92a2 100644
@@ -15,7 +15,6 @@ EXPORT_SYMBOL(__clear_user_hexagon);
 EXPORT_SYMBOL(raw_copy_from_user);
 EXPORT_SYMBOL(raw_copy_to_user);
 EXPORT_SYMBOL(iounmap);
-EXPORT_SYMBOL(__strnlen_user);
 EXPORT_SYMBOL(__vmgetie);
 EXPORT_SYMBOL(__vmsetie);
 EXPORT_SYMBOL(__vmyield);
arch/hexagon/mm/Makefile +1 -1
index 893838499591..49911a906fd0 100644
@@ -4,4 +4,4 @@
 #
 
 obj-y := init.o ioremap.o uaccess.o vm_fault.o cache.o
-obj-y += copy_to_user.o copy_from_user.o strnlen_user.o vm_tlb.o
+obj-y += copy_to_user.o copy_from_user.o vm_tlb.o
arch/hexagon/mm/strnlen_user.S deleted +0 -126

126 lines not on this page · raw message

Signature · 1 line
2.29.2
Reviewed-by on v3 3/9
#6 lore raw

Re: [PATCH v3 3/9] hexagon: use generic strncpy/strnlen from_user

Quoted 8 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the hexagon implementation of strncpy/strnlen and instead use the generic version. The hexagon version reads the data twice for strncpy() by doing an extra strnlen(), and it apparently lacks a check for user_addr_max().

Looks good,

#7 lore raw

PATCH v3 4/9 arc: use generic strncpy/strnlen from_user

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the arc implemenation of strncpy/strnlen and instead use the generic versions. The arc version is fairly slow because it always does byte accesses even for aligned data, and its checks for user_addr_max() differ from the generic code.

---
 arch/arc/Kconfig               |  2 +
 arch/arc/include/asm/uaccess.h | 83 ++--------------------------------
 arch/arc/mm/extable.c          | 12 -----
 3 files changed, 7 insertions(+), 90 deletions(-)
arch/arc/Kconfig +2 -0
index d8f51eb8963b..64e5f9366401 100644
@@ -27,6 +27,8 @@ config ARC
 	select GENERIC_PENDING_IRQ if SMP
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE if ARC_MMU_V4
arch/arc/include/asm/uaccess.h +5 -78

101 lines not on this page · raw message

arch/arc/mm/extable.c +0 -6
index b06b09ddf924..4e14c4244ea2 100644
@@ -32,16 +32,4 @@ unsigned long arc_clear_user_noinline(void __user *to,
 }
 EXPORT_SYMBOL(arc_clear_user_noinline);
 
-long arc_strncpy_from_user_noinline(char *dst, const char __user *src,
-		long count)
-{
-	return __arc_strncpy_from_user(dst, src, count);
-}
-EXPORT_SYMBOL(arc_strncpy_from_user_noinline);
Reviewed-by on v3 4/9
#8 lore raw

Re: [PATCH v3 4/9] arc: use generic strncpy/strnlen from_user

Quoted 8 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the arc implemenation of strncpy/strnlen and instead use the generic versions. The arc version is fairly slow because it always does byte accesses even for aligned data, and its checks for user_addr_max() differ from the generic code.

Looks good,

Acked-by on v3 4/9
#9 lore raw

Re: [PATCH v3 4/9] arc: use generic strncpy/strnlen from_user

In reply to Arnd Bergmann
Quoted 8 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the arc implemenation of strncpy/strnlen and instead use the generic versions. The arc version is fairly slow because it always does byte accesses even for aligned data, and its checks for user_addr_max() differ from the generic code.

LGTM. Thx for doing this Arnd !

-Vineet

Quoted 146 lines
---
  arch/arc/Kconfig               |  2 +
  arch/arc/include/asm/uaccess.h | 83 ++--------------------------------
  arch/arc/mm/extable.c          | 12 -----
  3 files changed, 7 insertions(+), 90 deletions(-)
arch/arc/Kconfig +2 -0
index d8f51eb8963b..64e5f9366401 100644
@@ -27,6 +27,8 @@ config ARC
  	select GENERIC_PENDING_IRQ if SMP
  	select GENERIC_SCHED_CLOCK
  	select GENERIC_SMP_IDLE_THREAD
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
  	select HAVE_ARCH_KGDB
  	select HAVE_ARCH_TRACEHOOK
  	select HAVE_ARCH_TRANSPARENT_HUGEPAGE if ARC_MMU_V4
arch/arc/include/asm/uaccess.h +5 -78
index 3476348f361e..754a23f26736 100644
@@ -655,96 +655,23 @@ static inline unsigned long __arc_clear_user(void __user *to, unsigned long n)
  	return res;
  }
 
-static inline long
-__arc_strncpy_from_user(char *dst, const char __user *src, long count)
-{
-	long res = 0;
-	char val;
-
-	if (!access_ok(src, 1))
-		return -EFAULT;
-
-	if (count == 0)
-		return 0;
-
-	__asm__ __volatile__(
-	"	mov	lp_count, %5		\n"
-	"	lp	3f			\n"
-	"1:	ldb.ab  %3, [%2, 1]		\n"
-	"	breq.d	%3, 0, 3f               \n"
-	"	stb.ab  %3, [%1, 1]		\n"
-	"	add	%0, %0, 1	# Num of NON NULL bytes copied	\n"
-	"3:								\n"
-	"	.section .fixup, \"ax\"		\n"
-	"	.align 4			\n"
-	"4:	mov %0, %4		# sets @res as -EFAULT	\n"
-	"	j   3b				\n"
-	"	.previous			\n"
-	"	.section __ex_table, \"a\"	\n"
-	"	.align 4			\n"
-	"	.word   1b, 4b			\n"
-	"	.previous			\n"
-	: "+r"(res), "+r"(dst), "+r"(src), "=r"(val)
-	: "g"(-EFAULT), "r"(count)
-	: "lp_count", "memory");
-
-	return res;
-}
-
-static inline long __arc_strnlen_user(const char __user *s, long n)
-{
-	long res, tmp1, cnt;
-	char val;
-
-	if (!access_ok(s, 1))
-		return 0;
-
-	__asm__ __volatile__(
-	"	mov %2, %1			\n"
-	"1:	ldb.ab  %3, [%0, 1]		\n"
-	"	breq.d  %3, 0, 2f		\n"
-	"	sub.f   %2, %2, 1		\n"
-	"	bnz 1b				\n"
-	"	sub %2, %2, 1			\n"
-	"2:	sub %0, %1, %2			\n"
-	"3:	;nop				\n"
-	"	.section .fixup, \"ax\"		\n"
-	"	.align 4			\n"
-	"4:	mov %0, 0			\n"
-	"	j   3b				\n"
-	"	.previous			\n"
-	"	.section __ex_table, \"a\"	\n"
-	"	.align 4			\n"
-	"	.word 1b, 4b			\n"
-	"	.previous			\n"
-	: "=r"(res), "=r"(tmp1), "=r"(cnt), "=r"(val)
-	: "0"(s), "1"(n)
-	: "memory");
-
-	return res;
-}
-
  #ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
 
  #define INLINE_COPY_TO_USER
  #define INLINE_COPY_FROM_USER
 
  #define __clear_user(d, n)		__arc_clear_user(d, n)
-#define strncpy_from_user(d, s, n)	__arc_strncpy_from_user(d, s, n)
-#define strnlen_user(s, n)		__arc_strnlen_user(s, n)
  #else
  extern unsigned long arc_clear_user_noinline(void __user *to,
  		unsigned long n);
-extern long arc_strncpy_from_user_noinline (char *dst, const char __user *src,
-		long count);
-extern long arc_strnlen_user_noinline(const char __user *src, long n);
-
  #define __clear_user(d, n)		arc_clear_user_noinline(d, n)
-#define strncpy_from_user(d, s, n)	arc_strncpy_from_user_noinline(d, s, n)
-#define strnlen_user(s, n)		arc_strnlen_user_noinline(s, n)
-
  #endif
 
+extern long strncpy_from_user(char *dst, const char __user *src, long count);
+#define strncpy_from_user(d, s, n)	strncpy_from_user(d, s, n)
+extern long strnlen_user(const char __user *src, long n);
+#define strnlen_user(s, n)		strnlen_user(s, n)
+
  #include <asm/segment.h>
  #include <asm-generic/uaccess.h>
 
arch/arc/mm/extable.c +0 -12
index b06b09ddf924..4e14c4244ea2 100644
@@ -32,16 +32,4 @@ unsigned long arc_clear_user_noinline(void __user *to,
  }
  EXPORT_SYMBOL(arc_clear_user_noinline);
 
-long arc_strncpy_from_user_noinline(char *dst, const char __user *src,
-		long count)
-{
-	return __arc_strncpy_from_user(dst, src, count);
-}
-EXPORT_SYMBOL(arc_strncpy_from_user_noinline);
-
-long arc_strnlen_user_noinline(const char __user *src, long n)
-{
-	return __arc_strnlen_user(src, n);
-}
-EXPORT_SYMBOL(arc_strnlen_user_noinline);
  #endif
#10 lore raw

PATCH v3 5/9 csky: use generic strncpy/strnlen from_user

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the csky implemenation of strncpy/strnlen and instead use the generic versions. The csky version is fairly slow because it always does byte accesses even for aligned data, and it lacks a checks for user_addr_max().

---
 arch/csky/Kconfig               |   2 +
 arch/csky/include/asm/uaccess.h |   6 --
 arch/csky/lib/usercopy.c        | 108 --------------------------------
 3 files changed, 2 insertions(+), 114 deletions(-)
arch/csky/Kconfig +2 -0
index 2716f6395ba7..5043e221ced4 100644
@@ -35,6 +35,8 @@ config CSKY
 	select GENERIC_IRQ_MULTI_HANDLER
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_VDSO_32
 	select GENERIC_GETTIMEOFDAY
arch/csky/include/asm/uaccess.h +0 -6
index e17c02a6709f..c40f06ee8d3e 100644
@@ -209,12 +209,6 @@ unsigned long raw_copy_to_user(void *to, const void *from, unsigned long n);
 unsigned long __clear_user(void __user *to, unsigned long n);
 #define __clear_user __clear_user
 
-long strncpy_from_user(char *dst, const char *src, long count);
-#define strncpy_from_user strncpy_from_user
-
-long strnlen_user(const char *s, long n);
-#define strnlen_user strnlen_user
-
 #include <asm/segment.h>
 #include <asm-generic/uaccess.h>
 
arch/csky/lib/usercopy.c +0 -108

114 lines not on this page · raw message

Signature · 1 line
2.29.2
Reviewed-by on v3 5/9
#11 lore raw

Re: [PATCH v3 5/9] csky: use generic strncpy/strnlen from_user

Quoted 6 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the csky implemenation of strncpy/strnlen and instead use the generic versions. The csky version is fairly slow because it always does byte accesses even for aligned data, and it lacks a checks for user_addr_max().

Looks good,

#12 lore raw

PATCH v3 6/9 microblaze: use generic strncpy/strnlen from_user

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the microblaze implemenation of strncpy/strnlen and instead use the generic versions. The microblaze version is fairly slow because it always does byte accesses even for aligned data, and it lacks a checks for user_addr_max().

---
 arch/microblaze/Kconfig                   |  2 +
 arch/microblaze/include/asm/uaccess.h     | 19 +----
 arch/microblaze/kernel/microblaze_ksyms.c |  3 -
 arch/microblaze/lib/uaccess_old.S         | 90 -----------------------
 4 files changed, 4 insertions(+), 110 deletions(-)
arch/microblaze/Kconfig +2 -0
index 14a67a42fcae..10dfa7b4feff 100644
@@ -21,6 +21,8 @@ config MICROBLAZE
 	select GENERIC_IRQ_SHOW
 	select GENERIC_PCI_IOMAP
 	select GENERIC_SCHED_CLOCK
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_HASH
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_SECCOMP
arch/microblaze/include/asm/uaccess.h +2 -17
index c44b59470e45..bbe39fe00461 100644
@@ -296,28 +296,13 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n)
 /*
  * Copy a null terminated string from userspace.
  */
-extern int __strncpy_user(char *to, const char __user *from, int len);
-
-static inline long
-strncpy_from_user(char *dst, const char __user *src, long count)
-{
-	if (!access_ok(src, 1))
-		return -EFAULT;
-	return __strncpy_user(dst, src, count);
-}
+extern long strncpy_from_user(char *dst, const char __user *src, long count);
 
 /*
  * Return the size of a string (including the ending 0)
  *
  * Return 0 on exception, a value greater than N if too long
  */
-extern int __strnlen_user(const char __user *sstr, int len);
-
-static inline long strnlen_user(const char __user *src, long n)
-{
-	if (!access_ok(src, 1))
-		return 0;
-	return __strnlen_user(src, n);
-}
+extern long strnlen_user(const char __user *sstr, int len);
 
 #endif /* _ASM_MICROBLAZE_UACCESS_H */
arch/microblaze/kernel/microblaze_ksyms.c +0 -3
index 303aaf13573b..14e0f2100c41 100644
@@ -25,9 +25,6 @@ EXPORT_SYMBOL(_mcount);
 /*
  * Assembly functions that may be used (directly or indirectly) by modules
  */
-EXPORT_SYMBOL(__copy_tofrom_user);
-EXPORT_SYMBOL(__strncpy_user);
-
 #ifdef CONFIG_OPT_LIB_ASM
 EXPORT_SYMBOL(memcpy);
 EXPORT_SYMBOL(memmove);
arch/microblaze/lib/uaccess_old.S +0 -90

96 lines not on this page · raw message

Signature · 1 line
2.29.2
Reviewed-by on v3 6/9
#13 lore raw

Re: [PATCH v3 6/9] microblaze: use generic strncpy/strnlen from_user

Quoted 6 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Remove the microblaze implemenation of strncpy/strnlen and instead use the generic versions. The microblaze version is fairly slow because it always does byte accesses even for aligned data, and it lacks a checks for user_addr_max().

Looks good,

#14 lore raw

Re: [PATCH v3 6/9] microblaze: use generic strncpy/strnlen from_user

In reply to Arnd Bergmann
I love your patch! Yet something to improve:

Hi Arnd,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.14-rc2 next-20210722]
[cannot apply to asm-generic/master arc/for-next uclinux-h8/h8300-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/asm-generic-uaccess-h-remove-__strncpy_from_user-__strnlen_user/20210722-214839
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3d5895cd351757f69c9a66fb5fc8cf19f454d773
config: microblaze-buildonly-randconfig-r005-20210722 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 10.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/50efd0b2c40f098c1c02453d22bc3f3ef58d8a76
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arnd-Bergmann/asm-generic-uaccess-h-remove-__strncpy_from_user-__strnlen_user/20210722-214839 git checkout 50efd0b2c40f098c1c02453d22bc3f3ef58d8a76
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=microblaze SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

All errors (new ones prefixed by >>):

lib/strncpy_from_user.c: In function 'strncpy_from_user':

>> lib/strncpy_from_user.c:123:13: error: implicit declaration of function 'user_addr_max' [-Werror=implicit-function-declaration]
     123 |  max_addr = user_addr_max();
         |             ^~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
>> lib/strnlen_user.c:92:6: error: conflicting types for 'strnlen_user'
      92 | long strnlen_user(const char __user *str, long count)
         |      ^~~~~~~~~~~~
   In file included from include/linux/uaccess.h:11,
                    from lib/strnlen_user.c:4:
   arch/microblaze/include/asm/uaccess.h:306:13: note: previous declaration of 'strnlen_user' was here
     306 | extern long strnlen_user(const char __user *sstr, int len);
         |             ^~~~~~~~~~~~
   lib/strnlen_user.c: In function 'strnlen_user':
>> lib/strnlen_user.c:99:13: error: implicit declaration of function 'user_addr_max' [-Werror=implicit-function-declaration]
      99 |  max_addr = user_addr_max();
         |             ^~~~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/strnlen_user +92 lib/strnlen_user.c

a08c5356a3aaf6 Linus Torvalds    2012-05-26  70  
a08c5356a3aaf6 Linus Torvalds    2012-05-26  71  /**
a08c5356a3aaf6 Linus Torvalds    2012-05-26  72   * strnlen_user: - Get the size of a user string INCLUDING final NUL.
a08c5356a3aaf6 Linus Torvalds    2012-05-26  73   * @str: The string to measure.
a08c5356a3aaf6 Linus Torvalds    2012-05-26  74   * @count: Maximum count (including NUL character)
a08c5356a3aaf6 Linus Torvalds    2012-05-26  75   *
b3c395ef5556a6 David Hildenbrand 2015-05-11  76   * Context: User context only. This function may sleep if pagefaults are
b3c395ef5556a6 David Hildenbrand 2015-05-11  77   *          enabled.
a08c5356a3aaf6 Linus Torvalds    2012-05-26  78   *
a08c5356a3aaf6 Linus Torvalds    2012-05-26  79   * Get the size of a NUL-terminated string in user space.
a08c5356a3aaf6 Linus Torvalds    2012-05-26  80   *
a08c5356a3aaf6 Linus Torvalds    2012-05-26  81   * Returns the size of the string INCLUDING the terminating NUL.
226a07ef0a5a2d Jan Kara          2015-06-03  82   * If the string is too long, returns a number larger than @count. User
226a07ef0a5a2d Jan Kara          2015-06-03  83   * has to check the return value against "> count".
a08c5356a3aaf6 Linus Torvalds    2012-05-26  84   * On exception (or invalid count), returns 0.
226a07ef0a5a2d Jan Kara          2015-06-03  85   *
226a07ef0a5a2d Jan Kara          2015-06-03  86   * NOTE! You should basically never use this function. There is
226a07ef0a5a2d Jan Kara          2015-06-03  87   * almost never any valid case for using the length of a user space
226a07ef0a5a2d Jan Kara          2015-06-03  88   * string, since the string can be changed at any time by other
226a07ef0a5a2d Jan Kara          2015-06-03  89   * threads. Use "strncpy_from_user()" instead to get a stable copy
226a07ef0a5a2d Jan Kara          2015-06-03  90   * of the string.
a08c5356a3aaf6 Linus Torvalds    2012-05-26  91   */
a08c5356a3aaf6 Linus Torvalds    2012-05-26 @92  long strnlen_user(const char __user *str, long count)
a08c5356a3aaf6 Linus Torvalds    2012-05-26  93  {
a08c5356a3aaf6 Linus Torvalds    2012-05-26  94  	unsigned long max_addr, src_addr;
a08c5356a3aaf6 Linus Torvalds    2012-05-26  95  
a08c5356a3aaf6 Linus Torvalds    2012-05-26  96  	if (unlikely(count <= 0))
a08c5356a3aaf6 Linus Torvalds    2012-05-26  97  		return 0;
a08c5356a3aaf6 Linus Torvalds    2012-05-26  98  
a08c5356a3aaf6 Linus Torvalds    2012-05-26 @99  	max_addr = user_addr_max();

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

#15 lore raw

Re: [PATCH v3 6/9] microblaze: use generic strncpy/strnlen from_user

Quoted 9 lines · kernel test robot

All errors (new ones prefixed by >>):

lib/strncpy_from_user.c: In function 'strncpy_from_user':

Quoted 1 line
Quoted 1 line

lib/strncpy_from_user.c:123:13: error: implicit declaration of function 'user_addr_max' [-Werror=implicit-function-declaration]

     123 |  max_addr = user_addr_max();
         |             ^~~~~~~~~~~~~

cc1: some warnings being treated as errors

Quoted 8 lines

--

Quoted 1 line
Quoted 1 line

lib/strnlen_user.c:92:6: error: conflicting types for 'strnlen_user'

92 | long strnlen_user(const char __user *str, long count)
| ^~~~~~~~~~~~
In file included from include/linux/uaccess.h:11,
from lib/strnlen_user.c:4:
arch/microblaze/include/asm/uaccess.h:306:13: note: previous declaration of 'strnlen_user' was here

     306 | extern long strnlen_user(const char __user *sstr, int len);

Fixed both now, thanks for the report.

I had to add a user_addr_max() definition here, which unfortunately will also conflict
with the get_fs() removal if Christoph plans to have that merged through the microblaze
tree.

If this becomes a problem, I could merge both series through the asm-generic tree.

Arnd

#16 lore raw

Re: [PATCH v3 6/9] microblaze: use generic strncpy/strnlen from_user

Quoted 5 lines · Arnd Bergmann

I had to add a user_addr_max() definition here, which unfortunately will also conflict
with the get_fs() removal if Christoph plans to have that merged through the microblaze
tree.

I don't have any patches to remove set_fs for microblaze so far.

#17 lore raw

PATCH v3 7/9 asm-generic: uaccess: remove inline strncpy_from_user/strnlen_user

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The inline version is used on three NOMMU architectures and is particularly inefficient when it scans the string one byte at a time twice. It also lacks a check for user_addr_max(), but this is probably ok on NOMMU targets.

Consolidate the asm-generic implementation with the library version that is used everywhere else. This version is generalized enough to work efficiently on both MMU and NOMMU targets, and using the same code everywhere reduces the potential for subtle bugs.

---
 arch/h8300/Kconfig            |  2 ++
 arch/m68k/Kconfig             |  4 +--
 arch/riscv/Kconfig            |  4 +--
 include/asm-generic/uaccess.h | 46 ++++++-----------------------------
 4 files changed, 14 insertions(+), 42 deletions(-)
arch/h8300/Kconfig +2 -0
index 3e3e0f16f7e0..53dfd2d47e0e 100644
@@ -11,6 +11,8 @@ config H8300
 	select GENERIC_IRQ_SHOW
 	select FRAME_POINTER
 	select GENERIC_CPU_DEVICES
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 	select MODULES_USE_ELF_RELA
 	select COMMON_CLK
 	select ARCH_WANT_FRAME_POINTERS
arch/m68k/Kconfig +2 -2
index 96989ad46f66..37a65bed6dfa 100644
@@ -16,8 +16,8 @@ config M68K
 	select GENERIC_CPU_DEVICES
 	select GENERIC_IOMAP
 	select GENERIC_IRQ_SHOW
-	select GENERIC_STRNCPY_FROM_USER if MMU
-	select GENERIC_STRNLEN_USER if MMU
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 	select HAVE_AOUT if MMU
 	select HAVE_ASM_MODVERSIONS
 	select HAVE_DEBUG_BUGVERBOSE
arch/riscv/Kconfig +2 -2
index 8fcceb8eda07..47bbbcab91b2 100644
@@ -56,8 +56,8 @@ config RISCV
 	select GENERIC_PTDUMP if MMU
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER if MMU
-	select GENERIC_STRNLEN_USER if MMU
+	select GENERIC_STRNCPY_FROM_USER
+	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL if MMU && 64BIT
 	select HANDLE_DOMAIN_IRQ
 	select HAVE_ARCH_AUDITSYSCALL
include/asm-generic/uaccess.h +8 -38

62 lines not on this page · raw message

Signature · 1 line
2.29.2
Reviewed-by on v3 7/9
#18 lore raw

Re: [PATCH v3 7/9] asm-generic: uaccess: remove inline strncpy_from_user/strnlen_user

Quoted 7 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The inline version is used on three NOMMU architectures and is particularly inefficient when it scans the string one byte at a time twice. It also lacks a check for user_addr_max(), but this is probably ok on NOMMU targets.

Quoted 8 lines

Consolidate the asm-generic implementation with the library version that is used everywhere else. This version is generalized enough to work efficiently on both MMU and NOMMU targets, and using the same code everywhere reduces the potential for subtle bugs.

Looks good,

#19 lore raw

PATCH v3 8/9 asm-generic: remove extra strn{cpy_from,len}_user declarations

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

As these are now in asm-generic, it's no longer necessary to declare them in the architecture.

---
 arch/arc/include/asm/uaccess.h     | 5 -----
 arch/hexagon/include/asm/uaccess.h | 6 ------
 arch/um/include/asm/uaccess.h      | 5 +----
 3 files changed, 1 insertion(+), 15 deletions(-)
arch/arc/include/asm/uaccess.h +0 -5
index 754a23f26736..783bfdb3bfa3 100644
@@ -667,11 +667,6 @@ extern unsigned long arc_clear_user_noinline(void __user *to,
 #define __clear_user(d, n)		arc_clear_user_noinline(d, n)
 #endif
 
-extern long strncpy_from_user(char *dst, const char __user *src, long count);
-#define strncpy_from_user(d, s, n)	strncpy_from_user(d, s, n)
-extern long strnlen_user(const char __user *src, long n);
-#define strnlen_user(s, n)		strnlen_user(s, n)
-
 #include <asm/segment.h>
 #include <asm-generic/uaccess.h>
 
arch/hexagon/include/asm/uaccess.h +0 -6
index d950df12d8c5..ef5bfef8d490 100644
@@ -57,12 +57,6 @@ unsigned long raw_copy_to_user(void __user *to, const void *from,
 __kernel_size_t __clear_user_hexagon(void __user *dest, unsigned long count);
 #define __clear_user(a, s) __clear_user_hexagon((a), (s))
 
-extern long strnlen_user(const char __user *src, long n);
-#define strnlen_user strnlen_user
-
-extern long strncpy_from_user(char *dst, const char __user *src, long n)
-#define strncpy_from_user strncpy_from_user
-
 #include <asm-generic/uaccess.h>
 
 
arch/um/include/asm/uaccess.h +1 -4
index 3bf209f683f8..191ef36dd543 100644
@@ -23,16 +23,13 @@
 
 extern unsigned long raw_copy_from_user(void *to, const void __user *from, unsigned long n);
 extern unsigned long raw_copy_to_user(void __user *to, const void *from, unsigned long n);
-extern long strncpy_from_user(char *dst, const char __user *src, long count);
-extern long strnlen_user(const void __user *str, long len);
 extern unsigned long __clear_user(void __user *mem, unsigned long len);
 static inline int __access_ok(unsigned long addr, unsigned long size);
 
 /* Teach asm-generic/uaccess.h that we have C functions for these. */
 #define __access_ok __access_ok
 #define __clear_user __clear_user
-#define strnlen_user strnlen_user
-#define strncpy_from_user strncpy_from_user
+
 #define INLINE_COPY_FROM_USER
 #define INLINE_COPY_TO_USER
 
Signature · 1 line
2.29.2
#21 lore raw

PATCH v3 9/9 asm-generic: reverse GENERIC_{STRNCPY_FROM,STRNLEN}_USER symbols

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Most architectures do not need a custom implementation, and in most cases the generic implementation is preferred, so change the polariy on these Kconfig symbols to require architectures to select them when they provide their own version.

The new name is CONFIG_ARCH_HAS_{STRNCPY_FROM,STRNLEN}_USER.

The remaining architectures at the moment are: ia64, mips, parisc, s390, um and xtensa. We should probably convert these as well, but I was not sure how far to take this series.

Cc "James E.J. Bottomley" <[email protected]>
Cc Anton Ivanov <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Helge Deller <[email protected]>
Cc Jeff Dike <[email protected]>
Cc Max Filippov <[email protected]>
Cc Richard Weinberger <[email protected]>
Cc Thomas Bogendoerfer <[email protected]>
Cc Vasily Gorbik <[email protected]>
---
 arch/alpha/Kconfig                |  2 --
 arch/arc/Kconfig                  |  2 --
 arch/arm/Kconfig                  |  2 --
 arch/arm64/Kconfig                |  2 --
 arch/csky/Kconfig                 |  2 --
 arch/h8300/Kconfig                |  2 --
 arch/hexagon/Kconfig              |  2 --
 arch/ia64/Kconfig                 |  2 ++
 arch/m68k/Kconfig                 |  2 --
 arch/microblaze/Kconfig           |  2 --
 arch/mips/Kconfig                 |  2 ++
 arch/nds32/Kconfig                |  2 --
 arch/nios2/Kconfig                |  2 --
 arch/openrisc/Kconfig             |  2 --
 arch/parisc/Kconfig               |  2 +-
 arch/powerpc/Kconfig              |  2 --
 arch/riscv/Kconfig                |  2 --
 arch/s390/Kconfig                 |  2 ++
 arch/sh/Kconfig                   |  2 --
 arch/sparc/Kconfig                |  2 --
 arch/um/Kconfig                   |  2 ++
 arch/x86/Kconfig                  |  2 --
 arch/xtensa/Kconfig               |  3 ++-
 arch/xtensa/include/asm/uaccess.h |  3 +--
 lib/Kconfig                       | 10 ++++++++--
 25 files changed, 20 insertions(+), 40 deletions(-)
arch/alpha/Kconfig +0 -2
index 77d3280dc678..62935da8e69e 100644
@@ -29,8 +29,6 @@ config ALPHA
 	select AUDIT_ARCH
 	select GENERIC_CPU_VULNERABILITIES
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_MOD_ARCH_SPECIFIC
 	select MODULES_USE_ELF_RELA
arch/arc/Kconfig +0 -2
index 64e5f9366401..d8f51eb8963b 100644
@@ -27,8 +27,6 @@ config ARC
 	select GENERIC_PENDING_IRQ if SMP
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE if ARC_MMU_V4
arch/arm/Kconfig +0 -2
index 82f908fa5676..e8ebbb9ededf 100644
@@ -63,8 +63,6 @@ config ARM
 	select GENERIC_PCI_IOMAP
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select HANDLE_DOMAIN_IRQ
 	select HARDIRQS_SW_RESEND
 	select HAVE_ARCH_AUDITSYSCALL if AEABI && !OABI_COMPAT
arch/arm64/Kconfig +0 -2
index b5b13a932561..e0c6c14a21d0 100644
@@ -130,8 +130,6 @@ config ARM64
 	select GENERIC_PTDUMP
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_GETTIMEOFDAY
 	select GENERIC_VDSO_TIME_NS
arch/csky/Kconfig +0 -2
index 5043e221ced4..2716f6395ba7 100644
@@ -35,8 +35,6 @@ config CSKY
 	select GENERIC_IRQ_MULTI_HANDLER
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_VDSO_32
 	select GENERIC_GETTIMEOFDAY
arch/h8300/Kconfig +0 -2
index 53dfd2d47e0e..3e3e0f16f7e0 100644
@@ -11,8 +11,6 @@ config H8300
 	select GENERIC_IRQ_SHOW
 	select FRAME_POINTER
 	select GENERIC_CPU_DEVICES
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select MODULES_USE_ELF_RELA
 	select COMMON_CLK
 	select ARCH_WANT_FRAME_POINTERS
arch/hexagon/Kconfig +0 -2
index 3bf4845fed4b..e5a852080730 100644
@@ -19,8 +19,6 @@ config HEXAGON
 	# GENERIC_ALLOCATOR is used by dma_alloc_coherent()
 	select GENERIC_ALLOCATOR
 	select GENERIC_IRQ_SHOW
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_TRACEHOOK
 	select NEED_SG_DMA_LENGTH
arch/ia64/Kconfig +2 -0
index cf425c2c63af..7aa5b94464c6 100644
@@ -9,6 +9,8 @@ menu "Processor type and features"
 config IA64
 	bool
 	select ARCH_HAS_DMA_MARK_CLEAN
+	select ARCH_HAS_STRNCPY_FROM_USER
+	select ARCH_HAS_STRNLEN_USER
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
 	select ACPI
arch/m68k/Kconfig +0 -2
index 37a65bed6dfa..7970d316dc9b 100644
@@ -16,8 +16,6 @@ config M68K
 	select GENERIC_CPU_DEVICES
 	select GENERIC_IOMAP
 	select GENERIC_IRQ_SHOW
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select HAVE_AOUT if MMU
 	select HAVE_ASM_MODVERSIONS
 	select HAVE_DEBUG_BUGVERBOSE
arch/microblaze/Kconfig +0 -2
index 10dfa7b4feff..14a67a42fcae 100644
@@ -21,8 +21,6 @@ config MICROBLAZE
 	select GENERIC_IRQ_SHOW
 	select GENERIC_PCI_IOMAP
 	select GENERIC_SCHED_CLOCK
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_HASH
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_SECCOMP
arch/mips/Kconfig +2 -0
index cee6087cd686..22aa4d75fa34 100644
@@ -12,6 +12,8 @@ config MIPS
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_HAS_GCOV_PROFILE_ALL
+	select ARCH_HAS_STRNCPY_FROM_USER
+	select ARCH_HAS_STRNLEN_USER
 	select ARCH_KEEP_MEMBLOCK
 	select ARCH_SUPPORTS_UPROBES
 	select ARCH_USE_BUILTIN_BSWAP
arch/nds32/Kconfig +0 -2
index 62313902d75d..9c9f3877abf9 100644
@@ -26,8 +26,6 @@ config NDS32
 	select GENERIC_LIB_LSHRDI3
 	select GENERIC_LIB_MULDI3
 	select GENERIC_LIB_UCMPDI2
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
 	select HANDLE_DOMAIN_IRQ
 	select HAVE_ARCH_TRACEHOOK
arch/nios2/Kconfig +0 -2
index c24955c81c92..3efe5533ea1c 100644
@@ -13,8 +13,6 @@ config NIOS2
 	select GENERIC_CPU_DEVICES
 	select GENERIC_IRQ_PROBE
 	select GENERIC_IRQ_SHOW
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_KGDB
 	select IRQ_DOMAIN
arch/openrisc/Kconfig +0 -2
index 591acc5990dc..50035a9816c8 100644
@@ -25,8 +25,6 @@ config OPENRISC
 	select HAVE_UID16
 	select GENERIC_ATOMIC64
 	select GENERIC_CLOCKEVENTS_BROADCAST
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_SMP_IDLE_THREAD
 	select MODULES_USE_ELF_RELA
 	select HAVE_DEBUG_STACKOVERFLOW
arch/parisc/Kconfig +1 -1
index bde9907bc5b2..727c823f866e 100644
@@ -11,6 +11,7 @@ config PARISC
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
+	select ARCH_HAS_STRNLEN_USER
 	select ARCH_NO_SG_CHAIN
 	select ARCH_SUPPORTS_HUGETLBFS if PA20
 	select ARCH_SUPPORTS_MEMORY_FAILURE
@@ -34,7 +35,6 @@ config PARISC
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
 	select GENERIC_SMP_IDLE_THREAD
 	select GENERIC_CPU_DEVICES
-	select GENERIC_STRNCPY_FROM_USER
 	select GENERIC_LIB_DEVMEM_IS_ALLOWED
 	select SYSCTL_ARCH_UNALIGN_ALLOW
 	select SYSCTL_EXCEPTION_TRACE
arch/powerpc/Kconfig +0 -2
index d01e3401581d..d2df3d2db3eb 100644
@@ -183,8 +183,6 @@ config PPC
 	select GENERIC_IRQ_SHOW_LEVEL
 	select GENERIC_PCI_IOMAP		if PCI
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_VDSO_TIME_NS
 	select HAVE_ARCH_AUDITSYSCALL
arch/riscv/Kconfig +0 -2
index 47bbbcab91b2..49d642df6ab9 100644
@@ -56,8 +56,6 @@ config RISCV
 	select GENERIC_PTDUMP if MMU
 	select GENERIC_SCHED_CLOCK
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL if MMU && 64BIT
 	select HANDLE_DOMAIN_IRQ
 	select HAVE_ARCH_AUDITSYSCALL
arch/s390/Kconfig +2 -0
index a0e2130f0100..f4f39087cad0 100644
@@ -75,6 +75,8 @@ config S390
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_STRICT_MODULE_RWX
+	select ARCH_HAS_STRNCPY_FROM_USER
+	select ARCH_HAS_STRNLEN_USER
 	select ARCH_HAS_SYSCALL_WRAPPER
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_HAS_VDSO_DATA
arch/sh/Kconfig +0 -2
index 45a0549421cd..5d20509e5556 100644
@@ -22,8 +22,6 @@ config SUPERH
 	select GENERIC_IRQ_SHOW
 	select GENERIC_PCI_IOMAP if PCI
 	select GENERIC_SCHED_CLOCK
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_SMP_IDLE_THREAD
 	select GUP_GET_PTE_LOW_HIGH if X2TLB
 	select HAVE_ARCH_AUDITSYSCALL
arch/sparc/Kconfig +0 -2
index c5fa7932b550..9f78822562eb 100644
@@ -38,8 +38,6 @@ config SPARC
 	select HAVE_EBPF_JIT if SPARC64
 	select HAVE_DEBUG_BUGVERBOSE
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select MODULES_USE_ELF_RELA
 	select PCI_SYSCALL if PCI
 	select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
arch/um/Kconfig +2 -0
index 0561b73cfd9a..77e66d3719f6 100644
@@ -7,6 +7,8 @@ config UML
 	default y
 	select ARCH_EPHEMERAL_INODES
 	select ARCH_HAS_KCOV
+	select ARCH_HAS_STRNCPY_FROM_USER
+	select ARCH_HAS_STRNLEN_USER
 	select ARCH_NO_PREEMPT
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_SECCOMP_FILTER
arch/x86/Kconfig +0 -2
index 49270655e827..5509c828bc93 100644
@@ -144,8 +144,6 @@ config X86
 	select GENERIC_PENDING_IRQ		if SMP
 	select GENERIC_PTDUMP
 	select GENERIC_SMP_IDLE_THREAD
-	select GENERIC_STRNCPY_FROM_USER
-	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_GETTIMEOFDAY
 	select GENERIC_VDSO_TIME_NS
arch/xtensa/Kconfig +2 -1
index 2332b2156993..282fc195680e 100644
@@ -7,6 +7,8 @@ config XTENSA
 	select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
 	select ARCH_HAS_DMA_SET_UNCACHED if MMU
+	select ARCH_HAS_STRNCPY_FROM_USER if !KASAN
+	select ARCH_HAS_STRNLEN_USER
 	select ARCH_USE_MEMTEST
 	select ARCH_USE_QUEUED_RWLOCKS
 	select ARCH_USE_QUEUED_SPINLOCKS
@@ -20,7 +22,6 @@ config XTENSA
 	select GENERIC_IRQ_SHOW
 	select GENERIC_PCI_IOMAP
 	select GENERIC_SCHED_CLOCK
-	select GENERIC_STRNCPY_FROM_USER if KASAN
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
 	select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL
arch/xtensa/include/asm/uaccess.h +1 -2
index 5c9fb8005aa8..250ee68e9d82 100644
@@ -290,8 +290,7 @@ clear_user(void __user *addr, unsigned long size)
 #define __clear_user  __xtensa_clear_user
 
 
-#ifndef CONFIG_GENERIC_STRNCPY_FROM_USER
-
+#ifdef CONFIG_HAS_STRNCPY_FROM_USER
 extern long __strncpy_user(char *dst, const char __user *src, long count);
 
 static inline long
lib/Kconfig +8 -2
index d241fe476fda..0e66c9fa636b 100644
@@ -50,12 +50,18 @@ config HAVE_ARCH_BITREVERSE
 	  This option enables the use of hardware bit-reversal instructions on
 	  architectures which support such operations.
 
-config GENERIC_STRNCPY_FROM_USER
+config ARCH_HAS_STRNCPY_FROM_USER
 	bool
 
-config GENERIC_STRNLEN_USER
+config ARCH_HAS_STRNLEN_USER
 	bool
 
+config GENERIC_STRNCPY_FROM_USER
+	def_bool !ARCH_HAS_STRNCPY_FROM_USER
+
+config GENERIC_STRNLEN_USER
+	def_bool !ARCH_HAS_STRNLEN_USER
+
 config GENERIC_NET_UTILS
 	bool
 
Signature · 1 line
2.29.2
Reviewed-by on v3 9/9
#22 lore raw

Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM,STRNLEN}_USER symbols

Quoted 7 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Most architectures do not need a custom implementation, and in most cases the generic implementation is preferred, so change the polariy on these Kconfig symbols to require architectures to select them when they provide their own version.

Quoted 5 lines

The new name is CONFIG_ARCH_HAS_{STRNCPY_FROM,STRNLEN}_USER.

The remaining architectures at the moment are: ia64, mips, parisc, s390, um and xtensa. We should probably convert these as well, but I was not sure how far to take this series.

Looks good,

maintainer
#23 lore raw

Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM, STRNLEN}_USER symbols

In reply to Arnd Bergmann
Quoted 3 lines

The remaining architectures at the moment are: ia64, mips, parisc, s390, um and xtensa. We should probably convert these as well, but

I'm not sure it makes sense to convert um, the implementation uses strncpy(), and that should use the libc implementation, which is tuned for the actual hardware that the binary is running on, so performance wise that might be better.

OTOH, maybe this is all in the noise given the huge syscall overhead in um, so maybe unifying it would make more sense.

johannes

#24 lore raw

Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM, STRNLEN}_USER symbols

Quoted 5 lines · Johannes Berg
Quoted 4 lines

The remaining architectures at the moment are: ia64, mips, parisc, s390, um and xtensa. We should probably convert these as well, but

Quoted 7 lines

I'm not sure it makes sense to convert um, the implementation uses strncpy(), and that should use the libc implementation, which is tuned for the actual hardware that the binary is running on, so performance wise that might be better.

OTOH, maybe this is all in the noise given the huge syscall overhead in um, so maybe unifying it would make more sense.

Right, makes sense. I suppose if we end up converting mips and s390, we should just do arch/um and the others as well for consistency, even if that adds some overhead.

Arnd

maintainer
#25 lore raw

Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM, STRNLEN}_USER symbols

Quoted 11 lines · Arnd Bergmann
Quoted 10 lines · Johannes Berg
Quoted 4 lines

The remaining architectures at the moment are: ia64, mips, parisc, s390, um and xtensa. We should probably convert these as well, but

I'm not sure it makes sense to convert um, the implementation uses strncpy(), and that should use the libc implementation, which is tuned for the actual hardware that the binary is running on, so performance wise that might be better.

Quoted 6 lines
Quoted 2 lines

OTOH, maybe this is all in the noise given the huge syscall overhead in um, so maybe unifying it would make more sense.

Right, makes sense. I suppose if we end up converting mips and s390, we should just do arch/um and the others as well for consistency, even if that adds some overhead.

Feel free to add the s390 patch below on top of your series. However one question: the strncpy_from_user() prototype now comes everywhere without the __must_check attribute. Is there any reason for that?

At least for s390 I want to keep that.

From 03ad679909af58e1dc6864f47ce67210f0534c39 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <[email protected]>
Date: Thu, 22 Jul 2021 21:48:18 +0200
Subject: [PATCH] s390: use generic strncpy/strnlen from_user

The s390 variant of strncpy_from_user() is slightly faster than the generic variant, however convert to the generic variant now to follow most if not all other architectures.

Converting to the generic variant was already considered a couple of years ago. See commit f5c8b9601036 ("s390/uaccess: use sane length for __strncpy_from_user()").

---
 arch/s390/Kconfig               |  2 --
 arch/s390/include/asm/uaccess.h | 18 ++----------
 arch/s390/lib/uaccess.c         | 52 ---------------------------------
 3 files changed, 2 insertions(+), 70 deletions(-)
arch/s390/Kconfig +0 -2
index f4f39087cad0..a0e2130f0100 100644
@@ -75,8 +75,6 @@ config S390
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_STRICT_MODULE_RWX
-	select ARCH_HAS_STRNCPY_FROM_USER
-	select ARCH_HAS_STRNLEN_USER
 	select ARCH_HAS_SYSCALL_WRAPPER
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_HAS_VDSO_DATA
arch/s390/include/asm/uaccess.h +2 -16
index 2316f2440881..9ed9aa37e836 100644
@@ -233,23 +233,9 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n);
 /*
  * Copy a null terminated string from userspace.
  */
+long __must_check strncpy_from_user(char *dst, const char __user *src, long count);
 
-long __strncpy_from_user(char *dst, const char __user *src, long count);
-
-static inline long __must_check
-strncpy_from_user(char *dst, const char __user *src, long count)
-{
-	might_fault();
-	return __strncpy_from_user(dst, src, count);
-}
-
-unsigned long __must_check __strnlen_user(const char __user *src, unsigned long count);
-
-static inline unsigned long strnlen_user(const char __user *src, unsigned long n)
-{
-	might_fault();
-	return __strnlen_user(src, n);
-}
+long __must_check strnlen_user(const char __user *src, long count);
 
 /*
  * Zero Userspace
arch/s390/lib/uaccess.c +0 -52

55 lines not on this page · raw message

Signature · 1 line
2.25.1
#26 lore raw

Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM, STRNLEN}_USER symbols

Quoted 2 lines · Heiko Carstens

Feel free to add the s390 patch below on top of your series.

Thanks a lot, added now.

Quoted 4 lines

However one question: the strncpy_from_user() prototype now comes everywhere without the __must_check attribute. Is there any reason for that?

At least for s390 I want to keep that.

Makes sense, I'll add a patch for that as well.

Arnd

maintainer Acked-by on v3 9/9
#27 lore raw

Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM,STRNLEN}_USER symbols

In reply to Arnd Bergmann
Quoted 31 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Most architectures do not need a custom implementation, and in most cases the generic implementation is preferred, so change the polariy on these Kconfig symbols to require architectures to select them when they provide their own version.

The new name is CONFIG_ARCH_HAS_{STRNCPY_FROM,STRNLEN}_USER.

The remaining architectures at the moment are: ia64, mips, parisc, s390, um and xtensa. We should probably convert these as well, but I was not sure how far to take this series.

Cc "James E.J. Bottomley" <[email protected]>
Cc Anton Ivanov <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Helge Deller <[email protected]>
Cc Jeff Dike <[email protected]>
Cc Max Filippov <[email protected]>
Cc Richard Weinberger <[email protected]>
Cc Thomas Bogendoerfer <[email protected]>
Cc Vasily Gorbik <[email protected]>
Quoted 1 line
 arch/m68k/Kconfig                 |  2 --

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
maintainer Acked-by on v3 9/9
#28 lore raw

Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM,STRNLEN}_USER symbols

In reply to Arnd Bergmann
Quoted 7 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

Most architectures do not need a custom implementation, and in most cases the generic implementation is preferred, so change the polariy on these Kconfig symbols to require architectures to select them when they provide their own version.

Quoted 5 lines

The new name is CONFIG_ARCH_HAS_{STRNCPY_FROM,STRNLEN}_USER.

The remaining architectures at the moment are: ia64, mips, parisc, s390, um and xtensa. We should probably convert these as well, but I was not sure how far to take this series.

Thanks!
Helge

maintainer
#29 lore raw
In reply to Arnd Bergmann

Hi Arnd,

Quoted 19 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

I had run into some regressions for the previous version of this series, the new version is based on v5.14-rc2 instead.

These two functions appear to be unnecessarily different between architectures, and the asm-generic version is a bit questionable, even for NOMMU architectures.

Clean this up to just use the generic library version for anything that uses the generic version today. I've expanded on the patch descriptions a little, as suggested by Christoph Hellwig, but I suspect a more detailed review would uncover additional problems with the custom versions that are getting removed.

I ended up adding patches for csky and microblaze as they had the same implementation that I removed elsewhere, these are now gone as well.

Quoted 7 lines

If I hear no objections from architecture maintainers or new build regressions, I'll queue these up in the asm-generic tree for 5.15.

Arnd

Are you planning to add this to asm-generic tree for 5.15 anytime soon. Also while there, any chance you could pick up [1] too which was Acked by Will.

Thx,
-Vineet

[1] https://lore.kernel.org/linux-arch/[email protected]/

Quoted 53 lines

Arnd Bergmann (9):
asm-generic/uaccess.h: remove __strncpy_from_user/__strnlen_user
h8300: remove stale strncpy_from_user
hexagon: use generic strncpy/strnlen from_user
arc: use generic strncpy/strnlen from_user
csky: use generic strncpy/strnlen from_user
microblaze: use generic strncpy/strnlen from_user
asm-generic: uaccess: remove inline strncpy_from_user/strnlen_user
asm-generic: remove extra strn{cpy_from,len}_user declarations
asm-generic: reverse GENERIC_{STRNCPY_FROM,STRNLEN}_USER symbols

  arch/alpha/Kconfig                        |   2 -
  arch/arc/include/asm/uaccess.h            |  72 -------------
  arch/arc/mm/extable.c                     |  12 ---
  arch/arm/Kconfig                          |   2 -
  arch/arm64/Kconfig                        |   2 -
  arch/csky/include/asm/uaccess.h           |   6 --
  arch/csky/lib/usercopy.c                  | 102 ------------------
  arch/h8300/kernel/h8300_ksyms.c           |   2 -
  arch/h8300/lib/Makefile                   |   2 +-
  arch/h8300/lib/strncpy.S                  |  35 ------
  arch/hexagon/include/asm/uaccess.h        |  31 ------
  arch/hexagon/kernel/hexagon_ksyms.c       |   1 -
  arch/hexagon/mm/Makefile                  |   2 +-
  arch/hexagon/mm/strnlen_user.S            | 126 ----------------------
  arch/ia64/Kconfig                         |   2 +
  arch/m68k/Kconfig                         |   2 -
  arch/microblaze/include/asm/uaccess.h     |  19 +---
  arch/microblaze/kernel/microblaze_ksyms.c |   3 -
  arch/microblaze/lib/uaccess_old.S         |  90 ----------------
  arch/mips/Kconfig                         |   2 +
  arch/nds32/Kconfig                        |   2 -
  arch/nios2/Kconfig                        |   2 -
  arch/openrisc/Kconfig                     |   2 -
  arch/parisc/Kconfig                       |   2 +-
  arch/powerpc/Kconfig                      |   2 -
  arch/riscv/Kconfig                        |   2 -
  arch/s390/Kconfig                         |   2 +
  arch/sh/Kconfig                           |   2 -
  arch/sparc/Kconfig                        |   2 -
  arch/um/Kconfig                           |   2 +
  arch/um/include/asm/uaccess.h             |   5 +-
  arch/um/kernel/skas/uaccess.c             |  14 ++-
  arch/x86/Kconfig                          |   2 -
  arch/xtensa/Kconfig                       |   3 +-
  arch/xtensa/include/asm/uaccess.h         |   3 +-
  include/asm-generic/uaccess.h             |  52 ++-------
  lib/Kconfig                               |  10 +-
  37 files changed, 43 insertions(+), 581 deletions(-)

delete mode 100644 arch/h8300/lib/strncpy.S delete mode 100644 arch/hexagon/mm/strnlen_user.S

#30 lore raw
Quoted 10 lines · Vineet Gupta
Quoted 7 lines · Arnd Bergmann

If I hear no objections from architecture maintainers or new build regressions, I'll queue these up in the asm-generic tree for 5.15.

Arnd

Are you planning to add this to asm-generic tree for 5.15 anytime soon.

Actually I just did that yesterday, but it was too late for linux-next that day. ;-)

Quoted 2 lines

Also while there, any chance you could pick up [1] too which was Acked by Will.

Done now as well, thanks for the pointer.

Arnd