linux-kernel - Re: [PATCH v2 05/18] x86/fixmap: Generalize the GDT fixmap mechanism

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: <20171122153327.fyxn4zspgdrjs45s@pd.tnic>
Date:   Wed, 22 Nov 2017 16:33:38 +0100
From:   Borislav Petkov <bp@...e.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     X86 ML <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Brian Gerst <brgerst@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH v2 05/18] x86/fixmap: Generalize the GDT fixmap mechanism

On Tue, Nov 21, 2017 at 08:44:00PM -0800, Andy Lutomirski wrote:
> Currently, the GDT is an ad-hoc array of pages, one per CPU, in the
> fixmap.  Generalize it to be an array of a new struct cpu_entry_area
> so that we can cleanly add new things to it.
> 
> Signed-off-by: Andy Lutomirski <luto@...nel.org>
> ---
>  arch/x86/include/asm/desc.h   |  9 +--------
>  arch/x86/include/asm/fixmap.h | 36 ++++++++++++++++++++++++++++++++++--
>  arch/x86/kernel/cpu/common.c  | 14 +++++++-------
>  arch/x86/xen/mmu_pv.c         |  2 +-
>  4 files changed, 43 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> index 95cd95eb7285..194ffab00ebe 100644
> --- a/arch/x86/include/asm/desc.h
> +++ b/arch/x86/include/asm/desc.h
> @@ -60,17 +60,10 @@ static inline struct desc_struct *get_current_gdt_rw(void)
>  	return this_cpu_ptr(&gdt_page)->gdt;
>  }
>  
> -/* Get the fixmap index for a specific processor */
> -static inline unsigned int get_cpu_gdt_ro_index(int cpu)
> -{
> -	return FIX_GDT_REMAP_END - cpu;
> -}
> -
>  /* Provide the fixmap address of the remapped GDT */
>  static inline struct desc_struct *get_cpu_gdt_ro(int cpu)
>  {
> -	unsigned int idx = get_cpu_gdt_ro_index(cpu);
> -	return (struct desc_struct *)__fix_to_virt(idx);
> +	return (struct desc_struct *)&get_cpu_entry_area(cpu)->gdt;
>  }
>  
>  /* Provide the current read-only GDT */
> diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
> index b0c505fe9a95..038b8474c7f8 100644
> --- a/arch/x86/include/asm/fixmap.h
> +++ b/arch/x86/include/asm/fixmap.h

Btw, the whole cpu_entry_area machinery looks like it should belong more
in asm/processor.h and not in fixmap.h

> @@ -44,6 +44,17 @@ extern unsigned long __FIXADDR_TOP;
>  			 PAGE_SIZE)
>  #endif
>  
> +/*
> + * cpu_entry_area is a percpu region in the fixmap that contains things
> + * needed by the CPU and early entry/exit code.  Real types aren't used
> + * for all fields here to about circular header dependencies.

s/about/avoid/

> + */
> +struct cpu_entry_area
> +{

ERROR: open brace '{' following struct go on the same line
#67: FILE: arch/x86/include/asm/fixmap.h:53:
+struct cpu_entry_area
+{

> +	char gdt[PAGE_SIZE];
> +};
> +
> +#define CPU_ENTRY_AREA_PAGES (sizeof(struct cpu_entry_area) / PAGE_SIZE)
>  
>  /*
>   * Here we define all the compile-time 'special' virtual
> @@ -101,8 +112,8 @@ enum fixed_addresses {
>  	FIX_LNW_VRTC,
>  #endif
>  	/* Fixmap entries to remap the GDTs, one per processor. */
> -	FIX_GDT_REMAP_BEGIN,
> -	FIX_GDT_REMAP_END = FIX_GDT_REMAP_BEGIN + NR_CPUS - 1,
> +	FIX_CPU_ENTRY_AREA_TOP,
> +	FIX_CPU_ENTRY_AREA_BOTTOM = FIX_CPU_ENTRY_AREA_TOP + (CPU_ENTRY_AREA_PAGES * NR_CPUS) - 1,
>  
>  #ifdef CONFIG_ACPI_APEI_GHES
>  	/* Used for GHES mapping from assorted contexts */
> @@ -191,5 +202,26 @@ void __init *early_memremap_decrypted_wp(resource_size_t phys_addr,
>  void __early_set_fixmap(enum fixed_addresses idx,
>  			phys_addr_t phys, pgprot_t flags);
>  
> +static inline unsigned int __get_cpu_entry_area_page_index(int cpu, int page)
> +{
> +	BUILD_BUG_ON(sizeof(struct cpu_entry_area) % PAGE_SIZE != 0);

	BUILD_BUG_ON(sizeof(struct cpu_entry_area) % PAGE_SIZE);

> +
> +	return FIX_CPU_ENTRY_AREA_BOTTOM - cpu*CPU_ENTRY_AREA_PAGES - page;
> +}
> +
> +#define __get_cpu_entry_area_offset_index(cpu, offset) ({		\
> +	BUILD_BUG_ON(offset % PAGE_SIZE != 0);				\

	BUILD_BUG_ON(offset % PAGE_SIZE);

> +	__get_cpu_entry_area_page_index(cpu, offset / PAGE_SIZE);	\
> +	})
> +
> +#define get_cpu_entry_area_index(cpu, field)				\
> +	__get_cpu_entry_area_offset_index((cpu), offsetof(struct cpu_entry_area, field))
> +
> +static inline struct cpu_entry_area *get_cpu_entry_area(int cpu)
> +{
> +	return (struct cpu_entry_area *)
> +		__fix_to_virt(__get_cpu_entry_area_page_index(cpu, 0));

Just let it stick out.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--