linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* clean up address limit helpers
@ 2020-07-10 13:57 Christoph Hellwig
  2020-07-10 13:57 ` [PATCH 1/6] syscalls: use uaccess_kernel in addr_limit_user_check Christoph Hellwig
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-10 13:57 UTC (permalink / raw)
  To: Nick Hu, Greentime Hu, Vincent Chen, Paul Walmsley,
	Palmer Dabbelt, Andrew Morton, Linus Torvalds
  Cc: linux-arch, linux-riscv, linux-kernel

Hi all,

in preparation for eventually phasing out direct use of set_fs(), this
series removes the segment_eq() arch helper that is only used to
implement or duplicate the uaccess_kernel() API, and then adds
descriptive helpers to force the kernel address limit.

Diffstat:
 arch/alpha/include/asm/uaccess.h      |    2 +-
 arch/arc/include/asm/segment.h        |    3 +--
 arch/arm/include/asm/uaccess.h        |    4 ++--
 arch/arm64/include/asm/uaccess.h      |    2 +-
 arch/arm64/kernel/sdei.c              |    2 +-
 arch/csky/include/asm/segment.h       |    2 +-
 arch/h8300/include/asm/segment.h      |    2 +-
 arch/ia64/include/asm/uaccess.h       |    2 +-
 arch/m68k/include/asm/segment.h       |    2 +-
 arch/m68k/include/asm/tlbflush.h      |   12 ++++++------
 arch/microblaze/include/asm/uaccess.h |    2 +-
 arch/mips/include/asm/uaccess.h       |    2 +-
 arch/mips/kernel/unaligned.c          |   27 +++++++++++++--------------
 arch/nds32/include/asm/uaccess.h      |    2 +-
 arch/nds32/kernel/process.c           |    2 +-
 arch/nds32/mm/alignment.c             |    7 +++----
 arch/nios2/include/asm/uaccess.h      |    2 +-
 arch/openrisc/include/asm/uaccess.h   |    2 +-
 arch/parisc/include/asm/uaccess.h     |    2 +-
 arch/powerpc/include/asm/uaccess.h    |    3 +--
 arch/riscv/include/asm/uaccess.h      |    6 +++---
 arch/s390/include/asm/uaccess.h       |    2 +-
 arch/sh/include/asm/segment.h         |    3 +--
 arch/sh/kernel/traps_32.c             |   18 ++++++++----------
 arch/sparc/include/asm/uaccess_32.h   |    2 +-
 arch/sparc/include/asm/uaccess_64.h   |    2 +-
 arch/x86/include/asm/uaccess.h        |    2 +-
 arch/xtensa/include/asm/uaccess.h     |    2 +-
 drivers/firmware/arm_sdei.c           |    5 ++---
 fs/exec.c                             |    7 ++++++-
 include/asm-generic/uaccess.h         |    4 ++--
 include/linux/syscalls.h              |    2 +-
 include/linux/uaccess.h               |   20 ++++++++++++++++++--
 kernel/events/callchain.c             |    5 ++---
 kernel/events/core.c                  |    5 ++---
 kernel/exit.c                         |    2 +-
 kernel/kthread.c                      |    5 ++---
 kernel/stacktrace.c                   |    5 ++---
 mm/maccess.c                          |   22 ++++++++++------------
 39 files changed, 105 insertions(+), 98 deletions(-)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/6] syscalls: use uaccess_kernel in addr_limit_user_check
  2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
@ 2020-07-10 13:57 ` Christoph Hellwig
  2020-07-10 13:57 ` [PATCH 2/6] nds32: use uaccess_kernel in show_regs Christoph Hellwig
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-10 13:57 UTC (permalink / raw)
  To: Nick Hu, Greentime Hu, Vincent Chen, Paul Walmsley,
	Palmer Dabbelt, Andrew Morton, Linus Torvalds
  Cc: linux-arch, linux-riscv, linux-kernel

Use the uaccess_kernel helper instead of duplicating it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/syscalls.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index b951a87da9877c..e933a43d4a69ac 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -263,7 +263,7 @@ static inline void addr_limit_user_check(void)
 		return;
 #endif
 
-	if (CHECK_DATA_CORRUPTION(!segment_eq(get_fs(), USER_DS),
+	if (CHECK_DATA_CORRUPTION(uaccess_kernel(),
 				  "Invalid address limit on user-mode return"))
 		force_sig(SIGKILL);
 
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/6] nds32: use uaccess_kernel in show_regs
  2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
  2020-07-10 13:57 ` [PATCH 1/6] syscalls: use uaccess_kernel in addr_limit_user_check Christoph Hellwig
@ 2020-07-10 13:57 ` Christoph Hellwig
  2020-07-13 10:02   ` Greentime Hu
  2020-07-10 13:57 ` [PATCH 3/6] riscv: include <asm/pgtable.h> in <asm/uaccess.h> Christoph Hellwig
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-10 13:57 UTC (permalink / raw)
  To: Nick Hu, Greentime Hu, Vincent Chen, Paul Walmsley,
	Palmer Dabbelt, Andrew Morton, Linus Torvalds
  Cc: linux-arch, linux-riscv, linux-kernel

Use the uaccess_kernel helper instead of duplicating it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/nds32/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/nds32/kernel/process.c b/arch/nds32/kernel/process.c
index 9712fd474f2ca3..f06265949ec28b 100644
--- a/arch/nds32/kernel/process.c
+++ b/arch/nds32/kernel/process.c
@@ -121,7 +121,7 @@ void show_regs(struct pt_regs *regs)
 		regs->uregs[3], regs->uregs[2], regs->uregs[1], regs->uregs[0]);
 	pr_info("  IRQs o%s  Segment %s\n",
 		interrupts_enabled(regs) ? "n" : "ff",
-		segment_eq(get_fs(), KERNEL_DS)? "kernel" : "user");
+		uaccess_kernel() ? "kernel" : "user");
 }
 
 EXPORT_SYMBOL(show_regs);
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/6] riscv: include <asm/pgtable.h> in <asm/uaccess.h>
  2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
  2020-07-10 13:57 ` [PATCH 1/6] syscalls: use uaccess_kernel in addr_limit_user_check Christoph Hellwig
  2020-07-10 13:57 ` [PATCH 2/6] nds32: use uaccess_kernel in show_regs Christoph Hellwig
@ 2020-07-10 13:57 ` Christoph Hellwig
  2020-07-21 23:02   ` Palmer Dabbelt
  2020-07-10 13:57 ` [PATCH 4/6] uaccess: remove segment_eq Christoph Hellwig
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-10 13:57 UTC (permalink / raw)
  To: Nick Hu, Greentime Hu, Vincent Chen, Paul Walmsley,
	Palmer Dabbelt, Andrew Morton, Linus Torvalds
  Cc: linux-arch, linux-riscv, linux-kernel

To ensure TASK_SIZE is defined for USER_DS.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/include/asm/uaccess.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 8ce9d607b53dce..22de922d6ecb2f 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -8,6 +8,8 @@
 #ifndef _ASM_RISCV_UACCESS_H
 #define _ASM_RISCV_UACCESS_H
 
