diff mbox

Re: [PATCH v3 1/4] x86/mm: Adapt MODULES_END based on Fixmap section size

Message ID 20170215135824.tflwuf3esu2npwwo@pd.tnic (mailing list archive)
State New, archived
Headers show

Commit Message

Borislav Petkov Feb. 15, 2017, 1:58 p.m. UTC
On Tue, Feb 14, 2017 at 11:42:56AM -0800, Thomas Garnier wrote:
> This patch aligns MODULES_END to the beginning of the Fixmap section.
> It optimizes the space available for both sections. The address is
> pre-computed based on the number of pages required by the Fixmap
> section.
> 
> It will allow GDT remapping in the Fixmap section. The current
> MODULES_END static address does not provide enough space for the kernel
> to support a large number of processors.
> 
> Signed-off-by: Thomas Garnier <thgarnie@google.com>
> ---
> Based on next-20170213
> ---
>  arch/x86/include/asm/fixmap.h           | 8 ++++++++
>  arch/x86/include/asm/pgtable_64_types.h | 3 ---
>  arch/x86/kernel/module.c                | 1 +
>  arch/x86/mm/dump_pagetables.c           | 1 +
>  arch/x86/mm/kasan_init_64.c             | 1 +
>  5 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
> index 8554f960e21b..20231189e0e3 100644
> --- a/arch/x86/include/asm/fixmap.h
> +++ b/arch/x86/include/asm/fixmap.h
> @@ -132,6 +132,14 @@ enum fixed_addresses {
>  
>  extern void reserve_top_address(unsigned long reserve);
>  
> +/* On 64-bit, the module sections ends with the start of the fixmap */
> +#ifdef CONFIG_X86_64
> +#define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)
> +#define MODULES_END   __fix_to_virt(__end_of_fixed_addresses + 1)
> +#define MODULES_LEN   (MODULES_END - MODULES_VADDR)
> +#endif /* CONFIG_X86_64 */

JFYI: so there's another patchset which adds KERNEL_MAPPING_SIZE:

https://lkml.kernel.org/r/1486040077-3719-1-git-send-email-bhe@redhat.com

and makes it a 1G, i.e., the KASLR default. I guess the above will have
to be KERNEL_MAPPING_SIZE then.

And why are you moving those to fixmap.h? What's wrong with
including fixmap.h into pgtable_64_types.h so that you can get
__end_of_fixed_addresses?

FWIW, I didn't even have to add any includes with my .config, i.e., that builds:

---
---

but I wouldn't be surprised if some strange configuration would need it.

>  #define FIXADDR_SIZE	(__end_of_permanent_fixed_addresses << PAGE_SHIFT)
>  #define FIXADDR_START		(FIXADDR_TOP - FIXADDR_SIZE)
>  
> diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
> index 3a264200c62f..de8bace10200 100644
> --- a/arch/x86/include/asm/pgtable_64_types.h
> +++ b/arch/x86/include/asm/pgtable_64_types.h
> @@ -66,9 +66,6 @@ typedef struct { pteval_t pte; } pte_t;
>  #define VMEMMAP_START	__VMEMMAP_BASE
>  #endif /* CONFIG_RANDOMIZE_MEMORY */
>  #define VMALLOC_END	(VMALLOC_START + _AC((VMALLOC_SIZE_TB << 40) - 1, UL))
> -#define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)
> -#define MODULES_END      _AC(0xffffffffff000000, UL)

How much of an ABI breakage would that be? See
Documentation/x86/x86_64/mm.txt.

With my .config MODULES_END becomes 0xffffffffff1fe000 and it'll remain
dynamic depending on .config. No idea how much in userspace relies on
MODULES_END being static 0xffffffffff000000...

Hmm.

Comments

Thomas Garnier Feb. 15, 2017, 3:40 p.m. UTC | #1
On Wed, Feb 15, 2017 at 5:58 AM, Borislav Petkov <bp@suse.de> wrote:
>
> On Tue, Feb 14, 2017 at 11:42:56AM -0800, Thomas Garnier wrote:
> > This patch aligns MODULES_END to the beginning of the Fixmap section.
> > It optimizes the space available for both sections. The address is
> > pre-computed based on the number of pages required by the Fixmap
> > section.
> >
> > It will allow GDT remapping in the Fixmap section. The current
> > MODULES_END static address does not provide enough space for the kernel
> > to support a large number of processors.
> >
> > Signed-off-by: Thomas Garnier <thgarnie@google.com>
> > ---
> > Based on next-20170213
> > ---
> >  arch/x86/include/asm/fixmap.h           | 8 ++++++++
> >  arch/x86/include/asm/pgtable_64_types.h | 3 ---
> >  arch/x86/kernel/module.c                | 1 +
> >  arch/x86/mm/dump_pagetables.c           | 1 +
> >  arch/x86/mm/kasan_init_64.c             | 1 +
> >  5 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
> > index 8554f960e21b..20231189e0e3 100644
> > --- a/arch/x86/include/asm/fixmap.h
> > +++ b/arch/x86/include/asm/fixmap.h
> > @@ -132,6 +132,14 @@ enum fixed_addresses {
> >
> >  extern void reserve_top_address(unsigned long reserve);
> >
> > +/* On 64-bit, the module sections ends with the start of the fixmap */
> > +#ifdef CONFIG_X86_64
> > +#define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)
> > +#define MODULES_END   __fix_to_virt(__end_of_fixed_addresses + 1)
> > +#define MODULES_LEN   (MODULES_END - MODULES_VADDR)
> > +#endif /* CONFIG_X86_64 */
>
> JFYI: so there's another patchset which adds KERNEL_MAPPING_SIZE:
>
> https://lkml.kernel.org/r/1486040077-3719-1-git-send-email-bhe@redhat.com
>
> and makes it a 1G, i.e., the KASLR default. I guess the above will have
> to be KERNEL_MAPPING_SIZE then.
>
> And why are you moving those to fixmap.h? What's wrong with
> including fixmap.h into pgtable_64_types.h so that you can get
> __end_of_fixed_addresses?
>
> FWIW, I didn't even have to add any includes with my .config, i.e., that builds:
>

I tried to add fixmap.h and I thought I tried without too but maybe
not, I will try to adapt it on v4.

> ---
> diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
> index 3a264200c62f..eda7fa856fa9 100644
> --- a/arch/x86/include/asm/pgtable_64_types.h
> +++ b/arch/x86/include/asm/pgtable_64_types.h
> @@ -67,7 +67,7 @@ typedef struct { pteval_t pte; } pte_t;
>  #endif /* CONFIG_RANDOMIZE_MEMORY */
>  #define VMALLOC_END    (VMALLOC_START + _AC((VMALLOC_SIZE_TB << 40) - 1, UL))
>  #define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)
> -#define MODULES_END      _AC(0xffffffffff000000, UL)
> +#define MODULES_END   __fix_to_virt(__end_of_fixed_addresses + 1)
>  #define MODULES_LEN   (MODULES_END - MODULES_VADDR)
>  #define ESPFIX_PGD_ENTRY _AC(-2, UL)
>  #define ESPFIX_BASE_ADDR (ESPFIX_PGD_ENTRY << PGDIR_SHIFT)
> ---
>
> but I wouldn't be surprised if some strange configuration would need it.
>
> >  #define FIXADDR_SIZE (__end_of_permanent_fixed_addresses << PAGE_SHIFT)
> >  #define FIXADDR_START                (FIXADDR_TOP - FIXADDR_SIZE)
> >
> > diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
> > index 3a264200c62f..de8bace10200 100644
> > --- a/arch/x86/include/asm/pgtable_64_types.h
> > +++ b/arch/x86/include/asm/pgtable_64_types.h
> > @@ -66,9 +66,6 @@ typedef struct { pteval_t pte; } pte_t;
> >  #define VMEMMAP_START        __VMEMMAP_BASE
> >  #endif /* CONFIG_RANDOMIZE_MEMORY */
> >  #define VMALLOC_END  (VMALLOC_START + _AC((VMALLOC_SIZE_TB << 40) - 1, UL))
> > -#define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)
> > -#define MODULES_END      _AC(0xffffffffff000000, UL)
>
> How much of an ABI breakage would that be? See
> Documentation/x86/x86_64/mm.txt.
>
> With my .config MODULES_END becomes 0xffffffffff1fe000 and it'll remain
> dynamic depending on .config. No idea how much in userspace relies on
> MODULES_END being static 0xffffffffff000000...
>
> Hmm.
>

Why do you think they rely on it being static? The VSYSCALL address is
not changed for example.

> --
> Regards/Gruss,
>     Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> --

Thanks for the feedback.
Borislav Petkov Feb. 15, 2017, 3:49 p.m. UTC | #2
On Wed, Feb 15, 2017 at 07:40:51AM -0800, Thomas Garnier wrote:
> Why do you think they rely on it being static? The VSYSCALL address is
> not changed for example.

I don't know, that's why I'm asking first. Userspace is known to pick
different aspects of the kernel and deciding to use them for whatever.
And once it relies on that, there's no changing it.
Borislav Petkov Feb. 15, 2017, 9:08 p.m. UTC | #3
On Wed, Feb 15, 2017 at 04:49:21PM +0100, Borislav Petkov wrote:
> On Wed, Feb 15, 2017 at 07:40:51AM -0800, Thomas Garnier wrote:
> > Why do you think they rely on it being static? The VSYSCALL address is
> > not changed for example.
> 
> I don't know, that's why I'm asking first. Userspace is known to pick
> different aspects of the kernel and deciding to use them for whatever.
> And once it relies on that, there's no changing it.

Ok, so we should be fine here because those addresses are internal to
the kernel, I'm being told, so anything using them will break and gets
to keep the pieces even.
diff mbox

Patch

diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
index 3a264200c62f..eda7fa856fa9 100644
--- a/arch/x86/include/asm/pgtable_64_types.h
+++ b/arch/x86/include/asm/pgtable_64_types.h
@@ -67,7 +67,7 @@  typedef struct { pteval_t pte; } pte_t;
 #endif /* CONFIG_RANDOMIZE_MEMORY */
 #define VMALLOC_END	(VMALLOC_START + _AC((VMALLOC_SIZE_TB << 40) - 1, UL))
 #define MODULES_VADDR    (__START_KERNEL_map + KERNEL_IMAGE_SIZE)
-#define MODULES_END      _AC(0xffffffffff000000, UL)
+#define MODULES_END   __fix_to_virt(__end_of_fixed_addresses + 1)
 #define MODULES_LEN   (MODULES_END - MODULES_VADDR)
 #define ESPFIX_PGD_ENTRY _AC(-2, UL)
 #define ESPFIX_BASE_ADDR (ESPFIX_PGD_ENTRY << PGDIR_SHIFT)