linux-kernel - Re: [PATCH v7 1/2] x86/split_lock: Rework the initialization flow of split lock detection

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: <20200330142648.GA24988@linux.intel.com>
Date:   Mon, 30 Mar 2020 07:26:48 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Xiaoyao Li <xiaoyao.li@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        hpa@...or.com, x86@...nel.org, linux-kernel@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>, luto@...nel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Tony Luck <tony.luck@...el.com>
Subject: Re: [PATCH v7 1/2] x86/split_lock: Rework the initialization flow of
 split lock detection

On Mon, Mar 30, 2020 at 09:26:25PM +0800, Xiaoyao Li wrote:
> On 3/29/2020 12:32 AM, Sean Christopherson wrote:
> >On Wed, Mar 25, 2020 at 11:09:23AM +0800, Xiaoyao Li wrote:
> >>  static void split_lock_init(void)
> >>  {
> >>-	if (sld_state == sld_off)
> >>-		return;
> >>-
> >>-	if (__sld_msr_set(true))
> >>-		return;
> >>-
> >>-	/*
> >>-	 * If this is anything other than the boot-cpu, you've done
> >>-	 * funny things and you get to keep whatever pieces.
> >>-	 */
> >>-	pr_warn("MSR fail -- disabled\n");
> >>-	sld_state = sld_off;
> >>+	split_lock_verify_msr(sld_state != sld_off);
> >
> >I think it'd be worth a WARN_ON() if this fails with sld_state != off.  If
> >the WRMSR fails, then presumably SLD is off when it's expected to be on.
> >The implied WARN on the unsafe WRMSR in sld_update_msr() won't fire unless
> >a task generates an #AC on a non-buggy core and then gets migrated to the
> >buggy core.  Even if the WARNs are redundant, if something is wrong it'd be
> >a lot easier for a user to triage/debug if there is a WARN in boot as
> >opposed to a runtime WARN that requires a misbehaving application and
> >scheduler behavior.
> >
> 
> IIUC, you're recommending something like below?
> 
>         WARN_ON(!split_lock_verify_msr(sld_state != sld_off) &&
> 		sld_state != sld_off);

Ya.