+#include <asm/pgtable.h>		/* for TASK_SIZE */
+
 /*
  * User space memory access functions
  */
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/6] uaccess: remove segment_eq
  2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-07-10 13:57 ` [PATCH 3/6] riscv: include <asm/pgtable.h> in <asm/uaccess.h> Christoph Hellwig
@ 2020-07-10 13:57 ` Christoph Hellwig
  2020-07-13  9:16   ` Geert Uytterhoeven
  2020-07-13 10:05   ` Greentime Hu
  2020-07-10 13:57 ` [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers Christoph Hellwig
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-10 13:57 UTC (permalink / raw)
  To: Nick Hu, Greentime Hu, Vincent Chen, Paul Walmsley,
	Palmer Dabbelt, Andrew Morton, Linus Torvalds
  Cc: linux-arch, linux-riscv, linux-kernel

segment_eq is only used to implement uaccess_kernel.  Just open code
uaccess_kernel in the arch uaccess headers and remove one layer of
indirection.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/alpha/include/asm/uaccess.h      | 2 +-
 arch/arc/include/asm/segment.h        | 3 +--
 arch/arm/include/asm/uaccess.h        | 4 ++--
 arch/arm64/include/asm/uaccess.h      | 2 +-
 arch/csky/include/asm/segment.h       | 2 +-
 arch/h8300/include/asm/segment.h      | 2 +-
 arch/ia64/include/asm/uaccess.h       | 2 +-
 arch/m68k/include/asm/segment.h       | 2 +-
 arch/microblaze/include/asm/uaccess.h | 2 +-
 arch/mips/include/asm/uaccess.h       | 2 +-
 arch/nds32/include/asm/uaccess.h      | 2 +-
 arch/nios2/include/asm/uaccess.h      | 2 +-
 arch/openrisc/include/asm/uaccess.h   | 2 +-
 arch/parisc/include/asm/uaccess.h     | 2 +-
 arch/powerpc/include/asm/uaccess.h    | 3 +--
 arch/riscv/include/asm/uaccess.h      | 4 +---
 arch/s390/include/asm/uaccess.h       | 2 +-
 arch/sh/include/asm/segment.h         | 3 +--
 arch/sparc/include/asm/uaccess_32.h   | 2 +-
 arch/sparc/include/asm/uaccess_64.h   | 2 +-
 arch/x86/include/asm/uaccess.h        | 2 +-
 arch/xtensa/include/asm/uaccess.h     | 2 +-
 include/asm-generic/uaccess.h         | 4 ++--
 include/linux/uaccess.h               | 2 --
 24 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/arch/alpha/include/asm/uaccess.h b/arch/alpha/include/asm/uaccess.h
index 1fe2b56cb861fe..1b6f25efa247f0 100644
--- a/arch/alpha/include/asm/uaccess.h
+++ b/arch/alpha/include/asm/uaccess.h
@@ -20,7 +20,7 @@
 #define get_fs()  (current_thread_info()->addr_limit)
 #define set_fs(x) (current_thread_info()->addr_limit = (x))
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel()	(get_fs().seg == KERNEL_DS.seg)
 
 /*
  * Is a address valid? This does a straightforward calculation rather
diff --git a/arch/arc/include/asm/segment.h b/arch/arc/include/asm/segment.h
index 6a2a5be5026de7..871f8ab11bfd02 100644
--- a/arch/arc/include/asm/segment.h
+++ b/arch/arc/include/asm/segment.h
@@ -14,8 +14,7 @@ typedef unsigned long mm_segment_t;
 
 #define KERNEL_DS		MAKE_MM_SEG(0)
 #define USER_DS			MAKE_MM_SEG(TASK_SIZE)
-
-#define segment_eq(a, b)	((a) == (b))
+#define uaccess_kernel()	(get_fs() == KERNEL_DS)
 
 #endif /* __ASSEMBLY__ */
 #endif /* __ASMARC_SEGMENT_H */
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 98c6b91be4a8ad..b19c9bec1f7a63 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -76,7 +76,7 @@ static inline void set_fs(mm_segment_t fs)
 	modify_domain(DOMAIN_KERNEL, fs ? DOMAIN_CLIENT : DOMAIN_MANAGER);
 }
 
-#define segment_eq(a, b)	((a) == (b))
+#define uaccess_kernel()	(get_fs() == KERNEL_DS)
 
 /* We use 33-bit arithmetic here... */
 #define __range_ok(addr, size) ({ \
@@ -263,7 +263,7 @@ extern int __put_user_8(void *, unsigned long long);
  */
 #define USER_DS			KERNEL_DS
 
-#define segment_eq(a, b)		(1)
+#define uaccess_kernel()	(true)
 #define __addr_ok(addr)		((void)(addr), 1)
 #define __range_ok(addr, size)	((void)(addr), 0)
 #define get_fs()		(KERNEL_DS)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index bc5c7b09115205..fcb8174de505ea 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -49,7 +49,7 @@ static inline void set_fs(mm_segment_t fs)
 				CONFIG_ARM64_UAO));
 }
 
-#define segment_eq(a, b)	((a) == (b))
+#define uaccess_kernel()	(get_fs() == KERNEL_DS)
 
 /*
  * Test whether a block of memory is a valid user space address.
diff --git a/arch/csky/include/asm/segment.h b/arch/csky/include/asm/segment.h
index db2640d5f57591..79ede9b1a6467f 100644
--- a/arch/csky/include/asm/segment.h
+++ b/arch/csky/include/asm/segment.h
@@ -13,6 +13,6 @@ typedef struct {
 #define USER_DS			((mm_segment_t) { 0x80000000UL })
 #define get_fs()		(current_thread_info()->addr_limit)
 #define set_fs(x)		(current_thread_info()->addr_limit = (x))
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel()	(get_fs().seg == KERNEL_DS.seg)
 
 #endif /* __ASM_CSKY_SEGMENT_H */
diff --git a/arch/h8300/include/asm/segment.h b/arch/h8300/include/asm/segment.h
index a407978f9f9fb8..37950725d9b9c8 100644
--- a/arch/h8300/include/asm/segment.h
+++ b/arch/h8300/include/asm/segment.h
@@ -33,7 +33,7 @@ static inline mm_segment_t get_fs(void)
 	return USER_DS;
 }
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel()	(get_fs().seg == KERNEL_DS.seg)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/ia64/include/asm/uaccess.h b/arch/ia64/include/asm/uaccess.h
index 8aa473a4b0f4ef..179243c3dfc702 100644
--- a/arch/ia64/include/asm/uaccess.h
+++ b/arch/ia64/include/asm/uaccess.h
@@ -50,7 +50,7 @@
 #define get_fs()  (current_thread_info()->addr_limit)
 #define set_fs(x) (current_thread_info()->addr_limit = (x))
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel()	(get_fs().seg == KERNEL_DS.seg)
 
 /*
  * When accessing user memory, we need to make sure the entire area really is in
diff --git a/arch/m68k/include/asm/segment.h b/arch/m68k/include/asm/segment.h
index c6686559e9b742..2b5e68a71ef78e 100644
--- a/arch/m68k/include/asm/segment.h
+++ b/arch/m68k/include/asm/segment.h
@@ -52,7 +52,7 @@ static inline void set_fs(mm_segment_t val)
 #define set_fs(x)	(current_thread_info()->addr_limit = (x))
 #endif
 
-#define segment_eq(a, b) ((a).seg == (b).seg)
+#define uaccess_kernel()	(get_fs().seg == KERNEL_DS.seg)
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h
index 6723c56ec3783b..304b04ffea2faf 100644
--- a/arch/microblaze/include/asm/uaccess.h
+++ b/arch/microblaze/include/asm/uaccess.h
@@ -41,7 +41,7 @@
 # define get_fs()	(current_thread_info()->addr_limit)
 # define set_fs(val)	(current_thread_info()->addr_limit = (val))
 
-# define segment_eq(a, b)	((a).seg == (b).seg)
+# define uaccess_kernel()	(get_fs().seg == KERNEL_DS.seg)
 
 #ifndef CONFIG_MMU
 
diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h
index 62b298c50905f6..61fc01f177a64b 100644
--- a/arch/mips/include/asm/uaccess.h
+++ b/arch/mips/include/asm/uaccess.h
@@ -72,7 +72,7 @@ extern u64 __ua_limit;
 #define get_fs()	(current_thread_info()->addr_limit)
 #define set_fs(x)	(current_thread_info()->addr_limit = (x))
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel()	(get_fs().seg == KERNEL_DS.seg)
 
 /*
  * eva_kernel_access() - determine whether kernel memory access on an EVA system
diff --git a/arch/nds32/include/asm/uaccess.h b/arch/nds32/include/asm/uaccess.h
index 3a9219f53ee0d8..010ba5f1d7dd6b 100644
--- a/arch/nds32/include/asm/uaccess.h
+++ b/arch/nds32/include/asm/uaccess.h
@@ -44,7 +44,7 @@ static inline void set_fs(mm_segment_t fs)
 	current_thread_info()->addr_limit = fs;
 }
 
-#define segment_eq(a, b)	((a) == (b))
+#define uaccess_kernel()	(get_fs() == KERNEL_DS)
 
 #define __range_ok(addr, size) (size <= get_fs() && addr <= (get_fs() -size))
 
diff --git a/arch/nios2/include/asm/uaccess.h b/arch/nios2/include/asm/uaccess.h
index e83f831a76f939..a741abbed6fbf5 100644
--- a/arch/nios2/include/asm/uaccess.h
+++ b/arch/nios2/include/asm/uaccess.h
@@ -30,7 +30,7 @@
 #define get_fs()		(current_thread_info()->addr_limit)
 #define set_fs(seg)		(current_thread_info()->addr_limit = (seg))
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 
 #define __access_ok(addr, len)			\
 	(((signed long)(((long)get_fs().seg) &	\
diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index 17c24f14615fb5..48b691530d3ed4 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -43,7 +43,7 @@
 #define get_fs()	(current_thread_info()->addr_limit)
 #define set_fs(x)	(current_thread_info()->addr_limit = (x))
 
-#define segment_eq(a, b)	((a) == (b))
+#define uaccess_kernel()	(get_fs() == KERNEL_DS)
 
 /* Ensure that the range from addr to addr+size is all within the process'
  * address space
diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index ebbb9ffe038c76..ed2cd4fb479b0c 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -14,7 +14,7 @@
 #define KERNEL_DS	((mm_segment_t){0})
 #define USER_DS 	((mm_segment_t){1})
 
-#define segment_eq(a, b) ((a).seg == (b).seg)
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 
 #define get_fs()	(current_thread_info()->addr_limit)
 #define set_fs(x)	(current_thread_info()->addr_limit = (x))
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 64c04ab0911236..00699903f1efca 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -38,8 +38,7 @@ static inline void set_fs(mm_segment_t fs)
 	set_thread_flag(TIF_FSCHECK);
 }
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
-
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 #define user_addr_max()	(get_fs().seg)
 
 #ifdef __powerpc64__
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 22de922d6ecb2f..f56c66b3f5fe21 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -64,11 +64,9 @@ static inline void set_fs(mm_segment_t fs)
 	current_thread_info()->addr_limit = fs;
 }
 
-#define segment_eq(a, b) ((a).seg == (b).seg)
-
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 #define user_addr_max()	(get_fs().seg)
 
-
 /**
  * access_ok: - Checks if a user space pointer is valid
  * @addr: User space pointer to start of block to check
diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
index 324438889fe168..f09444d6aeab30 100644
--- a/arch/s390/include/asm/uaccess.h
+++ b/arch/s390/include/asm/uaccess.h
@@ -32,7 +32,7 @@
 #define USER_DS_SACF	(3)
 
 #define get_fs()        (current->thread.mm_segment)
-#define segment_eq(a,b) (((a) & 2) == ((b) & 2))
+#define uaccess_kernel() ((get_fs() & 2) == KERNEL_DS)
 
 void set_fs(mm_segment_t fs);
 
diff --git a/arch/sh/include/asm/segment.h b/arch/sh/include/asm/segment.h
index 33d1d28057cbfb..02e54a3335d68f 100644
--- a/arch/sh/include/asm/segment.h
+++ b/arch/sh/include/asm/segment.h
@@ -24,8 +24,7 @@ typedef struct {
 #define USER_DS		KERNEL_DS
 #endif
 
-#define segment_eq(a, b) ((a).seg == (b).seg)
-
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 
 #define get_fs()	(current_thread_info()->addr_limit)
 #define set_fs(x)	(current_thread_info()->addr_limit = (x))
diff --git a/arch/sparc/include/asm/uaccess_32.h b/arch/sparc/include/asm/uaccess_32.h
index d6d8413eca835a..0a2d3ebc4bb86d 100644
--- a/arch/sparc/include/asm/uaccess_32.h
+++ b/arch/sparc/include/asm/uaccess_32.h
@@ -28,7 +28,7 @@
 #define get_fs()	(current->thread.current_ds)
 #define set_fs(val)	((current->thread.current_ds) = (val))
 
-#define segment_eq(a, b) ((a).seg == (b).seg)
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 
 /* We have there a nice not-mapped page at PAGE_OFFSET - PAGE_SIZE, so that this test
  * can be fairly lightweight.
diff --git a/arch/sparc/include/asm/uaccess_64.h b/arch/sparc/include/asm/uaccess_64.h
index bf9d330073b235..698cf69f74e998 100644
--- a/arch/sparc/include/asm/uaccess_64.h
+++ b/arch/sparc/include/asm/uaccess_64.h
@@ -32,7 +32,7 @@
 
 #define get_fs() ((mm_segment_t){(current_thread_info()->current_ds)})
 
-#define segment_eq(a, b)  ((a).seg == (b).seg)
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 
 #define set_fs(val)								\
 do {										\
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 18dfa07d3ef0df..dd3261f9f4eaad 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -33,7 +33,7 @@ static inline void set_fs(mm_segment_t fs)
 	set_thread_flag(TIF_FSCHECK);
 }
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 #define user_addr_max() (current->thread.addr_limit.seg)
 
 /*
diff --git a/arch/xtensa/include/asm/uaccess.h b/arch/xtensa/include/asm/uaccess.h
index e57f0d0a88d8ba..b9758119feca19 100644
--- a/arch/xtensa/include/asm/uaccess.h
+++ b/arch/xtensa/include/asm/uaccess.h
@@ -35,7 +35,7 @@
 #define get_fs()	(current->thread.current_ds)
 #define set_fs(val)	(current->thread.current_ds = (val))
 
-#define segment_eq(a, b)	((a).seg == (b).seg)
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 
 #define __kernel_ok (uaccess_kernel())
 #define __user_ok(addr, size) \
diff --git a/include/asm-generic/uaccess.h b/include/asm-generic/uaccess.h
index e935318804f8ae..ba68ee4dabfaa7 100644
--- a/include/asm-generic/uaccess.h
+++ b/include/asm-generic/uaccess.h
@@ -86,8 +86,8 @@ static inline void set_fs(mm_segment_t fs)
 }
 #endif
 
-#ifndef segment_eq
-#define segment_eq(a, b) ((a).seg == (b).seg)
+#ifndef uaccess_kernel
+#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
 #endif
 
 #define access_ok(addr, size) __access_ok((unsigned long)(addr),(size))
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 0a76ddc07d5970..5c62d0c6f15b16 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -6,8 +6,6 @@
 #include <linux/sched.h>
 #include <linux/thread_info.h>
 
-#define uaccess_kernel() segment_eq(get_fs(), KERNEL_DS)
-
 #include <asm/uaccess.h>
 
 /*
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers
  2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-07-10 13:57 ` [PATCH 4/6] uaccess: remove segment_eq Christoph Hellwig
