[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <98cced9a-8bb2-4984-9db4-562bec9f462d@mkarcher.dialup.fu-berlin.de>
Date: Fri, 29 Aug 2025 15:24:58 +0200
From: Michael Karcher <kernel@...rcher.dialup.fu-berlin.de>
To: John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
linux-kernel@...r.kernel.org
Cc: sparclinux@...r.kernel.org, Andreas Larsson <andreas@...sler.com>,
Anthony Yznaga <anthony.yznaga@...cle.com>
Subject: Re: [PATCH 4/4] sparc: fix accurate exception reporting in
copy_{from_to}_user for Niagara 4
Am 28.08.2025 um 22:21 schrieb John Paul Adrian Glaubitz:
> On Tue, 2025-08-26 at 18:03 +0200, Michael Karcher wrote:
>> EX_ST(STORE(stx, %g1, %o0 + 0x00), memcpy_retl_o2_plus_o5_plus_32)
>> EX_ST(STORE(stx, %g2, %o0 + 0x08), memcpy_retl_o2_plus_o5_plus_24)
>> - EX_ST(STORE(stx, GLOBAL_SPARE, %o0 + 0x10), memcpy_retl_o2_plus_o5_plus_24)
>> + EX_ST(STORE(stx, GLOBAL_SPARE, %o0 + 0x10), memcpy_retl_o2_plus_o5_plus_16)
>> EX_ST(STORE(stx, %o4, %o0 + 0x18), memcpy_retl_o2_plus_o5_plus_8)
> Verified to not cause any regressions on SPARC T4. Whether it improved anything is
> hard to say. It might be that previously observed crashes after long uptimes are
> gone now.
I don't see how my patch will generate observable improvements. The patch is
"obviously correct", as it fixes the arithmetic progression 32/24/16/8, which
was originally interrupted, and also my test program agrees that the return
value is what it is "supposed to be" after applying the patch.
The old (likely unintended) code returns 8 more bytes left to copy than there
actually are left. This means that all bytes that are not indicated as "left
to copy" have actually been successfully copied, as well as 8 "hidden" bytes.
So the return value is kind-of valid.
Thus I think verifying that there are no regressions and reviewing the code
are the only quality assurance measures we can apply to this patch. I'm afraid
that this patch most likely will not fix the previously observed crashes, as
in this case, the return value was never too low (which could cause the kernel
to trust ouput bytes that have not been copied), and always less than the
amount requested to copy. Returning more bytes left than actually requested
can cause any kind of strange behaviour, as this is not expected by the callers
of this function, and can generate invalid states (like we observed with
negative file offsets in folio-enabled ext4 code with transparent huge pages
enabled on UltraSparc III).
Thanks for testing that there are indeed no regressions!
Kind regards,
Michael Karcher
Powered by blists - more mailing lists