linux-kernel - Re: [PATCH 1/1] x86/mm/pae: Align up pteval_t, pmdval_t and pudval

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zg0L/OfuQtVgFSsG@gmail.com>
Date: Wed, 3 Apr 2024 09:57:48 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Javier Pello <devel@...eo.eu>, linux-kernel@...r.kernel.org,
	x86@...nel.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 1/1] x86/mm/pae: Align up pteval_t, pmdval_t and pudval_t
 to avoid split locks


* Dave Hansen <dave.hansen@...el.com> wrote:

> On 4/2/24 10:23, Javier Pello wrote:
> > On Mon, 1 Apr 2024 10:56:14 -0700 Dave Hansen wrote:
> >> First of all, how is it that you're running a PAE kernel on new,
> >> 64-bit hardware?  That's rather odd.
> > 
> > I got this motherboard and cpu fairly recently to replace old
> > hardware, and I just plugged my old hard disk and went along with
> > it, because I did not feel like bootstrapping a 64-bit system.
> 
> Fair enough.  I can totally understand wanting the convenience.  But
> you're leaving _so_ much performance on the floor that split locks are
> the least of your problems.
> 
> >> The case that you're hitting is actually an on-stack pmd_t.  The
> >> fun part is that it's not shared and doesn't even _need_ atomics.
> >> I think it's just using pmd_populate() because it's convenient.
> > 
> > I see. So just annotating the variable on the stack with
> > __aligned(8) should do it? But the code is under mm/, so it should
> > be arch-agnostic, right? What would the correct fix be, then? I take
> > from your message that using atomics through pmd_populate() here is
> > not needed, but what accessors should be used instead? I am not
> > familiar at all with this part of the kernel.
> 
> I don't think there's a better accessor.
> 
> >> I'd honestly much rather just disable split lock support in 32-bit
> >> builds than mess with this stuff.  You really shouldn't be running
> >> 32-but kernels on this hardware.
> > 
> > Why? Is it unsupported?
> 
> Yes, it's effectively unsupported.  We're not adding new hardware 
> features to 32-bit.  The fact that split lock detection got enabled 
> was an accident.

We do accept well-tested fixes and minor enablement patches though, 
within reason - but indeed this page table entry alignment quirk added 
for the sake of a split-lock debugging false positive doesn't seem to 
be worth it.

> It's not a technical reason.  It's a practical one: I don't want to 
> spend time reviewing the fixes and dealing with the fallout and 
> regressions that the fixes might cause.

Yeah, so it's an indirect technical argument: fixes *with tradeoffs* 
like this one have a future maintenance & robustness cost. Fixes 
without tradeoffs are fine of course.

Thanks,

	Ingo