@ 2020-07-10 13:57 ` Christoph Hellwig
  2020-07-13  9:12   ` Geert Uytterhoeven
                     ` (3 more replies)
  2020-07-10 13:57 ` [PATCH 6/6] exec: use force_uaccess_begin during exec and exit Christoph Hellwig
  2020-07-10 15:25 ` clean up address limit helpers Linus Torvalds
  6 siblings, 4 replies; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-10 13:57 UTC (permalink / raw)
  To: Nick Hu, Greentime Hu, Vincent Chen, Paul Walmsley,
	Palmer Dabbelt, Andrew Morton, Linus Torvalds
  Cc: linux-arch, linux-riscv, linux-kernel

Add helpers to wraper the get_fs/set_fs magic for undoing any damange
done by set_fs(KERNEL_DS).  There is no real functional benefit, but this
documents the intent of these calls better, and will allow stubbing the
functions out easily for kernels builds that do not allow address space
overrides in the future.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/kernel/sdei.c         |  2 +-
 arch/m68k/include/asm/tlbflush.h | 12 ++++++------
 arch/mips/kernel/unaligned.c     | 27 +++++++++++++--------------
 arch/nds32/mm/alignment.c        |  7 +++----
 arch/sh/kernel/traps_32.c        | 18 ++++++++----------
 drivers/firmware/arm_sdei.c      |  5 ++---
 include/linux/uaccess.h          | 18 ++++++++++++++++++
 kernel/events/callchain.c        |  5 ++---
 kernel/events/core.c             |  5 ++---
 kernel/kthread.c                 |  5 ++---
 kernel/stacktrace.c              |  5 ++---
 mm/maccess.c                     | 22 ++++++++++------------
 12 files changed, 69 insertions(+), 62 deletions(-)

diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index dab88260b13739..7689f2031c0c41 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -180,7 +180,7 @@ static __kprobes unsigned long _sdei_handler(struct pt_regs *regs,
 
 	/*
 	 * We didn't take an exception to get here, set PAN. UAO will be cleared
-	 * by sdei_event_handler()s set_fs(USER_DS) call.
+	 * by sdei_event_handler()s force_uaccess_begin() call.
 	 */
 	__uaccess_enable_hw_pan();
 
diff --git a/arch/m68k/include/asm/tlbflush.h b/arch/m68k/include/asm/tlbflush.h
index 191e75a6bb249e..30471549e1e224 100644
--- a/arch/m68k/include/asm/tlbflush.h
+++ b/arch/m68k/include/asm/tlbflush.h
@@ -13,13 +13,13 @@ static inline void flush_tlb_kernel_page(void *addr)
 	if (CPU_IS_COLDFIRE) {
 		mmu_write(MMUOR, MMUOR_CNL);
 	} else if (CPU_IS_040_OR_060) {
-		mm_segment_t old_fs = get_fs();
-		set_fs(KERNEL_DS);
+		mm_segment_t old_fs = force_uaccess_begin();
+
 		__asm__ __volatile__(".chip 68040\n\t"
 				     "pflush (%0)\n\t"
 				     ".chip 68k"
 				     : : "a" (addr));
-		set_fs(old_fs);
+		force_uaccess_end(old_fs);
 	} else if (CPU_IS_020_OR_030)
 		__asm__ __volatile__("pflush #4,#4,(%0)" : : "a" (addr));
 }
@@ -85,10 +85,10 @@ static inline void flush_tlb_mm(struct mm_struct *mm)
 static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long addr)
 {
 	if (vma->vm_mm == current->active_mm) {
-		mm_segment_t old_fs = get_fs();
-		set_fs(USER_DS);
+		mm_segment_t old_fs = force_uaccess_begin();
+
 		__flush_tlb_one(addr);
-		set_fs(old_fs);
+		force_uaccess_end(old_fs);
 	}
 }
 
diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c
index 0adce604fa44cb..126a5f3f4e4ce3 100644
--- a/arch/mips/kernel/unaligned.c
+++ b/arch/mips/kernel/unaligned.c
@@ -191,17 +191,16 @@ static void emulate_load_store_insn(struct pt_regs *regs,
 			 * memory, so we need to "switch" the address limit to
 			 * user space, so that address check can work properly.
 			 */
-			seg = get_fs();
-			set_fs(USER_DS);
+			seg = force_uaccess_begin();
 			switch (insn.spec3_format.func) {
 			case lhe_op:
 				if (!access_ok(addr, 2)) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto sigbus;
 				}
 				LoadHWE(addr, value, res);
 				if (res) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto fault;
 				}
 				compute_return_epc(regs);
@@ -209,12 +208,12 @@ static void emulate_load_store_insn(struct pt_regs *regs,
 				break;
 			case lwe_op:
 				if (!access_ok(addr, 4)) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto sigbus;
 				}
 				LoadWE(addr, value, res);
 				if (res) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto fault;
 				}
 				compute_return_epc(regs);
@@ -222,12 +221,12 @@ static void emulate_load_store_insn(struct pt_regs *regs,
 				break;
 			case lhue_op:
 				if (!access_ok(addr, 2)) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto sigbus;
 				}
 				LoadHWUE(addr, value, res);
 				if (res) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto fault;
 				}
 				compute_return_epc(regs);
@@ -235,35 +234,35 @@ static void emulate_load_store_insn(struct pt_regs *regs,
 				break;
 			case she_op:
 				if (!access_ok(addr, 2)) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto sigbus;
 				}
 				compute_return_epc(regs);
 				value = regs->regs[insn.spec3_format.rt];
 				StoreHWE(addr, value, res);
 				if (res) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto fault;
 				}
 				break;
 			case swe_op:
 				if (!access_ok(addr, 4)) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto sigbus;
 				}
 				compute_return_epc(regs);
 				value = regs->regs[insn.spec3_format.rt];
 				StoreWE(addr, value, res);
 				if (res) {
-					set_fs(seg);
+					force_uaccess_end(seg);
 					goto fault;
 				}
 				break;
 			default:
-				set_fs(seg);
+				force_uaccess_end(seg);
 				goto sigill;
 			}
-			set_fs(seg);
+			force_uaccess_end(seg);
 		}
 #endif
 		break;
diff --git a/arch/nds32/mm/alignment.c b/arch/nds32/mm/alignment.c
index c8b9061a2ee3d5..1eb7ded6992b57 100644
--- a/arch/nds32/mm/alignment.c
+++ b/arch/nds32/mm/alignment.c
@@ -512,7 +512,7 @@ int do_unaligned_access(unsigned long addr, struct pt_regs *regs)
 {
 	unsigned long inst;
 	int ret = -EFAULT;
-	mm_segment_t seg = get_fs();
+	mm_segment_t seg;
 
 	inst = get_inst(regs->ipc);
 
@@ -520,13 +520,12 @@ int do_unaligned_access(unsigned long addr, struct pt_regs *regs)
 	      "Faulting addr: 0x%08lx, pc: 0x%08lx [inst: 0x%08lx ]\n", addr,
 	      regs->ipc, inst);
 
-	set_fs(USER_DS);
-
+	seg = force_uaccess_begin();
 	if (inst & NDS32_16BIT_INSTRUCTION)
 		ret = do_16((inst >> 16) & 0xffff, regs);
 	else
 		ret = do_32(inst, regs);
-	set_fs(seg);
+	force_uaccess_end(seg);
 
 	return ret;
 }
diff --git a/arch/sh/kernel/traps_32.c b/arch/sh/kernel/traps_32.c
index 058c6181bb306c..950f05d9d68338 100644
--- a/arch/sh/kernel/traps_32.c
+++ b/arch/sh/kernel/traps_32.c
@@ -482,8 +482,6 @@ asmlinkage void do_address_error(struct pt_regs *regs,
 	error_code = lookup_exception_vector();
 #endif
 
-	oldfs = get_fs();
-
 	if (user_mode(regs)) {
 		int si_code = BUS_ADRERR;
 		unsigned int user_action;
@@ -491,13 +489,13 @@ asmlinkage void do_address_error(struct pt_regs *regs,
 		local_irq_enable();
 		inc_unaligned_user_access();
 
-		set_fs(USER_DS);
+		oldfs = force_uaccess_begin();
 		if (copy_from_user(&instruction, (insn_size_t *)(regs->pc & ~1),
 				   sizeof(instruction))) {
-			set_fs(oldfs);
+			force_uaccess_end(oldfs);
 			goto uspace_segv;
 		}
-		set_fs(oldfs);
+		force_uaccess_end(oldfs);
 
 		/* shout about userspace fixups */
 		unaligned_fixups_notify(current, instruction, regs);
@@ -520,11 +518,11 @@ asmlinkage void do_address_error(struct pt_regs *regs,
 			goto uspace_segv;
 		}
 
-		set_fs(USER_DS);
+		oldfs = force_uaccess_begin();
 		tmp = handle_unaligned_access(instruction, regs,
 					      &user_mem_access, 0,
 					      address);
-		set_fs(oldfs);
+		force_uaccess_end(oldfs);
 
 		if (tmp == 0)
 			return; /* sorted */
@@ -540,13 +538,13 @@ asmlinkage void do_address_error(struct pt_regs *regs,
 		if (regs->pc & 1)
 			die("unaligned program counter", regs, error_code);
 
-		set_fs(KERNEL_DS);
+		oldfs = force_uaccess_begin();
 		if (copy_from_user(&instruction, (void __user *)(regs->pc),
 				   sizeof(instruction))) {
 			/* Argh. Fault on the instruction itself.
 			   This should never happen non-SMP
 			*/
-			set_fs(oldfs);
+			force_uaccess_end(oldfs);
 			die("insn faulting in do_address_error", regs, 0);
 		}
 
@@ -554,7 +552,7 @@ asmlinkage void do_address_error(struct pt_regs *regs,
 
 		handle_unaligned_access(instruction, regs, &user_mem_access,
 					0, address);
-		set_fs(oldfs);
+		force_uaccess_end(oldfs);
 	}
 }
 
diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
index e7e36aab2386ff..b4b9ce97f415e3 100644
--- a/drivers/firmware/arm_sdei.c
+++ b/drivers/firmware/arm_sdei.c
@@ -1136,15 +1136,14 @@ int sdei_event_handler(struct pt_regs *regs,
 	 * access kernel memory.
 	 * Do the same here because this doesn't come via the same entry code.
 	*/
-	orig_addr_limit = get_fs();
-	set_fs(USER_DS);
+	orig_addr_limit = force_uaccess_begin();
 
 	err = arg->callback(event_num, regs, arg->callback_arg);
 	if (err)
 		pr_err_ratelimited("event %u on CPU %u failed with error: %d\n",
 				   event_num, smp_processor_id(), err);
 
-	set_fs(orig_addr_limit);
+	force_uaccess_end(orig_addr_limit);
 
 	return err;
 }
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 5c62d0c6f15b16..94b28541165929 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -8,6 +8,24 @@
 
 #include <asm/uaccess.h>
 
+/*
+ * Force the uaccess routines to be wired up for actual userspace access,
+ * overriding any possible set_fs(KERNEL_DS) still lingering around.  Undone
+ * using force_uaccess_end below.
+ */
+static inline mm_segment_t force_uaccess_begin(void)
+{
+	mm_segment_t fs = get_fs();
+
+	set_fs(USER_DS);
+	return fs;
+}
+
+static inline void force_uaccess_end(mm_segment_t oldfs)
+{
+	set_fs(oldfs);
+}
+
 /*
  * Architectures should provide two primitives (raw_copy_{to,from}_user())
  * and get rid of their private instances of copy_{to,from}_user() and
diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c
index 334d48b16c36d7..dddc773e99f7b5 100644
--- a/kernel/events/callchain.c
+++ b/kernel/events/callchain.c
@@ -218,10 +218,9 @@ get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user,
 			if (add_mark)
 				perf_callchain_store_context(&ctx, PERF_CONTEXT_USER);
 
-			fs = get_fs();
-			set_fs(USER_DS);
+			fs = force_uaccess_begin();
 			perf_callchain_user(&ctx, regs);
-			set_fs(fs);
+			force_uaccess_end(fs);
 		}
 	}
 
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 856d98c36f562d..c0c6f462a393e1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6436,10 +6436,9 @@ perf_output_sample_ustack(struct perf_output_handle *handle, u64 dump_size,
 
 		/* Data. */
 		sp = perf_user_stack_pointer(regs);
-		fs = get_fs();
-		set_fs(USER_DS);
+		fs = force_uaccess_begin();
 		rem = __output_copy_user(handle, (void *) sp, dump_size);
-		set_fs(fs);
+		force_uaccess_end(fs);
 		dyn_size = dump_size - rem;
 
 		perf_output_skip(handle, rem);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 132f84a5fde3f0..379bf1e543371a 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -1254,8 +1254,7 @@ void kthread_use_mm(struct mm_struct *mm)
 	if (active_mm != mm)
 		mmdrop(active_mm);
 
-	to_kthread(tsk)->oldfs = get_fs();
-	set_fs(USER_DS);
+	to_kthread(tsk)->oldfs = force_uaccess_begin();
 }
 EXPORT_SYMBOL_GPL(kthread_use_mm);
 
@@ -1270,7 +1269,7 @@ void kthread_unuse_mm(struct mm_struct *mm)
 	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
 	WARN_ON_ONCE(!tsk->mm);
 
-	set_fs(to_kthread(tsk)->oldfs);
+	force_uaccess_end(to_kthread(tsk)->oldfs);
 
 	task_lock(tsk);
 	sync_mm_rss(mm);
diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
index 2af66e449aa6a8..946f44a9e86afb 100644
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -233,10 +233,9 @@ unsigned int stack_trace_save_user(unsigned long *store, unsigned int size)
 	if (current->flags & PF_KTHREAD)
 		return 0;
 
-	fs = get_fs();
-	set_fs(USER_DS);
+	fs = force_uaccess_begin();
 	arch_stack_walk_user(consume_entry, &c, task_pt_regs(current));
-	set_fs(fs);
+	force_uaccess_end(fs);
 
 	return c.len;
 }
diff --git a/mm/maccess.c b/mm/maccess.c
index f98ff91e32c6df..3bd70405f2d848 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -205,15 +205,14 @@ long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
 long copy_from_user_nofault(void *dst, const void __user *src, size_t size)
 {
 	long ret = -EFAULT;
-	mm_segment_t old_fs = get_fs();
+	mm_segment_t old_fs = force_uaccess_begin();
 
-	set_fs(USER_DS);
 	if (access_ok(src, size)) {
 		pagefault_disable();
 		ret = __copy_from_user_inatomic(dst, src, size);
 		pagefault_enable();
 	}
-	set_fs(old_fs);
+	force_uaccess_end(old_fs);
 
 	if (ret)
 		return -EFAULT;
@@ -233,15 +232,14 @@ EXPORT_SYMBOL_GPL(copy_from_user_nofault);
 long copy_to_user_nofault(void __user *dst, const void *src, size_t size)
 {
 	long ret = -EFAULT;
-	mm_segment_t old_fs = get_fs();
+	mm_segment_t old_fs = force_uaccess_begin();
 
-	set_fs(USER_DS);
 	if (access_ok(dst, size)) {
 		pagefault_disable();
 		ret = __copy_to_user_inatomic(dst, src, size);
 		pagefault_enable();
 	}
-	set_fs(old_fs);
+	force_uaccess_end(old_fs);
 
 	if (ret)
 		return -EFAULT;
@@ -270,17 +268,17 @@ EXPORT_SYMBOL_GPL(copy_to_user_nofault);
 long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
 			      long count)
 {
-	mm_segment_t old_fs = get_fs();
+	mm_segment_t old_fs;
 	long ret;
 
 	if (unlikely(count <= 0))
 		return 0;
 
-	set_fs(USER_DS);
+	old_fs = force_uaccess_begin();
 	pagefault_disable();
 	ret = strncpy_from_user(dst, unsafe_addr, count);
 	pagefault_enable();
-	set_fs(old_fs);
+	force_uaccess_end(old_fs);
 
 	if (ret >= count) {
 		ret = count;
@@ -310,14 +308,14 @@ long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
  */
 long strnlen_user_nofault(const void __user *unsafe_addr, long count)
 {
-	mm_segment_t old_fs = get_fs();
+	mm_segment_t old_fs;
 	int ret;
 
-	set_fs(USER_DS);
+	old_fs = force_uaccess_begin();
 	pagefault_disable();
 	ret = strnlen_user(unsafe_addr, count);
 	pagefault_enable();
-	set_fs(old_fs);
+	force_uaccess_end(old_fs);
 
 	return ret;
 }
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/6] exec: use force_uaccess_begin during exec and exit
  2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
                   ` (4 preceding siblings ...)
  2020-07-10 13:57 ` [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers Christoph Hellwig
@ 2020-07-10 13:57 ` Christoph Hellwig
  2020-07-10 15:25 ` clean up address limit helpers Linus Torvalds
  6 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-10 13:57 UTC (permalink / raw)
  To: Nick Hu, Greentime Hu, Vincent Chen, Paul Walmsley,
	Palmer Dabbelt, Andrew Morton, Linus Torvalds
  Cc: linux-arch, linux-riscv, linux-kernel

Both exec and exit want to ensure that the uaccess routines actually do
access user pointers.  Use the newly added force_uaccess_begin helper
instead of an open coded set_fs for that to prepare for kernel builds
where set_fs() does not exist.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/exec.c     | 7 ++++++-
 kernel/exit.c | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index e6e8a9a7032784..769af470b69124 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1380,7 +1380,12 @@ int begin_new_exec(struct linux_binprm * bprm)
 	if (retval)
 		goto out_unlock;
 
-	set_fs(USER_DS);
+	/*
+	 * Ensure that the uaccess routines can actually operate on userspace
+	 * pointers:
+	 */
+	force_uaccess_begin();
+
 	me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD |
 					PF_NOFREEZE | PF_NO_SETAFFINITY);
 	flush_thread();
diff --git a/kernel/exit.c b/kernel/exit.c
index 727150f2810338..17d486a20f0dc6 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -731,7 +731,7 @@ void __noreturn do_exit(long code)
 	 * mm_release()->clear_child_tid() from writing to a user-controlled
 	 * kernel address.
 	 */
-	set_fs(USER_DS);
+	force_uaccess_begin();
 
 	if (unlikely(in_atomic())) {
 		pr_info("note: %s[%d] exited with preempt_count %d\n",
-- 
2.26.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: clean up address limit helpers
  2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
                   ` (5 preceding siblings ...)
  2020-07-10 13:57 ` [PATCH 6/6] exec: use force_uaccess_begin during exec and exit Christoph Hellwig
@ 2020-07-10 15:25 ` Linus Torvalds
  2020-07-14  7:09   ` Christoph Hellwig
  6 siblings, 1 reply; 20+ messages in thread
From: Linus Torvalds @ 2020-07-10 15:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Nick Hu, Linux Kernel Mailing List, Palmer Dabbelt,
	Greentime Hu, Paul Walmsley, Andrew Morton, Vincent Chen,
	linux-riscv

On Fri, Jul 10, 2020 at 6:57 AM Christoph Hellwig <hch@lst.de> wrote:
>
> in preparation for eventually phasing out direct use of set_fs(), this
> series removes the segment_eq() arch helper that is only used to
> implement or duplicate the uaccess_kernel() API, and then adds
> descriptive helpers to force the kernel address limit.

Ack. All the patches looked like no-ops to me, but with better naming
and clarity.

           Linus

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers
  2020-07-10 13:57 ` [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers Christoph Hellwig
@ 2020-07-13  9:12   ` Geert Uytterhoeven
  2020-07-13 10:06   ` Greentime Hu
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2020-07-13  9:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linux-Arch, Nick Hu, Linux Kernel Mailing List, Palmer Dabbelt,
	Greentime Hu, Paul Walmsley, Andrew Morton, Vincent Chen,
	Linus Torvalds, linux-riscv

On Fri, Jul 10, 2020 at 3:57 PM Christoph Hellwig <hch@lst.de> wrote:
> Add helpers to wraper the get_fs/set_fs magic for undoing any damage

to wrap

> done by set_fs(KERNEL_DS).  There is no real functional benefit, but this
> documents the intent of these calls better, and will allow stubbing the
> functions out easily for kernels builds that do not allow address space
> overrides in the future.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

>  arch/m68k/include/asm/tlbflush.h | 12 ++++++------

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] uaccess: remove segment_eq
  2020-07-10 13:57 ` [PATCH 4/6] uaccess: remove segment_eq Christoph Hellwig
@ 2020-07-13  9:16   ` Geert Uytterhoeven
  2020-07-13 10:05   ` Greentime Hu
  1 sibling, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2020-07-13  9:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Linux-Arch, Nick Hu, Linux Kernel Mailing List, Palmer Dabbelt,
	Greentime Hu, Paul Walmsley, Andrew Morton, Vincent Chen,
	Linus Torvalds, linux-riscv

On Fri, Jul 10, 2020 at 3:58 PM Christoph Hellwig <hch@lst.de> wrote:
> segment_eq is only used to implement uaccess_kernel.  Just open code
> uaccess_kernel in the arch uaccess headers and remove one layer of
> indirection.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

>  arch/m68k/include/asm/segment.h       | 2 +-

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/6] nds32: use uaccess_kernel in show_regs
  2020-07-10 13:57 ` [PATCH 2/6] nds32: use uaccess_kernel in show_regs Christoph Hellwig
