Re: [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully

[Finn Thain <fthain@linux-m68k.org>]

On Sat, 27 Apr 2024, Michael Schmitz wrote:

> 
> diff --git a/arch/m68k/lib/uaccess.c b/arch/m68k/lib/uaccess.c
> index 86b6fed5151c..e3ed893047f8 100644
> --- a/arch/m68k/lib/uaccess.c
> +++ b/arch/m68k/lib/uaccess.c
> @@ -62,35 +62,42 @@ unsigned long __generic_copy_to_user(void __user *to, const void *from,
>  
>  	asm volatile ("\n"
>  		"	tst.l	%0\n"
> -		"	jeq	4f\n"
> +		"	jeq	5f\n"
>  		"1:	move.l	(%1)+,%3\n"
>  		"2:	"MOVES".l	%3,(%2)+\n"
>  		"3:	subq.l	#1,%0\n"
> -		"	jne	1b\n"
> -		"4:	btst	#1,%5\n"
> -		"	jeq	6f\n"
> -		"	move.w	(%1)+,%3\n"
> -		"5:	"MOVES".w	%3,(%2)+\n"
> -		"6:	btst	#0,%5\n"
> +		"4:	jne	1b\n"
> +		"5:	btst	#1,%5\n"
>  		"	jeq	8f\n"
> -		"	move.b	(%1)+,%3\n"
> -		"7:	"MOVES".b  %3,(%2)+\n"
> -		"8:\n"
> +		"6:	move.w	(%1)+,%3\n"
> +		"7:	"MOVES".w	%3,(%2)+\n"
> +		"8:	btst	#0,%5\n"
> +		"9:	jeq	13f\n"
> +		"10:	move.b	(%1)+,%3\n"
> +		"11:	"MOVES".b	%3,(%2)+\n"
> +		"12:	nop\n"
> +		"13:\n"
>  		"	.section .fixup,\"ax\"\n"
>  		"	.even\n"
>  		"20:	lsl.l	#2,%0\n"
>  		"50:	add.l	%5,%0\n"
> -		"	jra	8b\n"
> +		"	jra	13b\n"
>  		"	.previous\n"
>  		"\n"
>  		"	.section __ex_table,\"a\"\n"
>  		"	.align	4\n"
> +		"	.long	1b,20b\n"
>  		"	.long	2b,20b\n"
>  		"	.long	3b,20b\n"
> -		"	.long	5b,50b\n"
> +		"	.long	4b,20b\n"
> +		"	.long	5b,20b\n"
>  		"	.long	6b,50b\n"

I think a fault at 6b has to get fixed up at 20b.

>  		"	.long	7b,50b\n"
>  		"	.long	8b,50b\n"
> +		"	.long	9b,50b\n"
> +		"	.long	10b,50b\n"
> +		"	.long	11b,50b\n"
> +		"	.long	12b,50b\n"
>  		"	.previous"
>  		: "=d" (res), "+a" (from), "+a" (to), "=&d" (tmp)
>  		: "0" (n / 4), "d" (n & 3));
> @@ -109,32 +116,35 @@ unsigned long __clear_user(void __user *to, unsigned long n)
>  
>  	asm volatile ("\n"
>  		"	tst.l	%0\n"
> -		"	jeq	3f\n"
> +		"	jeq	4f\n"
>  		"1:	"MOVES".l	%2,(%1)+\n"
>  		"2:	subq.l	#1,%0\n"
> -		"	jne	1b\n"
> -		"3:	btst	#1,%4\n"
> -		"	jeq	5f\n"
> -		"4:	"MOVES".w	%2,(%1)+\n"
> -		"5:	btst	#0,%4\n"
> -		"	jeq	7f\n"
> -		"6:	"MOVES".b	%2,(%1)\n"
> -		"7:\n"
> +		"3:	jne	1b\n"
> +		"4:	btst	#1,%4\n"
> +		"	jeq	6f\n"
> +		"5:	"MOVES".w	%2,(%1)+\n"
> +		"6:	btst	#0,%4\n"
> +		"7:	jeq	9f\n"
> +		"8:	"MOVES".b	%2,(%1)\n"
> +		"9:\n"

Should we put a NOP here to avoid having the unknown next instruction 
(label 9) in the exception table? We can't actually fix up a fault there 
unless by chance it was the MOVES that caused it.

>  		"	.section .fixup,\"ax\"\n"
>  		"	.even\n"
>  		"10:	lsl.l	#2,%0\n"
>  		"40:	add.l	%4,%0\n"
> -		"	jra	7b\n"
> +		"	jra	9b\n"
>  		"	.previous\n"
>  		"\n"
>  		"	.section __ex_table,\"a\"\n"
>  		"	.align	4\n"
>  		"	.long	1b,10b\n"
>  		"	.long	2b,10b\n"
> -		"	.long	4b,40b\n"
> +		"	.long	3b,10b\n"
> +		"	.long	4b,10b\n"
>  		"	.long	5b,40b\n"
>  		"	.long	6b,40b\n"
>  		"	.long	7b,40b\n"
> +		"	.long	8b,40b\n"
> +		"	.long	9b,40b\n"
>  		"	.previous"
>  		: "=d" (res), "+a" (to)
>  		: "d" (0), "0" (n / 4), "d" (n & 3));
>