linux-kernel - RE: [PATCH 3/3] x86/split_lock: Align the x86_capability array to size of unsigned long

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d75c94cf2ca345018ef60880ce6c4aeb@AcuMS.aculab.com>
Date:   Tue, 17 Sep 2019 08:29:28 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Tony Luck' <tony.luck@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>
CC:     Fenghua Yu <fenghua.yu@...el.com>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>, H Peter Anvin <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Dave Hansen" <dave.hansen@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        "Radim Krcmar" <rkrcmar@...hat.com>,
        Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>, x86 <x86@...nel.org>
Subject: RE: [PATCH 3/3] x86/split_lock: Align the x86_capability array to
 size of unsigned long

From: Tony Luck
> Sent: 16 September 2019 23:40
> From: Fenghua Yu <fenghua.yu@...el.com>
> 
> The x86_capability array in cpuinfo_x86 is defined as u32 and thus is
> naturally aligned to 4 bytes. But, set_bit() and clear_bit() require
> the array to be aligned to size of unsigned long (i.e. 8 bytes in
> 64-bit).
> 
> To fix the alignment issue, align the x86_capability array to size of
> unsigned long by using unnamed union and 'unsigned long array_align'
> to force the alignment.
> 
> Changing the x86_capability array's type to unsigned long may also fix
> the issue because the x86_capability array will be naturally aligned
> to size of unsigned long. But this needs additional code changes.
> So choose the simpler solution by setting the array's alignment to size
> of unsigned long.
> 
> Suggested-by: David Laight <David.Laight@...lab.com>

While this is probably the only play where this 'capabilities' array
has been detected as misaligned, ISTR there are several other places
where the identical array is defined and used.
These all need fixing as well.

	David

> Suggested-by: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
>  arch/x86/include/asm/processor.h | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 6e0a3b43d027..c073534ca485 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -93,7 +93,15 @@ struct cpuinfo_x86 {
>  	__u32			extended_cpuid_level;
>  	/* Maximum supported CPUID level, -1=no CPUID: */
>  	int			cpuid_level;
> -	__u32			x86_capability[NCAPINTS + NBUGINTS];
> +	/*
> +	 * Align to size of unsigned long because the x86_capability array
> +	 * is passed to bitops which require the alignment. Use unnamed
> +	 * union to enforce the array is aligned to size of unsigned long.
> +	 */
> +	union {
> +		__u32		x86_capability[NCAPINTS + NBUGINTS];
> +		unsigned long	x86_capability_alignment;
> +	};
>  	char			x86_vendor_id[16];
>  	char			x86_model_id[64];
>  	/* in KB - valid for CPUS which support this call: */
> --
> 2.20.1

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