@ 2020-07-13 10:02   ` Greentime Hu
  0 siblings, 0 replies; 20+ messages in thread
From: Greentime Hu @ 2020-07-13 10:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Nick Hu, Linux Kernel Mailing List, Palmer Dabbelt,
	Paul Walmsley, Andrew Morton, Vincent Chen, Linus Torvalds,
	linux-riscv

Christoph Hellwig <hch@lst.de> 於 2020年7月10日 週五 下午9:57寫道:
>
> Use the uaccess_kernel helper instead of duplicating it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/nds32/kernel/process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/nds32/kernel/process.c b/arch/nds32/kernel/process.c
> index 9712fd474f2ca3..f06265949ec28b 100644
> --- a/arch/nds32/kernel/process.c
> +++ b/arch/nds32/kernel/process.c
> @@ -121,7 +121,7 @@ void show_regs(struct pt_regs *regs)
>                 regs->uregs[3], regs->uregs[2], regs->uregs[1], regs->uregs[0]);
>         pr_info("  IRQs o%s  Segment %s\n",
>                 interrupts_enabled(regs) ? "n" : "ff",
> -               segment_eq(get_fs(), KERNEL_DS)? "kernel" : "user");
> +               uaccess_kernel() ? "kernel" : "user");
>  }
>
>  EXPORT_SYMBOL(show_regs);

Hi Christoph, Thank you.
Acked-by: Greentime Hu <green.hu@gmail.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] uaccess: remove segment_eq
  2020-07-10 13:57 ` [PATCH 4/6] uaccess: remove segment_eq Christoph Hellwig
  2020-07-13  9:16   ` Geert Uytterhoeven
@ 2020-07-13 10:05   ` Greentime Hu
  1 sibling, 0 replies; 20+ messages in thread
