[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <6347B1C6-343A-47CF-9A2D-0DD749BE95B5@linaro.org>
Date: Fri, 10 Feb 2017 18:53:38 +0000
From: Ard Biesheuvel <ard.biesheuvel@...aro.org>
To: Suzuki K Poulose <Suzuki.Poulose@....com>
Cc: linux-arm-kernel@...ts.infradead.org, mark.rutland@....com,
will.deacon@....com, catalin.marinas@....com, keescook@...omium.org,
labbott@...oraproject.org, james.morse@....com, marc.zyngier@....com,
andre.przywara@....com, kernel-hardening@...ts.openwall.com,
kvmarm@...ts.cs.columbia.edu
Subject: Re: [PATCH 2/4] arm64: alternatives: apply boot time fixups via the linear mapping
> On 10 Feb 2017, at 18:49, Suzuki K Poulose <Suzuki.Poulose@....com> wrote:
>
>> On 10/02/17 17:16, Ard Biesheuvel wrote:
>> One important rule of thumb when designing 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.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>> ---
>> arch/arm64/include/asm/mmu.h | 1 +
>> arch/arm64/kernel/alternative.c | 6 ++---
>> arch/arm64/kernel/smp.c | 1 +
>> arch/arm64/mm/mmu.c | 25 ++++++++++++++++----
>> 4 files changed, 25 insertions(+), 8 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..eacdbcc45630 100644
>> --- a/arch/arm64/kernel/alternative.c
>> +++ b/arch/arm64/kernel/alternative.c
>> @@ -122,7 +122,7 @@ static void __apply_alternatives(void *alt_region)
>>
>> pr_info_once("patching kernel code\n");
>>
>> - origptr = ALT_ORIG_PTR(alt);
>> + origptr = lm_alias(ALT_ORIG_PTR(alt));
>> replptr = ALT_REPL_PTR(alt);
>> nr_inst = alt->alt_len / sizeof(insn);
>
> Correct me if I am wrong, I think this would make "get_alt_insn" generate branch
> instructions based on the aliased linear mapped address, which could branch to linear
> address of the branch target which doesn't have Execute permissions set.
> I think we sould use ALT_ORIG_PTR(alt), instead of origptr for the calls to
> get_alt_insn().
>
Good point, you are probably right.
Will fix
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
