Re: [PATCH] sparc32: fix a braino in fault handling in csum_and_copy_..._user()

[Sam Ravnborg <sam@ravnborg.org>]

Hi Al,

On Thu, Oct 26, 2023 at 03:16:13AM +0100, Al Viro wrote:
> Fault handler used to make non-trivial calls, so it needed
> to set a stack frame up.  Used to be
> 	save ... - grab a stack frame, old %o... become %i...
> 	....
> 	ret	- go back to address originally in %o7, currently %i7
> 	 restore - switch to previous stack frame, in delay slot
> Non-trivial calls had been gone since ab5e8b331244 and that code should
> have become
> 	retl	- go back to address in %o7
> 	 clr %o0 - have return value set to 0
> What it had become instead was
> 	ret	- go back to address in %i7 - return address of *caller*
> 	 clr %o0 - have return value set to 0
> which is not good, to put it mildly - we forcibly return 0 from
> csum_and_copy_{from,to}_iter() (which is what the call of that
> thing had been inlined into) and do that without dropping the
> stack frame of said csum_and_copy_..._iter().  Confuses the
> hell out of the caller of csum_and_copy_..._iter(), obviously...

I wonder how you managed to find this? Do you actually use sparc32 these
days?

You could also kill the EX2 define while touchign the file,
it is no longer used after ab5e8b331244.
> 
> Fixes: ab5e8b331244 "sparc32: propagate the calling conventions change down to __csum_partial_copy_sparc_generic()"
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>


> ---
>  arch/sparc/lib/checksum_32.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/sparc/lib/checksum_32.S b/arch/sparc/lib/checksum_32.S
> index 84ad709cbecb..66eda40fce36 100644
> --- a/arch/sparc/lib/checksum_32.S
> +++ b/arch/sparc/lib/checksum_32.S
> @@ -453,5 +453,5 @@ ccslow:	cmp	%g1, 0
>   * we only bother with faults on loads... */
>  
>  cc_fault:
> -	ret
> +	retl
>  	 clr	%o0
> -- 
> 2.39.2