From: Greentime Hu @ 2020-07-13 10:05 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Nick Hu, Linux Kernel Mailing List, Palmer Dabbelt,
	Paul Walmsley, Andrew Morton, Vincent Chen, Linus Torvalds,
	linux-riscv

Christoph Hellwig <hch@lst.de> 於 2020年7月10日 週五 下午9:57寫道:
>
> segment_eq is only used to implement uaccess_kernel.  Just open code
> uaccess_kernel in the arch uaccess headers and remove one layer of
> indirection.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/alpha/include/asm/uaccess.h      | 2 +-
>  arch/arc/include/asm/segment.h        | 3 +--
>  arch/arm/include/asm/uaccess.h        | 4 ++--
>  arch/arm64/include/asm/uaccess.h      | 2 +-
>  arch/csky/include/asm/segment.h       | 2 +-
>  arch/h8300/include/asm/segment.h      | 2 +-
>  arch/ia64/include/asm/uaccess.h       | 2 +-
>  arch/m68k/include/asm/segment.h       | 2 +-
>  arch/microblaze/include/asm/uaccess.h | 2 +-
>  arch/mips/include/asm/uaccess.h       | 2 +-
>  arch/nds32/include/asm/uaccess.h      | 2 +-
>  arch/nios2/include/asm/uaccess.h      | 2 +-
>  arch/openrisc/include/asm/uaccess.h   | 2 +-
>  arch/parisc/include/asm/uaccess.h     | 2 +-
>  arch/powerpc/include/asm/uaccess.h    | 3 +--
>  arch/riscv/include/asm/uaccess.h      | 4 +---
>  arch/s390/include/asm/uaccess.h       | 2 +-
>  arch/sh/include/asm/segment.h         | 3 +--
>  arch/sparc/include/asm/uaccess_32.h   | 2 +-
>  arch/sparc/include/asm/uaccess_64.h   | 2 +-
>  arch/x86/include/asm/uaccess.h        | 2 +-
>  arch/xtensa/include/asm/uaccess.h     | 2 +-
>  include/asm-generic/uaccess.h         | 4 ++--
>  include/linux/uaccess.h               | 2 --
>  24 files changed, 25 insertions(+), 32 deletions(-)
>
[...]
> diff --git a/arch/nds32/include/asm/uaccess.h b/arch/nds32/include/asm/uaccess.h
> index 3a9219f53ee0d8..010ba5f1d7dd6b 100644
> --- a/arch/nds32/include/asm/uaccess.h
> +++ b/arch/nds32/include/asm/uaccess.h
> @@ -44,7 +44,7 @@ static inline void set_fs(mm_segment_t fs)
>         current_thread_info()->addr_limit = fs;
>  }
>
> -#define segment_eq(a, b)       ((a) == (b))
> +#define uaccess_kernel()       (get_fs() == KERNEL_DS)
>
>  #define __range_ok(addr, size) (size <= get_fs() && addr <= (get_fs() -size))
>
Hi Christoph, Thank you.
Acked-by: Greentime Hu <green.hu@gmail.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers
  2020-07-10 13:57 ` [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers Christoph Hellwig
  2020-07-13  9:12   ` Geert Uytterhoeven
