Inter-revision diff: patch 5

Comparing v2 (message) to v1 (message)

--- v2
+++ v1
@@ -1,121 +1,341 @@
 From: Arnd Bergmann <arnd@arndb.de>
 
-The __range_not_ok() helper is an x86 (and sparc64) specific interface
-that does roughly the same thing as __access_ok(), but with different
-calling conventions.
-
-Change this to use the normal interface in order for consistency as we
-clean up all access_ok() implementations.
-
-This changes the limit from TASK_SIZE to TASK_SIZE_MAX, which Al points
-out is the right thing do do here anyway.
-
-The callers have to use __access_ok() instead of the normal access_ok()
-though, because on x86 that contains a WARN_ON_IN_IRQ() check that cannot
-be used inside of NMI context while tracing.
-
-Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
-Suggested-by: Christoph Hellwig <hch@infradead.org>
-Link: https://lore.kernel.org/lkml/YgsUKcXGR7r4nINj@zeniv-ca.linux.org.uk/
+All architectures that don't provide __{get,put}_kernel_nofault() yet
+can implement this on top of __{get,put}_user.
+
+Add a generic version that lets everything use the normal
+copy_{from,to}_kernel_nofault() code based on these, removing the last
+use of get_fs()/set_fs() from architecture-independent code.
+
 Signed-off-by: Arnd Bergmann <arnd@arndb.de>
 ---
