linux-kernel - Re: [PATCH v10 13/28] x86/fpu/xstate: Use feature disable (XFD) to protect dynamic user state

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: <87bl45ge7t.ffs@tglx>
Date:   Mon, 04 Oct 2021 14:35:18 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Bae, Chang Seok" <chang.seok.bae@...el.com>
Cc:     "bp@...e.de" <bp@...e.de>, "Lutomirski, Andy" <luto@...nel.org>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "Brown, Len" <len.brown@...el.com>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "Macieira, Thiago" <thiago.macieira@...el.com>,
        "Liu, Jing2" <jing2.liu@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v10 13/28] x86/fpu/xstate: Use feature disable (XFD) to
 protect dynamic user state

On Sun, Oct 03 2021 at 22:38, Chang Seok Bae wrote:
> On Oct 1, 2021, at 08:10, Thomas Gleixner <tglx@...utronix.de> wrote:
>> 	local_irq_enable();
>
> v1 had some similar ones (not the same though) [1]. FWIW, I think Andy’s point
> is worth to be noted here:
>
>     First, you can't just enable IRQs here. If IRQs are off, they're off for a
>     reason. Secondly, if they're *on*, you just forgot that fact.

The #NM comes from user space where interrupts are always enabled. So we
can enable interrupts _after_ doing the sanity checks.

Also we reenable interrupts in various other trap handlers when the trap
comes from user space as well. That's perfectly fine and required. How
would e.g. fault handling or single stepping ever work otherwise?

I have no idea where you had places the local_irq_enable(), but the code
I outlined is correct.

Thanks,

        tglx