linux-kernel - Re: [PATCH v4 04/17] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access

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]

Date:   Mon, 4 Mar 2019 11:29:41 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     Fenghua Yu <fenghua.yu@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        Xiaoyao Li <xiaoyao.li@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        x86 <x86@...nel.org>, kvm@...r.kernel.org
Subject: Re: [PATCH v4 04/17] x86/split_lock: Align x86_capability to unsigned
 long to avoid split locked access

On 3/4/19 11:15 AM, Fenghua Yu wrote:
> On Mon, Mar 04, 2019 at 10:52:19AM -0800, Dave Hansen wrote:
>> On 3/1/19 6:44 PM, Fenghua Yu wrote:
>>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>>> index 33051436c864..eb8ae701ef65 100644
>>> --- a/arch/x86/include/asm/processor.h
>>> +++ b/arch/x86/include/asm/processor.h
>>> @@ -93,7 +93,9 @@ struct cpuinfo_x86 {
>>>  	__u32			extended_cpuid_level;
>>>  	/* Maximum supported CPUID level, -1=no CPUID: */
>>>  	int			cpuid_level;
>>> -	__u32			x86_capability[NCAPINTS + NBUGINTS];
>>> +	/* Unsigned long alignment to avoid split lock in atomic bitmap ops */
>>> +	__u32			x86_capability[NCAPINTS + NBUGINTS]
>>> +				__aligned(sizeof(unsigned long));
>> I think this also warrants a comment in the changelog about the
>> alignment of 'struct cpuinfo_x86'.
> How about add "Depending on the starting address where GCC generates
> for data of struct cpuinfo_x86, x86_capability[] may or may not align to
> unsigned long...."?

If that's the case, then what good is this patch?  Sounds like some of
the story is missing.

You might want to dig through some of the past discussions about this.
I know this exact topic has been broached with Thomas.