From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
will.deacon@arm.com, labbott@fedoraproject.org,
kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com,
andre.przywara@arm.com, Suzuki.Poulose@arm.com,
james.morse@arm.com, keescook@chromium.org,
kernel-hardening@lists.openwall.com, nd@arm.com
Subject: [kernel-hardening] Re: [PATCH v2 2/5] arm64: mmu: move TLB maintenance from callers to create_mapping_late()
Date: Tue, 14 Feb 2017 15:54:36 +0000 [thread overview]
Message-ID: <20170214155436.GC23718@leverpostej> (raw)
In-Reply-To: <1486844586-26135-3-git-send-email-ard.biesheuvel@linaro.org>
On Sat, Feb 11, 2017 at 08:23:03PM +0000, Ard Biesheuvel wrote:
> In preparation of changing the way we invoke create_mapping_late() (which
> is currently invoked twice from the same function), move the TLB flushing
> it performs from the caller into create_mapping_late() itself, and change
> it to a TLB maintenance by VA rather than a full flush, which is more
> appropriate here.
It's not immediately clear what's meant by "changing the way we invoke
create_mapping_late()" here.
It's probably worth explicitly mentioning that we need to add another
caller of create_mapping_late(), and this saves us adding (overly
strong) TLB maintenance to all callers.
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/mm/mmu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 2131521ddc24..9e0ec1a8cd3b 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -356,6 +356,9 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
>
> __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
> NULL, debug_pagealloc_enabled());
> +
> + /* flush the TLBs after updating live kernel mappings */
> + flush_tlb_kernel_range(virt, virt + size);
> }
It feels a little odd to have the maintenance here given we still call
this *create*_mapping_late.
Given the only users of this are changing permissions, perhaps we should
rename this to change_mapping_prot(), or something like that?
Otherwise, this looks fine to me, and boots fine. Either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> @@ -438,9 +441,6 @@ void mark_rodata_ro(void)
> create_mapping_late(__pa_symbol(__start_rodata), (unsigned long)__start_rodata,
> section_size, PAGE_KERNEL_RO);
>
> - /* flush the TLBs after updating live kernel mappings */
> - flush_tlb_all();
> -
> debug_checkwx();
> }
>
> --
> 2.7.4
>
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: keescook@chromium.org, marc.zyngier@arm.com,
catalin.marinas@arm.com, kernel-hardening@lists.openwall.com,
will.deacon@arm.com, andre.przywara@arm.com, nd@arm.com,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org, labbott@fedoraproject.org
Subject: Re: [PATCH v2 2/5] arm64: mmu: move TLB maintenance from callers to create_mapping_late()
Date: Tue, 14 Feb 2017 15:54:36 +0000 [thread overview]
Message-ID: <20170214155436.GC23718@leverpostej> (raw)
In-Reply-To: <1486844586-26135-3-git-send-email-ard.biesheuvel@linaro.org>
On Sat, Feb 11, 2017 at 08:23:03PM +0000, Ard Biesheuvel wrote:
> In preparation of changing the way we invoke create_mapping_late() (which
> is currently invoked twice from the same function), move the TLB flushing
> it performs from the caller into create_mapping_late() itself, and change
> it to a TLB maintenance by VA rather than a full flush, which is more
> appropriate here.
It's not immediately clear what's meant by "changing the way we invoke
create_mapping_late()" here.
It's probably worth explicitly mentioning that we need to add another
caller of create_mapping_late(), and this saves us adding (overly
strong) TLB maintenance to all callers.
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/mm/mmu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 2131521ddc24..9e0ec1a8cd3b 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -356,6 +356,9 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
>
> __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
> NULL, debug_pagealloc_enabled());
> +
> + /* flush the TLBs after updating live kernel mappings */
> + flush_tlb_kernel_range(virt, virt + size);
> }
It feels a little odd to have the maintenance here given we still call
this *create*_mapping_late.
Given the only users of this are changing permissions, perhaps we should
rename this to change_mapping_prot(), or something like that?
Otherwise, this looks fine to me, and boots fine. Either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> @@ -438,9 +441,6 @@ void mark_rodata_ro(void)
> create_mapping_late(__pa_symbol(__start_rodata), (unsigned long)__start_rodata,
> section_size, PAGE_KERNEL_RO);
>
> - /* flush the TLBs after updating live kernel mappings */
> - flush_tlb_all();
> -
> debug_checkwx();
> }
>
> --
> 2.7.4
>
WARNING: multiple messages have this Message-ID (diff)
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/5] arm64: mmu: move TLB maintenance from callers to create_mapping_late()
Date: Tue, 14 Feb 2017 15:54:36 +0000 [thread overview]
Message-ID: <20170214155436.GC23718@leverpostej> (raw)
In-Reply-To: <1486844586-26135-3-git-send-email-ard.biesheuvel@linaro.org>
On Sat, Feb 11, 2017 at 08:23:03PM +0000, Ard Biesheuvel wrote:
> In preparation of changing the way we invoke create_mapping_late() (which
> is currently invoked twice from the same function), move the TLB flushing
> it performs from the caller into create_mapping_late() itself, and change
> it to a TLB maintenance by VA rather than a full flush, which is more
> appropriate here.
It's not immediately clear what's meant by "changing the way we invoke
create_mapping_late()" here.
It's probably worth explicitly mentioning that we need to add another
caller of create_mapping_late(), and this saves us adding (overly
strong) TLB maintenance to all callers.
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/mm/mmu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 2131521ddc24..9e0ec1a8cd3b 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -356,6 +356,9 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
>
> __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
> NULL, debug_pagealloc_enabled());
> +
> + /* flush the TLBs after updating live kernel mappings */
> + flush_tlb_kernel_range(virt, virt + size);
> }
It feels a little odd to have the maintenance here given we still call
this *create*_mapping_late.
Given the only users of this are changing permissions, perhaps we should
rename this to change_mapping_prot(), or something like that?
Otherwise, this looks fine to me, and boots fine. Either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> @@ -438,9 +441,6 @@ void mark_rodata_ro(void)
> create_mapping_late(__pa_symbol(__start_rodata), (unsigned long)__start_rodata,
> section_size, PAGE_KERNEL_RO);
>
> - /* flush the TLBs after updating live kernel mappings */
> - flush_tlb_all();
> -
> debug_checkwx();
> }
>
> --
> 2.7.4
>
next prev parent reply other threads:[~2017-02-14 15:54 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-11 20:23 [kernel-hardening] [PATCH v2 0/5] arm64: mmu: avoid writeable-executable mappings Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-11 20:23 ` [kernel-hardening] [PATCH v2 1/5] arm: kvm: move kvm_vgic_global_state out of .text section Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-13 17:58 ` [kernel-hardening] " Mark Rutland
2017-02-13 17:58 ` Mark Rutland
2017-02-11 20:23 ` [kernel-hardening] [PATCH v2 2/5] arm64: mmu: move TLB maintenance from callers to create_mapping_late() Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-14 15:54 ` Mark Rutland [this message]
2017-02-14 15:54 ` Mark Rutland
2017-02-14 15:54 ` Mark Rutland
2017-02-11 20:23 ` [kernel-hardening] [PATCH v2 3/5] arm64: alternatives: apply boot time fixups via the linear mapping Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-14 15:56 ` [kernel-hardening] " Mark Rutland
2017-02-14 15:56 ` Mark Rutland
2017-02-14 15:56 ` Mark Rutland
2017-02-11 20:23 ` [kernel-hardening] [PATCH v2 4/5] arm64: mmu: map .text as read-only from the outset Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-14 15:57 ` [kernel-hardening] " Mark Rutland
2017-02-14 15:57 ` Mark Rutland
2017-02-14 15:57 ` Mark Rutland
2017-02-14 16:15 ` [kernel-hardening] " Ard Biesheuvel
2017-02-14 16:15 ` Ard Biesheuvel
2017-02-14 16:15 ` Ard Biesheuvel
2017-02-14 17:40 ` [kernel-hardening] " Mark Rutland
2017-02-14 17:40 ` Mark Rutland
2017-02-14 17:40 ` Mark Rutland
2017-02-14 17:49 ` [kernel-hardening] " Ard Biesheuvel
2017-02-14 17:49 ` Ard Biesheuvel
2017-02-14 17:49 ` Ard Biesheuvel
2017-02-14 17:54 ` [kernel-hardening] " Mark Rutland
2017-02-14 17:54 ` Mark Rutland
2017-02-14 17:54 ` Mark Rutland
2017-02-11 20:23 ` [kernel-hardening] [PATCH v2 5/5] arm64: mmu: apply strict permissions to .init.text and .init.data Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-11 20:23 ` Ard Biesheuvel
2017-02-14 15:57 ` [kernel-hardening] " Mark Rutland
2017-02-14 15:57 ` Mark Rutland
2017-02-14 15:57 ` Mark Rutland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170214155436.GC23718@leverpostej \
--to=mark.rutland@arm.com \
--cc=Suzuki.Poulose@arm.com \
--cc=andre.przywara@arm.com \
--cc=ard.biesheuvel@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=labbott@fedoraproject.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=marc.zyngier@arm.com \
--cc=nd@arm.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.