linux-kernel - RE: [PATCH v12 10/18] x86/fsgsbase/64: Enable FSGSBASE instructions in helper functions

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: <238a6e2609eb4e0497b13fbe5f531917@AcuMS.aculab.com>
Date:   Tue, 19 May 2020 12:20:16 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Sasha Levin' <sashal@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "bp@...en8.de" <bp@...en8.de>, "luto@...nel.org" <luto@...nel.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "dave.hansen@...el.com" <dave.hansen@...el.com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "ravi.v.shankar@...el.com" <ravi.v.shankar@...el.com>,
        "chang.seok.bae@...el.com" <chang.seok.bae@...el.com>,
        Andrew Cooper <andrew.cooper3@...rix.com>,
        "x86@...nel.org" <x86@...nel.org>
Subject: RE: [PATCH v12 10/18] x86/fsgsbase/64: Enable FSGSBASE instructions
 in helper functions

From: Sasha Levin
> Sent: 18 May 2020 21:25
> Thank you for taking the time to review this.
> 
> On Mon, May 18, 2020 at 08:20:08PM +0200, Thomas Gleixner wrote:
> >Sasha Levin <sashal@...nel.org> writes:
> >> +unsigned long x86_gsbase_read_cpu_inactive(void)
> >> +{
> >> +	unsigned long gsbase;
> >> +
> >> +	if (static_cpu_has(X86_FEATURE_FSGSBASE)) {
> >> +		bool need_restore = false;
> >> +		unsigned long flags;
> >> +
> >> +		/*
> >> +		 * We read the inactive GS base value by swapping
> >> +		 * to make it the active one. But we cannot allow
> >> +		 * an interrupt while we switch to and from.
> >> +		 */
> >> +		if (!irqs_disabled()) {
> >> +			local_irq_save(flags);
> >> +			need_restore = true;
> >> +		}
> >> +
> >> +		native_swapgs();
> >> +		gsbase = rdgsbase();
> >> +		native_swapgs();

Does local_irq_save() even do anything useful here.
You need to actually execute CLI, not just set a
flag that indicates interrupts shouldn't happen.
(Which is what I think local_irq_save() might do.)

You also (probably) need to disable NMIs.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)