On Sat, Feb 11, 2017 at 08:23:04PM +0000, Ard Biesheuvel wrote:
> One important rule of thumb when desiging a secure software system is
> that memory should never be writable and executable at the same time.
> We mostly adhere to this rule in the kernel, except at boot time, when
> regions may be mapped RWX until after we are done applying alternatives
> or making other one-off changes.
> 
> For the alternative patching, we can improve the situation by applying
> the fixups via the linear mapping, which is never mapped with executable
> permissions. So map the linear alias of .text with RW- permissions
> initially, and remove the write permissions as soon as alternative
> patching has completed.
> 
> Reviewed-by: Laura Abbott <labb...@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>

Reviewed-by: Mark Rutland <mark.rutl...@arm.com>
Tested-by: Mark Rutland <mark.rutl...@arm.com>

Mark.

> ---
>  arch/arm64/include/asm/mmu.h    |  1 +
>  arch/arm64/kernel/alternative.c |  2 +-
>  arch/arm64/kernel/smp.c         |  1 +
>  arch/arm64/mm/mmu.c             | 22 +++++++++++++++-----
>  4 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 47619411f0ff..5468c834b072 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -37,5 +37,6 @@ extern void create_pgd_mapping(struct mm_struct *mm, 
> phys_addr_t phys,
>                              unsigned long virt, phys_addr_t size,
>                              pgprot_t prot, bool page_mappings_only);
>  extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
> +extern void mark_linear_text_alias_ro(void);
>  
>  #endif
> diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
> index 06d650f61da7..8cee29d9bc07 100644
> --- a/arch/arm64/kernel/alternative.c
> +++ b/arch/arm64/kernel/alternative.c
> @@ -128,7 +128,7 @@ static void __apply_alternatives(void *alt_region)
>  
>               for (i = 0; i < nr_inst; i++) {
>                       insn = get_alt_insn(alt, origptr + i, replptr + i);
> -                     *(origptr + i) = cpu_to_le32(insn);
> +                     ((u32 *)lm_alias(origptr))[i] = cpu_to_le32(insn);
>               }
>  
>               flush_icache_range((uintptr_t)origptr,
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index a8ec5da530af..d6307e311a10 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -432,6 +432,7 @@ void __init smp_cpus_done(unsigned int max_cpus)
>       setup_cpu_features();
>       hyp_mode_check();
>       apply_alternatives_all();
> +     mark_linear_text_alias_ro();
>  }
>  
>  void __init smp_prepare_boot_cpu(void)
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9e0ec1a8cd3b..7ed981c7f4c0 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -398,16 +398,28 @@ static void __init __map_memblock(pgd_t *pgd, 
> phys_addr_t start, phys_addr_t end
>                                    debug_pagealloc_enabled());
>  
>       /*
> -      * Map the linear alias of the [_text, __init_begin) interval as
> -      * read-only/non-executable. This makes the contents of the
> -      * region accessible to subsystems such as hibernate, but
> -      * protects it from inadvertent modification or execution.
> +      * Map the linear alias of the [_text, __init_begin) interval
> +      * as non-executable now, and remove the write permission in
> +      * mark_linear_text_alias_ro() below (which will be called after
> +      * alternative patching has completed). This makes the contents
> +      * of the region accessible to subsystems such as hibernate,
> +      * but protects it from inadvertent modification or execution.
>        */
>       __create_pgd_mapping(pgd, kernel_start, __phys_to_virt(kernel_start),
> -                          kernel_end - kernel_start, PAGE_KERNEL_RO,
> +                          kernel_end - kernel_start, PAGE_KERNEL,
>                            early_pgtable_alloc, debug_pagealloc_enabled());
>  }
>  
> +void __init mark_linear_text_alias_ro(void)
> +{
> +     /*
> +      * Remove the write permissions from the linear alias of .text/.rodata
> +      */
> +     create_mapping_late(__pa_symbol(_text), (unsigned long)lm_alias(_text),
> +                         (unsigned long)__init_begin - (unsigned long)_text,
> +                         PAGE_KERNEL_RO);
> +}
> +
>  static void __init map_mem(pgd_t *pgd)
>  {
>       struct memblock_region *reg;
> -- 
> 2.7.4
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to