linux-kernel - Re: [PATCHv6 06/30] x86/traps: Refactor exc_general

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: <20220317140536.ebab2nenzr64mizp@black.fi.intel.com>
Date:   Thu, 17 Mar 2022 17:05:36 +0300
From:   "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     mingo@...hat.com, bp@...en8.de, dave.hansen@...el.com,
        luto@...nel.org, peterz@...radead.org,
        sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        thomas.lendacky@....com, brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org,
        Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCHv6 06/30] x86/traps: Refactor exc_general_protection()

On Thu, Mar 17, 2022 at 01:21:53AM +0100, Thomas Gleixner wrote:
> On Wed, Mar 16 2022 at 05:08, Kirill A. Shutemov wrote:
> > TDX brings a new exception -- Virtualization Exception (#VE). Handling
> > of #VE structurally very similar to handling #GP.
> >
> > Extract two helpers from exc_general_protection() that can be reused for
> > handling #VE.
> >
> > No functional changes.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> > Reviewed-by: Dave Hansen <dave.hansen@...ux.intel.com>
> 
> Not that I care much, but this is the second instance of something I
> suggested. We have tags for that...

Sorry, will add the tag.

By the other instance you meant common base for SEAMCALL and TDCALL C
wrappers, right? Will fix too.

> > +static bool gp_try_fixup_and_notify(struct pt_regs *regs, int trapnr,
> > +				    unsigned long error_code, const char *str)
> > +{
> > +	int ret;
> 
> Adding this to make my suggestion compile is the easy way out, but...
> 
> > +
> > +	if (fixup_exception(regs, trapnr, error_code, 0))
> > +		return true;
> > +
> > +	current->thread.error_code = error_code;
> > +	current->thread.trap_nr = trapnr;
> > +
> > +	/*
> > +	 * To be potentially processing a kprobe fault and to trust the result
> > +	 * from kprobe_running(), we have to be non-preemptible.
> > +	 */
> > +	if (!preemptible() && kprobe_running() &&
> > +	    kprobe_fault_handler(regs, trapnr))
> > +		return true;
> > +
> > +	ret = notify_die(DIE_GPF, str, regs, error_code, trapnr, SIGSEGV);
> > +	return ret == NOTIFY_STOP;
> 
> Why not doing the obvious:
> 
> 	return notify_die(DIE_GPF, str, regs, error_code, trapnr, SIGSEGV) == NOTIFY_STOP;
> 
> Hmm?

I don't like lines this long (although my variant doesn't look good too).

Will do your way.

-- 
 Kirill A. Shutemov