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

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Oct 27, 2023 at 08:47:53PM +0200, Sam Ravnborg wrote:
> 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?

Looking at the csum_and_copy_... instances on various architectures,
noticing that and going "how the fuck could it possibly work and
what moron had broken it?  Oh, lovely - it couldn't, it doesn't
and that moron had been myself ;-/"

> Do you actually use sparc32 these
> days?

qemu image only, TBH - I have an SS20 box, but it hadn't booted for
about 10 years...

> You could also kill the EX2 define while touchign the file,
> it is no longer used after ab5e8b331244.

Er?  No EX2 in checksum_32.S...  There is one in copy_user.S,
but that one _is_ used -

copy_user_table_end:
        be      copy_user_last7
         andcc  %g1, 4, %g0

        EX(ldd  [%o1], %g2, and %g1, 0xf)
        add     %o0, 8, %o0
        add     %o1, 8, %o1
        EX(st   %g2, [%o0 - 0x08], and %g1, 0xf)
        EX2(st  %g3, [%o0 - 0x04], and %g1, 0xf, %g1, sub %g1, 4)

> > 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>