diff mbox series

[5/6,v2] asm-generic: uaccess: remove inline strncpy_from_user/strnlen_user

Message ID 20210515101803.924427-6-arnd@kernel.org
State New
Headers show
Series asm-generic: strncpy_from_user/strnlen_user cleanup | expand

Commit Message

Arnd Bergmann May 15, 2021, 10:18 a.m. UTC 
From: Arnd Bergmann <arnd@arndb.de>

Consolidate the asm-generic implementation with the library version
that is used everywhere else.

These are the three versions for NOMMU kernels, which can in principle
fall back to strncpy()/strnlen() as the version in asm-generic does,
but this version is particularly inefficient when it scans the string
one byte at a time twice. The generic version also lacks a check
for user_addr_max(), but this is probably ok on NOMMU targets.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 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(-)

Comments

Christoph Hellwig May 17, 2021, 6:20 a.m. UTC | #1 
On Sat, May 15, 2021 at 12:18:02PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Consolidate the asm-generic implementation with the library version
> that is used everywhere else.
> 
> These are the three versions for NOMMU kernels,

I don't get the three versions part?

> +	select GENERIC_STRNCPY_FROM_USER
> +	select GENERIC_STRNLEN_USER

Given that most architetures select the generic version I wonder
if it might be worth to add another patch to invert the logic so
that architectures with their own implementation need to sekect a symbol.

> +extern long strncpy_from_user(char *dst, const char __user *src, long count);
> +extern long strnlen_user(const char __user *src, long n);

No need for the extern here.
Arnd Bergmann May 17, 2021, 7:27 a.m. UTC | #2 
On Mon, May 17, 2021 at 8:20 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Sat, May 15, 2021 at 12:18:02PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > Consolidate the asm-generic implementation with the library version
> > that is used everywhere else.
> >
> > These are the three versions for NOMMU kernels,
>
> I don't get the three versions part?

Right, that was confusing. Rewording to

| 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.

> > +     select GENERIC_STRNCPY_FROM_USER
> > +     select GENERIC_STRNLEN_USER
>
> Given that most architetures select the generic version I wonder
> if it might be worth to add another patch to invert the logic so
> that architectures with their own implementation need to sekect a symbol.

Done now, using 'CONFIG_ARCH_HAS_{STRNCPY_FROM,STRNLEN}_USER'.

There are still seven or eight architectures that provide their own though.

> > +extern long strncpy_from_user(char *dst, const char __user *src, long count);
> > +extern long strnlen_user(const char __user *src, long n);
>
> No need for the extern here.

Removed.

       Arnd
diff mbox series

Patch

diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 3e3e0f16f7e0..53dfd2d47e0e 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -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
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index ed6b8050e3ed..5f1aafa7b2e2 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -17,8 +17,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
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a8ad8eb76120..ada7a2918c05 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -55,8 +55,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
diff --git a/include/asm-generic/uaccess.h b/include/asm-generic/uaccess.h
index c03889cc904c..83a48f430951 100644
--- a/include/asm-generic/uaccess.h
+++ b/include/asm-generic/uaccess.h
@@ -119,6 +119,11 @@  static inline void set_fs(mm_segment_t fs)
 #ifndef uaccess_kernel
 #define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 #endif
+
+#ifndef user_addr_max
+#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)
+#endif
+
 #endif /* CONFIG_SET_FS */
 
 #define access_ok(addr, size) __access_ok((unsigned long)(addr),(size))
@@ -243,44 +248,6 @@  static inline int __get_user_fn(size_t size, const void __user *ptr, void *x)
 
 extern int __get_user_bad(void) __attribute__((noreturn));
 
-/*
- * Copy a null terminated string from userspace.
- */
-#ifndef strncpy_from_user
-static inline long
-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--)
-		;
-	return (tmp - dst);
-}
-#endif
-
-#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
- *
- * 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.
- */
-static inline long strnlen_user(const char __user *src, long n)
-{
-	if (!access_ok(src, 1))
-		return 0;
-
-	return strnlen(src, n) + 1;
-}
-#endif
-
 /*
  * Zero Userspace
  */
@@ -305,4 +272,7 @@  clear_user(void __user *to, unsigned long n)
 
 #include <asm/extable.h>
 
+extern long strncpy_from_user(char *dst, const char __user *src, long count);
+extern long strnlen_user(const char __user *src, long n);
+
 #endif /* __ASM_GENERIC_UACCESS_H */