[PATCH RFC] Fixes to Linus' 'remove set_fs patch'

[PATCH RFC] Fixes to Linus' 'remove set_fs patch'

[Michael Schmitz]

Linus' patch didn't compile out of the box. I'm a little
worried that I had to patch mm/maccess.c, but I didn't
apply Christoph's patches first (plain v5.13). 

This (on top of Linus' patch) compiles and boots for m68k/mmu
(tested on ARAnyM, ramdisk and disk image).

Link: https://lore.kernel.org/r/CAHk-=wgjWebav7K_F7WS7KiwOAYr8KktsZiaV+jYP5LU5RB3Sg@mail.gmail.com
CC: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
---
 arch/m68k/include/asm/thread_info.h |  2 +-
 arch/m68k/include/asm/tlbflush.h    |  4 ++--
 arch/m68k/include/asm/uaccess.h     |  3 ---
 arch/m68k/kernel/process.c          |  2 +-
 arch/m68k/kernel/traps.c            |  4 ++--
 arch/m68k/mm/cache.c                | 10 +++++-----
 arch/m68k/mm/motorola.c             |  2 +-
 include/linux/uaccess.h             |  3 ---
 mm/maccess.c                        | 15 ++++++---------
 9 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/arch/m68k/include/asm/thread_info.h b/arch/m68k/include/asm/thread_info.h
index d813fed..4753d52 100644
--- a/arch/m68k/include/asm/thread_info.h
+++ b/arch/m68k/include/asm/thread_info.h
@@ -37,7 +37,7 @@ struct thread_info {
 #define INIT_THREAD_INFO(tsk)			\
 {						\
 	.task		= &tsk,			\
-	.addr_limit	= KERNEL_DS,		\
+	.addr_limit	= MAKE_MM_SEG(SUPER_DATA),	\
 	.preempt_count	= INIT_PREEMPT_COUNT,	\
 }
 
diff --git a/arch/m68k/include/asm/tlbflush.h b/arch/m68k/include/asm/tlbflush.h
index 47d4c3b..c6a85d4 100644
--- a/arch/m68k/include/asm/tlbflush.h
+++ b/arch/m68k/include/asm/tlbflush.h
@@ -14,12 +14,12 @@ static inline void flush_tlb_kernel_page(void *addr)
 		mmu_write(MMUOR, MMUOR_CNL);
 	} else if (CPU_IS_040_OR_060) {
 		preempt_disable();
-		set_segment(SUPER_DATA);
+		set_segment(MAKE_MM_SEG(SUPER_DATA));
 		__asm__ __volatile__(".chip 68040\n\t"
 				     "pflush (%0)\n\t"
 				     ".chip 68k"
 				     : : "a" (addr));
-		set_segment(USER_DATA);
+		set_segment(MAKE_MM_SEG(USER_DATA));
 		preempt_enable();
 	} else if (CPU_IS_020_OR_030)
 		__asm__ __volatile__("pflush #4,#4,(%0)" : : "a" (addr));
diff --git a/arch/m68k/include/asm/uaccess.h b/arch/m68k/include/asm/uaccess.h
index f98208c..95f661e 100644
--- a/arch/m68k/include/asm/uaccess.h
+++ b/arch/m68k/include/asm/uaccess.h
@@ -380,9 +380,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 user_addr_max() \
-	(uaccess_kernel() ? ~0UL : TASK_SIZE)
-
 extern long strncpy_from_user(char *dst, const char __user *src, long count);
 extern __must_check long strnlen_user(const char __user *str, long n);
 
diff --git a/arch/m68k/kernel/process.c b/arch/m68k/kernel/process.c
index 8298574..f1c3e68 100644
--- a/arch/m68k/kernel/process.c
+++ b/arch/m68k/kernel/process.c
@@ -155,7 +155,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp, unsigned long arg,
 	 * Must save the current SFC/DFC value, NOT the value when
 	 * the parent was last descheduled - RGH  10-08-96
 	 */
-	p->thread.fs = USER_DATA.seg;
+	p->thread.fs = USER_DATA;
 
 	if (unlikely(p->flags & (PF_KTHREAD | PF_IO_WORKER))) {
 		/* kernel thread */
diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c
index 8586318..f74205a 100644
--- a/arch/m68k/kernel/traps.c
+++ b/arch/m68k/kernel/traps.c
@@ -192,7 +192,7 @@ static inline unsigned long probe040(int iswrite, unsigned long addr, int wbs)
 
 	asm volatile (".chip 68040; movec %%mmusr,%0; .chip 68k" : "=r" (mmusr));
 
-	set_segment(USER_DATA);
+	set_segment(MAKE_MM_SEG(USER_DATA));
 	preempt_enable();
 
 	return mmusr;
@@ -218,7 +218,7 @@ static inline int do_040writeback1(unsigned short wbs, unsigned long wba,
 		break;
 	}
 
-	set_segment(USER_DATA);
+	set_segment(MAKE_MM_SEG(USER_DATA));
 	preempt_enable();
 
 
diff --git a/arch/m68k/mm/cache.c b/arch/m68k/mm/cache.c
index 80e311aa..cb513f6 100644
--- a/arch/m68k/mm/cache.c
+++ b/arch/m68k/mm/cache.c
@@ -73,7 +73,7 @@ static unsigned long virt_to_phys_slow(unsigned long vaddr, mm_segment_t seg)
 
 /* Push n pages at kernel virtual address and clear the icache */
 /* RZ: use cpush %bc instead of cpush %dc, cinv %ic */
-static void do_flush_icache_user_range(unsigned long address, unsigned long endaddr, mm_segment_t seg)
+static void do_flush_icache_range(unsigned long address, unsigned long endaddr, mm_segment_t seg)
 {
 	if (CPU_IS_COLDFIRE) {
 		unsigned long start, end;
@@ -107,15 +107,15 @@ static void do_flush_icache_user_range(unsigned long address, unsigned long enda
 
 void flush_icache_user_range(unsigned long address, unsigned long endaddr)
 {
-	do_flush_icache_range(address, endaddr, USER_DATA);
+	do_flush_icache_range(address, endaddr, MAKE_MM_SEG(USER_DATA));
 }
 
 void flush_icache_range(unsigned long address, unsigned long endaddr)
 {
 	preempt_disable();
-	set_segment(SUPER_DATA);
-	do_flush_icache_range(address, endaddr, SUPER_DATA);
-	set_segment(USER_DATA);
+	set_segment(MAKE_MM_SEG(SUPER_DATA));
+	do_flush_icache_range(address, endaddr, MAKE_MM_SEG(SUPER_DATA));
+	set_segment(MAKE_MM_SEG(USER_DATA));
 	preempt_enable();
 }
 EXPORT_SYMBOL(flush_icache_range);
diff --git a/arch/m68k/mm/motorola.c b/arch/m68k/mm/motorola.c
index 7af7d5a..b0063f3 100644
--- a/arch/m68k/mm/motorola.c
+++ b/arch/m68k/mm/motorola.c
@@ -467,7 +467,7 @@ void __init paging_init(void)
 	/*
 	 * Set up SFC/DFC registers
 	 */
-	set_segment(USER_DATA);
+	set_segment(MAKE_MM_SEG(USER_DATA));
 
 #ifdef DEBUG
 	printk ("before free_area_init\n");
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index c05e903..ec43aff 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -29,9 +29,6 @@ static inline void force_uaccess_end(mm_segment_t oldfs)
 	set_fs(oldfs);
 }
 #else /* CONFIG_SET_FS */
-typedef struct {
-	/* empty dummy */
-} mm_segment_t;
 
 #ifndef TASK_SIZE_MAX
 #define TASK_SIZE_MAX			TASK_SIZE
diff --git a/mm/maccess.c b/mm/maccess.c
index 3bd7040..690e9bb 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -105,17 +105,16 @@ long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
 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);
+	set_segment(MAKE_MM_SEG(SUPER_DATA));
 	pagefault_disable();
 	ret = __copy_from_user_inatomic(dst, (__force const void __user *)src,
 			size);
 	pagefault_enable();
-	set_fs(old_fs);
+	set_segment(MAKE_MM_SEG(USER_DATA));
 
 	if (ret)
 		return -EFAULT;
@@ -135,13 +134,12 @@ EXPORT_SYMBOL_GPL(copy_from_kernel_nofault);
 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);
+	set_segment(MAKE_MM_SEG(SUPER_DATA));
 	pagefault_disable();
 	ret = __copy_to_user_inatomic((__force void __user *)dst, src, size);
 	pagefault_enable();
-	set_fs(old_fs);
+	set_segment(MAKE_MM_SEG(USER_DATA));
 
 	if (ret)
 		return -EFAULT;
@@ -169,7 +167,6 @@ long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
  */
 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;
 
@@ -178,7 +175,7 @@ long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
 	if (!copy_from_kernel_nofault_allowed(unsafe_addr, count))
 		return -ERANGE;
 
-	set_fs(KERNEL_DS);
+	set_segment(MAKE_MM_SEG(SUPER_DATA));
 	pagefault_disable();
 
 	do {
@@ -187,7 +184,7 @@ long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
 
 	dst[-1] = '\0';
 	pagefault_enable();
-	set_fs(old_fs);
+	set_segment(MAKE_MM_SEG(USER_DATA));
 
 	return ret ? -EFAULT : src - unsafe_addr;
 }
-- 
2.7.4

Re: [PATCH RFC] Fixes to Linus' 'remove set_fs patch'

[Linus Torvalds]

On Mon, Jul 5, 2021 at 4:20 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
>
> Linus' patch didn't compile out of the box. I'm a little
> worried that I had to patch mm/maccess.c,

Yeah, that's not the right way to do it.

See the other thread where Christoph rightly points out that the
problem is that m68k needs __{get,put}_kernel_nofault() and then just
enable CONFIG_HAVE_GET_KERNEL_NOFAULT.

__{get,put}_kernel_nofault() is basically the similar to
__get_user/__put_user, except for

 (a) it uses kernel addresses (so in m68k parlance, "move" rather than "moves")

 (b) it uses the "goto err_label" approach instead of being an
expression that returns an error value

That (b) is what allows the use of "asm goto" to implement the
exception case much more efficiently (but you can avoid having to
convert to that model by just doing something like

        if (unlikely(__gu_err)) goto err_label;

inside the macro instead - even architectures that use the "asm goto"
do that for old compilers that don't support "asm goto" with output
values.

It shouldn't be too hard to implement on m68k - you'd likely want to
make the helper macros in <asm/uaccess.h> take that "MOVES" thing as a
new argument, so that you can generate code for the "move" vs "moves"
cases using the same helper macros, just with different arguments.

            Linus

Re: [PATCH RFC] Fixes to Linus' 'remove set_fs patch'

[Michael Schmitz]

Hi Linus,

On 7/07/21 6:47 am, Linus Torvalds wrote:
> On Mon, Jul 5, 2021 at 4:20 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Linus' patch didn't compile out of the box. I'm a little
>> worried that I had to patch mm/maccess.c,
> Yeah, that's not the right way to do it.
>
> See the other thread where Christoph rightly points out that the
> problem is that m68k needs __{get,put}_kernel_nofault() and then just
> enable CONFIG_HAVE_GET_KERNEL_NOFAULT.
Yep, saw that late yesterday and started implementing those.
>
> __{get,put}_kernel_nofault() is basically the similar to
> __get_user/__put_user, except for
>
>   (a) it uses kernel addresses (so in m68k parlance, "move" rather than "moves")
Thanks for reminding me ... after backing out the mm/maccess.c changes, 
new __get/put_kernel_asm() macros and basically copying the 
__get/put_kernel_nofault() framework from mips, I still have a working 
kernel. I'd like to test those changes on actual hardware though (and 
will need help from someone with a 040 or 060 machine for that).
>
>   (b) it uses the "goto err_label" approach instead of being an
> expression that returns an error value
>
> That (b) is what allows the use of "asm goto" to implement the
> exception case much more efficiently (but you can avoid having to
> convert to that model by just doing something like
>
>          if (unlikely(__gu_err)) goto err_label;
>
> inside the macro instead - even architectures that use the "asm goto"
> do that for old compilers that don't support "asm goto" with output
> values.
>
> It shouldn't be too hard to implement on m68k - you'd likely want to
> make the helper macros in <asm/uaccess.h> take that "MOVES" thing as a
> new argument, so that you can generate code for the "move" vs "moves"
> cases using the same helper macros, just with different arguments.

True - let's get some test coverage first though.

Cheers,

     Michael


>
>              Linus