linux-kernel - Re: [PATCH v6 10/20] x86/split_lock: Handle #AC exception for split lock

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: <20190404224913.GB178488@romley-ivt3.sc.intel.com>
Date:   Thu, 4 Apr 2019 15:49:13 -0700
From:   Fenghua Yu <fenghua.yu@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        Xiaoyao Li <xiaoyao.li@...el.com>,
        Michael Chan <michael.chan@...adcom.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        x86 <x86@...nel.org>, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v6 10/20] x86/split_lock: Handle #AC exception for split
 lock

On Thu, Apr 04, 2019 at 07:31:59PM +0200, Thomas Gleixner wrote:
> On Wed, 3 Apr 2019, Fenghua Yu wrote:
> > +dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code)
> > +{
> > +	unsigned int trapnr = X86_TRAP_AC;
> > +	char str[] = "alignment check";
> > +	int signr = SIGBUS;
> > +
> > +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> > +
> > +	/*
> > +	 * WARN*()s end up here; fix them up before we call the
> > +	 * notifier chain.
> > +	 */
> 
> How exactly is WARN*() ending up here?
> 
> > +	if (!user_mode(regs) && fixup_bug(regs, trapnr))
> 
> And that fixup_bug() check does what?
> 
> int fixup_bug(struct pt_regs *regs, int trapnr)
> {
>  	if (trapnr != X86_TRAP_UD)
>                 return 0;
> 
> Copy and paste from do_error_trap() ....

As you can see, do_alignment_check() is copied from do_error_trap().
But seems this part of code is irrelevant to #AC handler.

So I will remove the "if (!user_mode(regs) && fixup_bug(regs, trapnr))"
and surrounding code, right?

> 
> > +		return;
> > +
> > +	if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) ==
> > +		       NOTIFY_STOP)
> > +		return;
> > +
> > +	cond_local_irq_enable(regs);
> > +	if (!user_mode(regs) &&
> > +	    static_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT)) {
> > +		/*
> > +		 * Only split lock can generate #AC from kernel at this point.
> > +		 * Warn and disable split lock detection on this CPU. The
> > +		 * faulting instruction will be executed without generating
> > +		 * another #AC fault. User needs to check the warning and
> > +		 * fix the split lock issue in the faulting instruction.
> 
>   "User needs to check the warning and fix the issue ..."
> 
> I'm looking forward to all the fixes from Joe Users.
> 
> Please remove that sentence. It's useless. Users report warnings if at all
> and the kernel developers who actually look at them surely don't need an
> advice like that.

Sure. Will do this.

Thanks.

-Fenghua