@ 2020-07-13 10:06   ` Greentime Hu
  2020-07-13 12:21   ` Mark Rutland
  2020-07-13 12:26   ` Mark Rutland
  3 siblings, 0 replies; 20+ messages in thread
From: Greentime Hu @ 2020-07-13 10:06 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Nick Hu, Linux Kernel Mailing List, Palmer Dabbelt,
	Paul Walmsley, Andrew Morton, Vincent Chen, Linus Torvalds,
	linux-riscv

Christoph Hellwig <hch@lst.de> 於 2020年7月10日 週五 下午9:57寫道:
>
> Add helpers to wraper the get_fs/set_fs magic for undoing any damange
> done by set_fs(KERNEL_DS).  There is no real functional benefit, but this
> documents the intent of these calls better, and will allow stubbing the
> functions out easily for kernels builds that do not allow address space
> overrides in the future.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/arm64/kernel/sdei.c         |  2 +-
>  arch/m68k/include/asm/tlbflush.h | 12 ++++++------
>  arch/mips/kernel/unaligned.c     | 27 +++++++++++++--------------
>  arch/nds32/mm/alignment.c        |  7 +++----
>  arch/sh/kernel/traps_32.c        | 18 ++++++++----------
>  drivers/firmware/arm_sdei.c      |  5 ++---
>  include/linux/uaccess.h          | 18 ++++++++++++++++++
>  kernel/events/callchain.c        |  5 ++---
>  kernel/events/core.c             |  5 ++---
>  kernel/kthread.c                 |  5 ++---
>  kernel/stacktrace.c              |  5 ++---
>  mm/maccess.c                     | 22 ++++++++++------------
>  12 files changed, 69 insertions(+), 62 deletions(-)
>
[...]
> diff --git a/arch/nds32/mm/alignment.c b/arch/nds32/mm/alignment.c
> index c8b9061a2ee3d5..1eb7ded6992b57 100644
> --- a/arch/nds32/mm/alignment.c
> +++ b/arch/nds32/mm/alignment.c
> @@ -512,7 +512,7 @@ int do_unaligned_access(unsigned long addr, struct pt_regs *regs)
>  {
>         unsigned long inst;
>         int ret = -EFAULT;
> -       mm_segment_t seg = get_fs();
> +       mm_segment_t seg;
>
>         inst = get_inst(regs->ipc);
>
> @@ -520,13 +520,12 @@ int do_unaligned_access(unsigned long addr, struct pt_regs *regs)
>               "Faulting addr: 0x%08lx, pc: 0x%08lx [inst: 0x%08lx ]\n", addr,
>               regs->ipc, inst);
>
> -       set_fs(USER_DS);
> -
> +       seg = force_uaccess_begin();
>         if (inst & NDS32_16BIT_INSTRUCTION)
>                 ret = do_16((inst >> 16) & 0xffff, regs);
>         else
>                 ret = do_32(inst, regs);
> -       set_fs(seg);
> +       force_uaccess_end(seg);
>
>         return ret;
>  }

Hi Christoph, Thank you.
Acked-by: Greentime Hu <green.hu@gmail.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers
  2020-07-10 13:57 ` [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers Christoph Hellwig
  2020-07-13  9:12   ` Geert Uytterhoeven
  2020-07-13 10:06   ` Greentime Hu
@ 2020-07-13 12:21   ` Mark Rutland
  2020-07-13 13:19     ` Geert Uytterhoeven
  2020-07-13 12:26   ` Mark Rutland
  3 siblings, 1 reply; 20+ messages in thread
From: Mark Rutland @ 2020-07-13 12:21 UTC (permalink / raw)
  To: Christoph Hellwig, Geert Uytterhoeven
  Cc: linux-arch, Nick Hu, linux-kernel, Palmer Dabbelt, Greentime Hu,
	Paul Walmsley, Andrew Morton, Vincent Chen, Linus Torvalds,
	linux-riscv

On Fri, Jul 10, 2020 at 03:57:05PM +0200, Christoph Hellwig wrote:
> Add helpers to wraper the get_fs/set_fs magic for undoing any damange
> done by set_fs(KERNEL_DS).  There is no real functional benefit, but this
> documents the intent of these calls better, and will allow stubbing the
> functions out easily for kernels builds that do not allow address space
> overrides in the future.

> diff --git a/arch/m68k/include/asm/tlbflush.h b/arch/m68k/include/asm/tlbflush.h
> index 191e75a6bb249e..30471549e1e224 100644
> --- a/arch/m68k/include/asm/tlbflush.h
> +++ b/arch/m68k/include/asm/tlbflush.h
> @@ -13,13 +13,13 @@ static inline void flush_tlb_kernel_page(void *addr)
>  	if (CPU_IS_COLDFIRE) {
>  		mmu_write(MMUOR, MMUOR_CNL);
>  	} else if (CPU_IS_040_OR_060) {
> -		mm_segment_t old_fs = get_fs();
> -		set_fs(KERNEL_DS);
> +		mm_segment_t old_fs = force_uaccess_begin();
> +

This used to set KERNEL_DS, and now it sets USER_DS, which looks wrong
superficially.

If the new behaviour is fine it suggests that the old behaviour was
wrong, or that this is superfluous and could go entirely.

Geert?

Mark.

>  		__asm__ __volatile__(".chip 68040\n\t"
>  				     "pflush (%0)\n\t"
>  				     ".chip 68k"
>  				     : : "a" (addr));
> -		set_fs(old_fs);
> +		force_uaccess_end(old_fs);
>  	} else if (CPU_IS_020_OR_030)
>  		__asm__ __volatile__("pflush #4,#4,(%0)" : : "a" (addr));

> +/*
> + * Force the uaccess routines to be wired up for actual userspace access,
> + * overriding any possible set_fs(KERNEL_DS) still lingering around.  Undone
> + * using force_uaccess_end below.
> + */
> +static inline mm_segment_t force_uaccess_begin(void)
> +{
> +	mm_segment_t fs = get_fs();
> +
> +	set_fs(USER_DS);
> +	return fs;
> +}
> +
> +static inline void force_uaccess_end(mm_segment_t oldfs)
> +{
> +	set_fs(oldfs);
> +}

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers
  2020-07-10 13:57 ` [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers Christoph Hellwig
                     ` (2 preceding siblings ...)
  2020-07-13 12:21   ` Mark Rutland
@ 2020-07-13 12:26   ` Mark Rutland
  3 siblings, 0 replies; 20+ messages in thread
From: Mark Rutland @ 2020-07-13 12:26 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Nick Hu, linux-kernel, Palmer Dabbelt, Greentime Hu,
	Paul Walmsley, Andrew Morton, Vincent Chen, Linus Torvalds,
	linux-riscv

On Fri, Jul 10, 2020 at 03:57:05PM +0200, Christoph Hellwig wrote:
> Add helpers to wraper the get_fs/set_fs magic for undoing any damange
> done by set_fs(KERNEL_DS).  There is no real functional benefit, but this
> documents the intent of these calls better, and will allow stubbing the
> functions out easily for kernels builds that do not allow address space
> overrides in the future.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/arm64/kernel/sdei.c         |  2 +-
>  arch/m68k/include/asm/tlbflush.h | 12 ++++++------
>  arch/mips/kernel/unaligned.c     | 27 +++++++++++++--------------
>  arch/nds32/mm/alignment.c        |  7 +++----
>  arch/sh/kernel/traps_32.c        | 18 ++++++++----------
>  drivers/firmware/arm_sdei.c      |  5 ++---
>  include/linux/uaccess.h          | 18 ++++++++++++++++++
>  kernel/events/callchain.c        |  5 ++---
>  kernel/events/core.c             |  5 ++---
>  kernel/kthread.c                 |  5 ++---
>  kernel/stacktrace.c              |  5 ++---
>  mm/maccess.c                     | 22 ++++++++++------------
>  12 files changed, 69 insertions(+), 62 deletions(-)

The perf core and arm64/sdei bits look sound. FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers
  2020-07-13 12:21   ` Mark Rutland
@ 2020-07-13 13:19     ` Geert Uytterhoeven
  2020-07-14  7:12       ` Christoph Hellwig
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2020-07-13 13:19 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Linux-Arch, Nick Hu, Linux Kernel Mailing List, linux-riscv,
	Palmer Dabbelt, Greentime Hu, Paul Walmsley, Andrew Morton,
	Vincent Chen, Linus Torvalds, Christoph Hellwig

Hi Mark,

On Mon, Jul 13, 2020 at 2:21 PM Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Jul 10, 2020 at 03:57:05PM +0200, Christoph Hellwig wrote:
> > Add helpers to wraper the get_fs/set_fs magic for undoing any damange
> > done by set_fs(KERNEL_DS).  There is no real functional benefit, but this
> > documents the intent of these calls better, and will allow stubbing the
> > functions out easily for kernels builds that do not allow address space
> > overrides in the future.
>
> > diff --git a/arch/m68k/include/asm/tlbflush.h b/arch/m68k/include/asm/tlbflush.h
> > index 191e75a6bb249e..30471549e1e224 100644
> > --- a/arch/m68k/include/asm/tlbflush.h
> > +++ b/arch/m68k/include/asm/tlbflush.h
> > @@ -13,13 +13,13 @@ static inline void flush_tlb_kernel_page(void *addr)
> >       if (CPU_IS_COLDFIRE) {
> >               mmu_write(MMUOR, MMUOR_CNL);
> >       } else if (CPU_IS_040_OR_060) {
> > -             mm_segment_t old_fs = get_fs();
> > -             set_fs(KERNEL_DS);
> > +             mm_segment_t old_fs = force_uaccess_begin();
> > +
>
> This used to set KERNEL_DS, and now it sets USER_DS, which looks wrong
> superficially.

Thanks for noticing, and sorry for missing that myself.

The same issue is present for SuperH:

    -               set_fs(KERNEL_DS);
    +               oldfs = force_uaccess_begin();

So the patch description should be:

    "Add helpers to wraper the get_fs/set_fs magic for undoing any damage
     done by set_fs(USER_DS)."

and leave alone users setting KERNEL_DS?

> If the new behaviour is fine it suggests that the old behaviour was
> wrong, or that this is superfluous and could go entirely.
>
> Geert?

Nope, on m68k, TLB cache operations operate on the current address space.
Hence to flush a kernel TLB entry, you have to switch to KERNEL_DS first.

If we're guaranteed to be already using KERNEL_DS, I guess the
address space handling can be removed.  But can we be sure?


> >               __asm__ __volatile__(".chip 68040\n\t"
> >                                    "pflush (%0)\n\t"
> >                                    ".chip 68k"
> >                                    : : "a" (addr));
> > -             set_fs(old_fs);
> > +             force_uaccess_end(old_fs);
> >       } else if (CPU_IS_020_OR_030)
> >               __asm__ __volatile__("pflush #4,#4,(%0)" : : "a" (addr));
>
> > +/*
> > + * Force the uaccess routines to be wired up for actual userspace access,
> > + * overriding any possible set_fs(KERNEL_DS) still lingering around.  Undone
> > + * using force_uaccess_end below.
> > + */
> > +static inline mm_segment_t force_uaccess_begin(void)
> > +{
> > +     mm_segment_t fs = get_fs();
> > +
> > +     set_fs(USER_DS);
> > +     return fs;
> > +}
> > +
> > +static inline void force_uaccess_end(mm_segment_t oldfs)
> > +{
> > +     set_fs(oldfs);
> > +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: clean up address limit helpers
  2020-07-10 15:25 ` clean up address limit helpers Linus Torvalds
@ 2020-07-14  7:09   ` Christoph Hellwig
  2020-07-14 15:33     ` Linus Torvalds
  0 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-14  7:09 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch, Nick Hu, Linux Kernel Mailing List, linux-riscv,
	Palmer Dabbelt, Greentime Hu, Paul Walmsley, Andrew Morton,
	Vincent Chen, Christoph Hellwig

On Fri, Jul 10, 2020 at 08:25:35AM -0700, Linus Torvalds wrote:
> On Fri, Jul 10, 2020 at 6:57 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > in preparation for eventually phasing out direct use of set_fs(), this
> > series removes the segment_eq() arch helper that is only used to
> > implement or duplicate the uaccess_kernel() API, and then adds
> > descriptive helpers to force the kernel address limit.
> 
> Ack. All the patches looked like no-ops to me, but with better naming
> and clarity.

Is that a formal Acked-by?

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers
  2020-07-13 13:19     ` Geert Uytterhoeven
@ 2020-07-14  7:12       ` Christoph Hellwig
  0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2020-07-14  7:12 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Rutland, Linux-Arch, Nick Hu, Linux Kernel Mailing List,
	linux-riscv, Palmer Dabbelt, Greentime Hu, Paul Walmsley,
	Andrew Morton, Vincent Chen, Linus Torvalds, Christoph Hellwig

On Mon, Jul 13, 2020 at 03:19:42PM +0200, Geert Uytterhoeven wrote:
> > This used to set KERNEL_DS, and now it sets USER_DS, which looks wrong
> > superficially.
> 
> Thanks for noticing, and sorry for missing that myself.
> 
> The same issue is present for SuperH:
> 
>     -               set_fs(KERNEL_DS);
>     +               oldfs = force_uaccess_begin();
> 
> So the patch description should be:
> 
>     "Add helpers to wraper the get_fs/set_fs magic for undoing any damage
>      done by set_fs(USER_DS)."
> 
> and leave alone users setting KERNEL_DS?

Yes, this was broken.  Fixed for the next version.

> > If the new behaviour is fine it suggests that the old behaviour was
> > wrong, or that this is superfluous and could go entirely.
> >
> > Geert?
> 
> Nope, on m68k, TLB cache operations operate on the current address space.
> Hence to flush a kernel TLB entry, you have to switch to KERNEL_DS first.
> 
> If we're guaranteed to be already using KERNEL_DS, I guess the
> address space handling can be removed.  But can we be sure?

We can't be sure yet.  But with a lot of my pending work we should be
able to get there in the not too far future.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: clean up address limit helpers
  2020-07-14  7:09   ` Christoph Hellwig
@ 2020-07-14 15:33     ` Linus Torvalds
  0 siblings, 0 replies; 20+ messages in thread
From: Linus Torvalds @ 2020-07-14 15:33 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Nick Hu, Linux Kernel Mailing List, Palmer Dabbelt,
	Greentime Hu, Paul Walmsley, Andrew Morton, Vincent Chen,
	linux-riscv

On Tue, Jul 14, 2020 at 12:09 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Jul 10, 2020 at 08:25:35AM -0700, Linus Torvalds wrote:
> >
> > Ack. All the patches looked like no-ops to me, but with better naming
> > and clarity.
>
> Is that a formal Acked-by?

Yup, the patch series looks fine to me, and it looks like you fixed up
the things people noticed about off m68k behavior.

              Linus

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/6] riscv: include <asm/pgtable.h> in <asm/uaccess.h>
  2020-07-10 13:57 ` [PATCH 3/6] riscv: include <asm/pgtable.h> in <asm/uaccess.h> Christoph Hellwig
@ 2020-07-21 23:02   ` Palmer Dabbelt
  0 siblings, 0 replies; 20+ messages in thread
From: Palmer Dabbelt @ 2020-07-21 23:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, nickhu, linux-kernel, green.hu, Paul Walmsley, akpm,
	deanbo422, Linus Torvalds, linux-riscv

On Fri, 10 Jul 2020 06:57:03 PDT (-0700), Christoph Hellwig wrote:
> To ensure TASK_SIZE is defined for USER_DS.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/include/asm/uaccess.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> index 8ce9d607b53dce..22de922d6ecb2f 100644
> --- a/arch/riscv/include/asm/uaccess.h
> +++ b/arch/riscv/include/asm/uaccess.h
> @@ -8,6 +8,8 @@
>  #ifndef _ASM_RISCV_UACCESS_H
>  #define _ASM_RISCV_UACCESS_H
>
> +#include <asm/pgtable.h>		/* for TASK_SIZE */
> +
>  /*
>   * User space memory access functions
>   */

Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-07-21 23:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-10 13:57 clean up address limit helpers Christoph Hellwig
2020-07-10 13:57 ` [PATCH 1/6] syscalls: use uaccess_kernel in addr_limit_user_check Christoph Hellwig
2020-07-10 13:57 ` [PATCH 2/6] nds32: use uaccess_kernel in show_regs Christoph Hellwig
2020-07-13 10:02   ` Greentime Hu
2020-07-10 13:57 ` [PATCH 3/6] riscv: include <asm/pgtable.h> in <asm/uaccess.h> Christoph Hellwig
2020-07-21 23:02   ` Palmer Dabbelt
2020-07-10 13:57 ` [PATCH 4/6] uaccess: remove segment_eq Christoph Hellwig
2020-07-13  9:16   ` Geert Uytterhoeven
2020-07-13 10:05   ` Greentime Hu
2020-07-10 13:57 ` [PATCH 5/6] uaccess: add force_uaccess_{begin,end} helpers Christoph Hellwig
2020-07-13  9:12   ` Geert Uytterhoeven
2020-07-13 10:06   ` Greentime Hu
2020-07-13 12:21   ` Mark Rutland
2020-07-13 13:19     ` Geert Uytterhoeven
2020-07-14  7:12       ` Christoph Hellwig
2020-07-13 12:26   ` Mark Rutland
2020-07-10 13:57 ` [PATCH 6/6] exec: use force_uaccess_begin during exec and exit Christoph Hellwig
2020-07-10 15:25 ` clean up address limit helpers Linus Torvalds
2020-07-14  7:09   ` Christoph Hellwig
2020-07-14 15:33     ` Linus Torvalds

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).