linux-kernel - Re: [PATCH v10 6/6] x86/split_lock: Enable split lock detection by kernel parameter

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: <20191121161410.GA199273@romley-ivt3.sc.intel.com>
Date:   Thu, 21 Nov 2019 08:14:10 -0800
From:   Fenghua Yu <fenghua.yu@...el.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Tony Luck <tony.luck@...el.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>, x86 <x86@...nel.org>
Subject: Re: [PATCH v10 6/6] x86/split_lock: Enable split lock detection by
 kernel parameter

On Thu, Nov 21, 2019 at 02:01:53PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 21, 2019 at 07:04:44AM +0100, Ingo Molnar wrote:
> > * Fenghua Yu <fenghua.yu@...el.com> wrote:
> 
> > > +	split_lock_detect
> > > +			[X86] Enable split lock detection
> > > +			This is a real time or debugging feature. When enabled
> > > +			(and if hardware support is present), atomic
> > > +			instructions that access data across cache line
> > > +			boundaries will result in an alignment check exception.
> > > +			When triggered in applications the kernel will send
> > > +			SIGBUS. The kernel will panic for a split lock in
> > > +			OS code.
> > 
> > It would be really nice to be able to enable/disable this runtime as 
> > well, has this been raised before, and what was the conclusion?
> 
> It has, previous versions had that. Somehow a lot of things went missing
> and we're back to a broken neutered useless mess.
> 
> The problem appears to be that due to hardware design the feature cannot
> be virtualized, and instead of then disabling it when a VM runs/exists
> they just threw in the towel and went back to useless mode.. :-(

It's a bit complex to virtualize the TEST_CTRL MSR because it's per core
instead of per thread. But it's still doable to virtualize it as
discussion:
https://lore.kernel.org/lkml/20191017233824.GA23654@linux.intel.com/

KVM code will be released later. Even if there is no KVM code for split
lock, the patch set will killl qemu/guest if split lock happens there.
The goal of this patch set is to have a basic enabling code.

> 
> This feature MUST be default enabled, otherwise everything will
> be/remain broken and we'll end up in the situation where you can't use
> it even if you wanted to.

The usage scope of this patch set is largely reduced to only real time.
The long split lock processing time (>1000 cycles) cannot be tolerated
by real time.

Real time customers do want to use this feature to detect the fatal
split lock error. They don't want any split lock issue from BIOS/EFI/
firmware/kerne/drivers/user apps.

Real time can enable the feature (set bit 29 in TEST_CTRL MSR) in BIOS and
don't need OS to enable it. But, #AC handler cannot handle split lock
in the kernel and will return to the faulting instruction and re-enter #AC. So
current #AC handler doesn't provide useful information for the customers.
That's why we add the new #AC handler in this patch set.

>
> Imagine the BIOS/EFI/firmware containing an #AC exception. At that point
> the feature becomes useless, because you cannot enable it without your
> machine dying.

I believe Intel real time team guarantees to deliever a split lock FREE
BIOS/EFI/firmware to their real time users.

>From kernel point of view, we are working on a split lock free kernel.
Some blocking split lock issues have been fixed in TIP tree.

Only limited user apps can run on real time and should be split lock
free before they are allowed to run on the real time system.

So the feature is enabled only for real time that wants to have
a controlled split lock free environment.

The point is a split lock issue is a FATAL error on real time. Whenever
it happens, the long processing time (>1000 cycles) cannot meet hard real
time requirement any more and the system/user app has to die.

> 
> Now, from long and painful experience we all know that if a BIOS can be
> wrong, it will be. Therefore this feature will be/is useless as
> presented.
> 
> And I can't be arsed to look it up, but we've been making this very same
> argument since very early (possible the very first) version.
> 
> So this version goes straight into the bit bucket. Please try again.

In summary, the patch set only wants to enable the feature for real time
and disable it by default.

Thanks.

-Fenghua