lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