linux-kernel - Re: correct section for cpu_tss

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: <alpine.DEB.2.20.1801032025310.1957@nanos>
Date:   Wed, 3 Jan 2018 20:27:32 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
cc:     Tejun Heo <tj@...nel.org>, cl@...ux.com, mingo@...hat.com,
        "H. Peter Anvin" <hpa@...or.com>, bp@...e.de,
        Andy Lutomirski <luto@...nel.org>,
        kirill.shutemov@...ux.intel.com, thgarnie@...gle.com,
        Josh Poimboeuf <jpoimboe@...hat.com>, minipli@...glemail.com,
        me@...ehuey.com, namit@...are.com, tklauser@...tanz.ch,
        thomas.lendacky@....com, LKML <linux-kernel@...r.kernel.org>,
        x86@...nel.org
Subject: Re: correct section for cpu_tss_rw?

On Tue, 2 Jan 2018, Nick Desaulniers wrote:
> (emailing the folks listed from running `./scripts/get_maintainer.pl
> -f` on arch/x86/kernel/process.c, arch/x86/include/asm/processor.h,
> and include/linux/percpu-defs.h)
> 
> Clang emits the following warning:
> 
> arch/x86/kernel/process.c:50:11: warning: section does not match
> previous declaration [-Wsection]
> __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
>           ^
> ./include/linux/percpu-defs.h:144:2: note: expanded from macro
> 'DEFINE_PER_CPU_SHARED_ALIGNED'
>         DEFINE_PER_CPU_SECTION(type, name, PER_CPU_SHARED_ALIGNED_SECTION) \
>         ^
> ./include/linux/percpu-defs.h:104:2: note: expanded from macro
> 'DEFINE_PER_CPU_SECTION'
>         __PCPU_ATTRS(sec) PER_CPU_DEF_ATTRIBUTES                        \
>         ^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> ./arch/x86/include/asm/processor.h:365:1: note: previous attribute is here
> DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw);
> ^
> ./include/linux/percpu-defs.h:159:2: note: expanded from macro
> 'DECLARE_PER_CPU_PAGE_ALIGNED'
>         DECLARE_PER_CPU_SECTION(type, name, "..page_aligned")           \
>         ^
> ./include/linux/percpu-defs.h:101:9: note: expanded from macro
> 'DECLARE_PER_CPU_SECTION'
>         extern __PCPU_ATTRS(sec) __typeof__(type) name
>                ^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
>         __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))     \
>                                 ^
> 
> it seems that from commit c482feefe1a ("x86/entry/64: Make
> cpu_entry_area.tss read-only") that cpu_tss_rw is declared but then
> defined in two different sections. (Though, it looks like this issue
> predates that commit).
>
> It seems that cpu_tss_rw is defined as SHARED_ALIGNED, but then
> declared as PAGE_ALIGNED.  Should be an easy fix (that I'm happy to
> author), but what section *should* cpu_tss_rw be in (SHARED_ALIGNED or
> PAGE_ALIGNED)?  That affects whether I fix the declaration or
> definition (and thus the .h or the .c file).
> 
> >From the comment in arch/x86/kernel/process.c#50:
>  43 /*
>  44  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
>  45  * no more per-task TSS's. The TSS size is kept cacheline-aligned
>  46  * so they are allowed to end up in the .data..cacheline_aligned
>  47  * section. Since TSS's are completely CPU-local, we want them
>  48  * on exact cacheline boundaries, to eliminate cacheline
> ping-pong.
>  49  */
>  50 __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
> 
> I suspect that cache-line alignment is stricter than page alignment,
> so the declaration should be fixed, but I was not sure and wanted to
> check?

It must be page aligned.

Thanks,

	tglx