- arch/x86/events/core.c         |  2 +-
- arch/x86/include/asm/uaccess.h | 10 ++++++----
- arch/x86/kernel/dumpstack.c    |  2 +-
- arch/x86/kernel/stacktrace.c   |  2 +-
- arch/x86/lib/usercopy.c        |  2 +-
- 5 files changed, 10 insertions(+), 8 deletions(-)
-
-diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
-index e686c5e0537b..eef816fc216d 100644
---- a/arch/x86/events/core.c
-+++ b/arch/x86/events/core.c
-@@ -2794,7 +2794,7 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
- static inline int
- valid_user_frame(const void __user *fp, unsigned long size)
- {
--	return (__range_not_ok(fp, size, TASK_SIZE) == 0);
-+	return __access_ok(fp, size);
- }
- 
- static unsigned long get_segment_base(unsigned int segment)
+ arch/arm/include/asm/uaccess.h      |   2 -
+ arch/arm64/include/asm/uaccess.h    |   2 -
+ arch/m68k/include/asm/uaccess.h     |   2 -
+ arch/mips/include/asm/uaccess.h     |   2 -
+ arch/parisc/include/asm/uaccess.h   |   1 -
+ arch/powerpc/include/asm/uaccess.h  |   2 -
+ arch/riscv/include/asm/uaccess.h    |   2 -
+ arch/s390/include/asm/uaccess.h     |   2 -
+ arch/sparc/include/asm/uaccess_64.h |   2 -
+ arch/um/include/asm/uaccess.h       |   2 -
+ arch/x86/include/asm/uaccess.h      |   2 -
+ include/asm-generic/uaccess.h       |   2 -
+ include/linux/uaccess.h             |  19 +++++
+ mm/maccess.c                        | 108 ----------------------------
+ 14 files changed, 19 insertions(+), 131 deletions(-)
+
+diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
+index 32dbfd81f42a..d20d78c34b94 100644
+--- a/arch/arm/include/asm/uaccess.h
++++ b/arch/arm/include/asm/uaccess.h
+@@ -476,8 +476,6 @@ do {									\
+ 	: "r" (x), "i" (-EFAULT)				\
+ 	: "cc")
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ #define __get_kernel_nofault(dst, src, type, err_label)			\
+ do {									\
+ 	const type *__pk_ptr = (src);					\
+diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
+index 3a5ff5e20586..2e20879fe3cf 100644
+--- a/arch/arm64/include/asm/uaccess.h
++++ b/arch/arm64/include/asm/uaccess.h
+@@ -26,8 +26,6 @@
+ #include <asm/memory.h>
+ #include <asm/extable.h>
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ /*
+  * Test whether a block of memory is a valid user space address.
+  * Returns 1 if the range is valid, 0 otherwise.
+diff --git a/arch/m68k/include/asm/uaccess.h b/arch/m68k/include/asm/uaccess.h
+index ba670523885c..79617c0b2f91 100644
+--- a/arch/m68k/include/asm/uaccess.h
++++ b/arch/m68k/include/asm/uaccess.h
+@@ -390,8 +390,6 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n)
+ #define INLINE_COPY_FROM_USER
+ #define INLINE_COPY_TO_USER
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ #define __get_kernel_nofault(dst, src, type, err_label)			\
+ do {									\
+ 	type *__gk_dst = (type *)(dst);					\
+diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h
+index f8f74f9f5883..db9a8e002b62 100644
+--- a/arch/mips/include/asm/uaccess.h
++++ b/arch/mips/include/asm/uaccess.h
+@@ -296,8 +296,6 @@ struct __large_struct { unsigned long buf[100]; };
+ 	(val) = __gu_tmp.t;						\
+ }
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ #define __get_kernel_nofault(dst, src, type, err_label)			\
+ do {									\
+ 	int __gu_err;							\
+diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
+index ebf8a845b017..0925bbd6db67 100644
+--- a/arch/parisc/include/asm/uaccess.h
++++ b/arch/parisc/include/asm/uaccess.h
+@@ -95,7 +95,6 @@ struct exception_table_entry {
+ 	(val) = (__force __typeof__(*(ptr))) __gu_val;	\
+ }
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+ #define __get_kernel_nofault(dst, src, type, err_label)	\
+ {							\
+ 	type __z;					\
+diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
+index 63316100080c..a0032c2e7550 100644
+--- a/arch/powerpc/include/asm/uaccess.h
++++ b/arch/powerpc/include/asm/uaccess.h
+@@ -467,8 +467,6 @@ do {									\
+ 		unsafe_put_user(*(u8*)(_src + _i), (u8 __user *)(_dst + _i), e); \
+ } while (0)
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ #define __get_kernel_nofault(dst, src, type, err_label)			\
+ 	__get_user_size_goto(*((type *)(dst)),				\
+ 		(__force type __user *)(src), sizeof(type), err_label)
+diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
+index c701a5e57a2b..4407b9e48d2c 100644
+--- a/arch/riscv/include/asm/uaccess.h
++++ b/arch/riscv/include/asm/uaccess.h
+@@ -346,8 +346,6 @@ unsigned long __must_check clear_user(void __user *to, unsigned long n)
+ 		__clear_user(to, n) : n;
+ }
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ #define __get_kernel_nofault(dst, src, type, err_label)			\
+ do {									\
+ 	long __kr_err;							\
+diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
+index d74e26b48604..29332edf46f0 100644
+--- a/arch/s390/include/asm/uaccess.h
++++ b/arch/s390/include/asm/uaccess.h
+@@ -282,8 +282,6 @@ static inline unsigned long __must_check clear_user(void __user *to, unsigned lo
+ int copy_to_user_real(void __user *dest, void *src, unsigned long count);
+ void *s390_kernel_write(void *dst, const void *src, size_t size);
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ int __noreturn __put_kernel_bad(void);
+ 
+ #define __put_kernel_asm(val, to, insn)					\
+diff --git a/arch/sparc/include/asm/uaccess_64.h b/arch/sparc/include/asm/uaccess_64.h
+index b283798315b1..5c12fb46bc61 100644
+--- a/arch/sparc/include/asm/uaccess_64.h
++++ b/arch/sparc/include/asm/uaccess_64.h
+@@ -210,8 +210,6 @@ __asm__ __volatile__(							\
+ 	       : "=r" (ret), "=r" (x) : "r" (__m(addr)),		\
+ 		 "i" (-EFAULT))
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ #define __get_user_nocheck(data, addr, size, type) ({			     \
+ 	register int __gu_ret;						     \
+ 	register unsigned long __gu_val;				     \
+diff --git a/arch/um/include/asm/uaccess.h b/arch/um/include/asm/uaccess.h
+index 17d18cfd82a5..1ecfc96bcc50 100644
+--- a/arch/um/include/asm/uaccess.h
++++ b/arch/um/include/asm/uaccess.h
+@@ -44,8 +44,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
+ }
+ 
+ /* no pagefaults for kernel addresses in um */
+-#define HAVE_GET_KERNEL_NOFAULT 1
+-
+ #define __get_kernel_nofault(dst, src, type, err_label)			\
+ do {									\
+ 	*((type *)dst) = get_unaligned((type *)(src));			\
 diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
-index ac96f9b2d64b..79c4869ccdd6 100644
+index 6956a63291b6..c6d9dc42724d 100644
 --- a/arch/x86/include/asm/uaccess.h
 +++ b/arch/x86/include/asm/uaccess.h
-@@ -16,8 +16,10 @@
-  * Test whether a block of memory is a valid user space address.
-  * Returns 0 if the range is valid, nonzero otherwise.
-  */
--static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, unsigned long limit)
-+static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size)
+@@ -510,8 +510,6 @@ do {									\
+ 	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u8, label);	\
+ } while (0)
+ 
+-#define HAVE_GET_KERNEL_NOFAULT
+-
+ #ifdef CONFIG_CC_HAS_ASM_GOTO_OUTPUT
+ #define __get_kernel_nofault(dst, src, type, err_label)			\
+ 	__get_user_size(*((type *)(dst)), (__force type __user *)(src),	\
+diff --git a/include/asm-generic/uaccess.h b/include/asm-generic/uaccess.h
+index 10ffa8b5c117..0870fa11a7c5 100644
+--- a/include/asm-generic/uaccess.h
++++ b/include/asm-generic/uaccess.h
+@@ -77,8 +77,6 @@ do {									\
+ 		goto err_label;						\
+ } while (0)
+ 
+-#define HAVE_GET_KERNEL_NOFAULT 1
+-
+ static inline __must_check unsigned long
+ raw_copy_from_user(void *to, const void __user * from, unsigned long n)
  {
-+	unsigned long limit = TASK_SIZE_MAX;
+diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
+index ac0394087f7d..67e9bc94dc40 100644
+--- a/include/linux/uaccess.h
++++ b/include/linux/uaccess.h
+@@ -368,6 +368,25 @@ long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
+ 		long count);
+ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
+ 
++#ifndef __get_kernel_nofault
++#define __get_kernel_nofault(dst, src, type, label)	\
++do {							\
++	type __user *p = (type __force __user *)(src);	\
++	type data;					\
++	if (__get_user(data, p))			\
++		goto label;				\
++	*(type *)dst = data;				\
++} while (0)
 +
- 	/*
- 	 * If we have used "sizeof()" for the size,
- 	 * we know it won't overflow the limit (but
-@@ -35,10 +37,10 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
- 	return unlikely(addr > limit);
- }
- 
--#define __range_not_ok(addr, size, limit)				\
-+#define __access_ok(addr, size)						\
- ({									\
- 	__chk_user_ptr(addr);						\
--	__chk_range_not_ok((unsigned long __force)(addr), size, limit); \
-+	!__chk_range_not_ok((unsigned long __force)(addr), size);	\
- })
- 
- #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
-@@ -69,7 +71,7 @@ static inline bool pagefault_disabled(void);
- #define access_ok(addr, size)					\
- ({									\
- 	WARN_ON_IN_IRQ();						\
--	likely(!__range_not_ok(addr, size, TASK_SIZE_MAX));		\
-+	likely(__access_ok(addr, size));				\
- })
- 
- extern int __get_user_1(void);
-diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
-index 53de044e5654..da534fb7b5c6 100644
---- a/arch/x86/kernel/dumpstack.c
-+++ b/arch/x86/kernel/dumpstack.c
-@@ -85,7 +85,7 @@ static int copy_code(struct pt_regs *regs, u8 *buf, unsigned long src,
- 	 * Make sure userspace isn't trying to trick us into dumping kernel
- 	 * memory by pointing the userspace instruction pointer at it.
- 	 */
--	if (__chk_range_not_ok(src, nbytes, TASK_SIZE_MAX))
-+	if (!__access_ok((void __user *)src, nbytes))
- 		return -EINVAL;
- 
- 	/*
-diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
-index 15b058eefc4e..ee117fcf46ed 100644
---- a/arch/x86/kernel/stacktrace.c
-+++ b/arch/x86/kernel/stacktrace.c
-@@ -90,7 +90,7 @@ copy_stack_frame(const struct stack_frame_user __user *fp,
- {
- 	int ret;
- 
--	if (__range_not_ok(fp, sizeof(*frame), TASK_SIZE))
-+	if (!__access_ok(fp, sizeof(*frame)))
- 		return 0;
- 
- 	ret = 1;
-diff --git a/arch/x86/lib/usercopy.c b/arch/x86/lib/usercopy.c
-index c3e8a62ca561..ad0139d25401 100644
---- a/arch/x86/lib/usercopy.c
-+++ b/arch/x86/lib/usercopy.c
-@@ -32,7 +32,7 @@ copy_from_user_nmi(void *to, const void __user *from, unsigned long n)
- {
- 	unsigned long ret;
- 
--	if (__range_not_ok(from, n, TASK_SIZE))
-+	if (!__access_ok(from, n))
- 		return n;
- 
- 	if (!nmi_uaccess_okay())
++#define __put_kernel_nofault(dst, src, type, label)	\
++do {							\
++	type __user *p = (type __force __user *)(dst);	\
++	type data = *(type *)src;			\
++	if (__put_user(data, p))			\
++		goto label;				\
++} while (0)
++#endif
++
+ /**
+  * get_kernel_nofault(): safely attempt to read from a location
+  * @val: read into this variable
+diff --git a/mm/maccess.c b/mm/maccess.c
+index d3f1a1f0b1c1..cbd1b3959af2 100644
+--- a/mm/maccess.c
++++ b/mm/maccess.c
+@@ -12,8 +12,6 @@ bool __weak copy_from_kernel_nofault_allowed(const void *unsafe_src,
+ 	return true;
+ }
+ 
+-#ifdef HAVE_GET_KERNEL_NOFAULT
+-
+ #define copy_from_kernel_nofault_loop(dst, src, len, type, err_label)	\
+ 	while (len >= sizeof(type)) {					\
+ 		__get_kernel_nofault(dst, src, type, err_label);		\
+@@ -102,112 +100,6 @@ long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
+ 	dst[-1] = '\0';
+ 	return -EFAULT;
+ }
+-#else /* HAVE_GET_KERNEL_NOFAULT */
+-/**
+- * copy_from_kernel_nofault(): safely attempt to read from kernel-space
+- * @dst: pointer to the buffer that shall take the data
+- * @src: address to read from
+- * @size: size of the data chunk
+- *
+- * Safely read from kernel address @src to the buffer at @dst.  If a kernel
+- * fault happens, handle that and return -EFAULT.  If @src is not a valid kernel
+- * address, return -ERANGE.
+- *
+- * We ensure that the copy_from_user is executed in atomic context so that
+- * do_page_fault() doesn't attempt to take mmap_lock.  This makes
+- * copy_from_kernel_nofault() suitable for use within regions where the caller
+- * already holds mmap_lock, or other locks which nest inside mmap_lock.
+- */
+-long copy_from_kernel_nofault(void *dst, const void *src, size_t size)
+-{
+-	long ret;
+-	mm_segment_t old_fs = get_fs();
+-
+-	if (!copy_from_kernel_nofault_allowed(src, size))
+-		return -ERANGE;
+-
+-	set_fs(KERNEL_DS);
+-	pagefault_disable();
+-	ret = __copy_from_user_inatomic(dst, (__force const void __user *)src,
+-			size);
+-	pagefault_enable();
+-	set_fs(old_fs);
+-
+-	if (ret)
+-		return -EFAULT;
+-	return 0;
+-}
+-EXPORT_SYMBOL_GPL(copy_from_kernel_nofault);
+-
+-/**
+- * copy_to_kernel_nofault(): safely attempt to write to a location
+- * @dst: address to write to
+- * @src: pointer to the data that shall be written
+- * @size: size of the data chunk
+- *
+- * Safely write to address @dst from the buffer at @src.  If a kernel fault
+- * happens, handle that and return -EFAULT.
+- */
+-long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
+-{
+-	long ret;
+-	mm_segment_t old_fs = get_fs();
+-
+-	set_fs(KERNEL_DS);
+-	pagefault_disable();
+-	ret = __copy_to_user_inatomic((__force void __user *)dst, src, size);
+-	pagefault_enable();
+-	set_fs(old_fs);
+-
+-	if (ret)
+-		return -EFAULT;
+-	return 0;
+-}
+-
+-/**
+- * strncpy_from_kernel_nofault: - Copy a NUL terminated string from unsafe
+- *				 address.
+- * @dst:   Destination address, in kernel space.  This buffer must be at
+- *         least @count bytes long.
+- * @unsafe_addr: Unsafe address.
+- * @count: Maximum number of bytes to copy, including the trailing NUL.
+- *
+- * Copies a NUL-terminated string from unsafe address to kernel buffer.
+- *
+- * On success, returns the length of the string INCLUDING the trailing NUL.
+- *
+- * If access fails, returns -EFAULT (some data may have been copied and the
+- * trailing NUL added).  If @unsafe_addr is not a valid kernel address, return
+- * -ERANGE.
+- *
+- * If @count is smaller than the length of the string, copies @count-1 bytes,
+- * sets the last byte of @dst buffer to NUL and returns @count.
+- */
+-long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
+-{
+-	mm_segment_t old_fs = get_fs();
+-	const void *src = unsafe_addr;
+-	long ret;
+-
+-	if (unlikely(count <= 0))
+-		return 0;
+-	if (!copy_from_kernel_nofault_allowed(unsafe_addr, count))
+-		return -ERANGE;
+-
+-	set_fs(KERNEL_DS);
+-	pagefault_disable();
+-
+-	do {
+-		ret = __get_user(*dst++, (const char __user __force *)src++);
+-	} while (dst[-1] && ret == 0 && src - unsafe_addr < count);
+-
+-	dst[-1] = '\0';
+-	pagefault_enable();
+-	set_fs(old_fs);
+-
+-	return ret ? -EFAULT : src - unsafe_addr;
+-}
+-#endif /* HAVE_GET_KERNEL_NOFAULT */
+ 
+ /**
+  * copy_from_user_nofault(): safely attempt to read from a user-space location
 -- 
 2.29.2
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help