linux-kernel - Re: [PATCH v3 1/3] x86/fpu: Make XCR0 accessors immune to unwanted compiler reordering

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: <1cb5ccc3-a84d-06da-a8e8-d9d3d8418d25@intel.com>
Date:   Fri, 1 Apr 2022 11:16:19 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     "Chang S. Bae" <chang.seok.bae@...el.com>,
        linux-kernel@...r.kernel.org, x86@...nel.org,
        linux-pm@...r.kernel.org
Cc:     tglx@...utronix.de, dave.hansen@...ux.intel.com,
        peterz@...radead.org, bp@...en8.de, rafael@...nel.org,
        ravi.v.shankar@...el.com
Subject: Re: [PATCH v3 1/3] x86/fpu: Make XCR0 accessors immune to unwanted
 compiler reordering

On 4/1/22 10:58, Dave Hansen wrote:
> On 3/24/22 19:22, Chang S. Bae wrote:
>> Some old GCC versions (4.9.x and 5.x) have an issue that incorrectly
>> reordering volatile asm statements with each other [1]. While this bug was
>> fixed on later versions (8.1, 7.3, and 6.5), and the kernel's current XCR0
>> read/write do not appear to be impacted, it is preventive to ensure them on
>> the program order.
> I thought you *actually* saw xgetbv() be reordered, though.  Was that on
> a buggy compiler?

Actually, reading the gcc bug[1] a bit more, it says:

> However, correctness here depends on the compiler honouring the
> ordering of volatile asm statements with respect to other volatile
> code, and this patch fixes that.
In your case, there was presumably no volatile code, just a
fpu_state_size_dynamic() call.  The compiler thought the xgetbv() was
safe to reorder, and did so.

So, I'm not quite sure how that bug is relevant.  It just dealt with
multiple "asm volatile" statements that don't have memory clobbers.  We
only have one "asm volatile" in play.  Adding the memory clobber should
make that bug totally irrelevant.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82602