This series implements KASLR for arm64, by building the kernel as a PIE
executable that can relocate itself at runtime, and moving it to a random
offset in the vmalloc area. v2 and up also implement physical randomization,
i.e., it allows the kernel to deal with being loaded at any physical offset
(modulo the required alignment), and invokes the EFI_RNG_PROTOCOL from the
UEFI stub to obtain random bits and perform the actual randomization of the
physical load address.
Changes since v2:
- Incorporated feedback from Marc Zyngier into the KVM patch (#5)
- Dropped the pgdir section and the patch that memblock_reserve()'s the kernel
sections at a smaller granularity. This is no longer necessary with the pgdir
section gone. This also fixes an issue spotted by James Morse where the fixmap
page tables are not zeroed correctly; these have been moved back to the .bss
section.
- Got rid of all ifdef'ery regarding the number of translation levels in the
changed .c files, by introducing new definitions in pgtable.h (#3, #6)
- Fixed KAsan support, which was broken by all earlier versions.
- Moved module region along with the virtually randomized kernel, so that module
addresses become unpredictable as well, and we only have to rely on veneers in
the PLTs when the module region is exhausted (which is somewhat more likely
since the module region is now shared with other uses of the vmalloc area)
- Added support for the 'nokaslr' command line option. This affects the
randomization performed by the stub, and results in a warning if passed while
the bootloader also presented a random seed for virtual KASLR in register x1.
- The .text/.rodata sections of the kernel are no longer aliased in the linear
region with a writable mapping.
- Added a separate image header flag for kernel images that may be loaded at any
2 MB aligned offset (+ TEXT_OFFSET)
- The KASLR displacement is now corrected if it results in the kernel image
intersecting a PUD/PMD boundary (4k and 16k/64k granule kernels, respectively)
- Split out UEFI stub random routines into separate patches.
- Implemented a weight based EFI random allocation routine so that each suitable
offset in available memory is equally likely to be selected (as suggested by
Kees Cook)
- Reused CONFIG_RELOCATABLE and CONFIG_RANDOMIZE_BASE instead of introducing
new Kconfig symbols to describe the same functionality.
- Reimplemented mem= logic so memory is clipped from the top first.
Changes since v1/RFC:
- This series now implements fully independent virtual and physical address
randomization at load time. I have recycled some patches from this series:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/455151, and updated the
final UEFI stub patch to randomize the physical address as well.
- Added a patch to deal with the way KVM on arm64 makes assumptions about the
relation between kernel symbols and the linear mapping (on which the HYP
mapping is based), as these assumptions cease to be valid once we move the
kernel Image out of the linear mapping.
- Updated the module PLT patch so it works on BE kernels as well.
- Moved the constant Image header values to head.S, and updated the linker
script to provide the kernel size using R_AARCH64_ABS32 relocation rather
than a R_AARCH64_ABS64 relocation, since those are always resolved at build
time. This allows me to get rid of the post-build perl script to swab header
values on BE kernels.
- Minor style tweaks.
Notes:
- These patches apply on top of Mark Rutland's pagetable rework series:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/462438
- The arm64 Image is uncompressed by default, and the Elf64_Rela format uses
24 bytes per relocation entry. This results in considerable bloat (i.e., a
couple of MBs worth of relocation data in an .init section). However, no
build time postprocessing is required, we rely fully on the toolchain to
produce the image
- We have to rely on the bootloader to supply some randomness in register x1
upon kernel entry. Since we have no decompressor, it is simply not feasible
to collect randomness in the head.S code path before mapping the kernel and
enabling the MMU.
- The EFI_RNG_PROTOCOL that is invoked in patch #13 to supply randomness on
UEFI systems is not universally available. A QEMU/KVM firmware image that
implements a pseudo-random version is available here:
http://people.linaro.org/~ard.biesheuvel/QEMU_EFI.fd.aarch64-rng.bz2
(requires access to PMCCNTR_EL0 and support for AES instructions)
See below for instructions how to run the pseudo-random version on real
hardware.
- Only mildly tested. Help appreciated.
Code can be found here:
git://git.linaro.org/people/ard.biesheuvel/linux-arm.git arm64-kaslr-v3
https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/shortlog/refs/heads/arm64-kaslr-v3
Patch #1 updates the OF code to allow the minimum memblock physical address to
be overridden by the arch.
Patch #2 introduces KIMAGE_VADDR as the base of the kernel virtual region.
Patch #3 introduces dummy pud_index() and pmd_index() macros that are intended
to be optimized away if the configured number of translation levels does not
actually use them.
Patch #4 rewrites early_fixmap_init() so it does not rely on the linear mapping
(i.e., the use of phys_to_virt() is avoided)
Patch #5 updates KVM on arm64 so it can deal with kernel symbols whose addresses
are not covered by the linear mapping.
Patch #6 introduces pte_offset_kimg(), pmd_offset_kimg() and pud_offset_kimg()
that allow statically allocated page tables (i.e., by fixmap and kasan) to be
traversed before the linear mapping is installed.
Patch #7 moves the kernel virtual mapping to the vmalloc area, along with the
module region which is kept right below it, as before.
Patch #8 adds support for PLTs in modules so that relative branches can be
resolved via a PLT if the target is out of range. This is required for KASLR,
since modules may be loaded far away from the core kernel.
Patch #9 and #10 move arm64 to the a new generic relative version of the extable
implementation so that it no longer contains absolute addresses that require
fixing up at relocation time, but uses relative offsets instead.
Patch #11 reverts some changes to the Image header population code so we no
longer depend on the linker to populate the header fields. This is necessary
since the R_AARCH64_ABS64 relocations that are emitted for these fields are not
resolved at build time for PIE executables.
Patch #12 updates the code in head.S that needs to execute before relocation to
avoid the use of values that are subject to dynamic relocation. These values
will not be populated in PIE executables.
Patch #13 allows the kernel Image to be loaded anywhere in physical memory, by
decoupling PHYS_OFFSET from the base of the kernel image.
Patch #14 redefines SWAPPER_TABLE_SHIFT in a way that allows it to be used from
assembler code regardless of the number of configured translation levels.
Patch #15 (from Mark Rutland) moves the ELF relocation type #defines to a
separate file so we can use it from head.S later
Patch #16 updates scripts/sortextable.c so it accepts ET_DYN (relocatable)
executables as well as ET_EXEC (static) executables.
Patch #17 implements the core KASLR, by taking randomness supplied in register
x1 and using it to move the kernel inside the vmalloc area.
Patch #18 implements efi_get_random_bytes() based on the EFI_RNG_PROTOCOL
Patch #19 implements efi_random_alloc()
Patch #20 moves the allocation for the converted command line (UTF-16 to ASCII)
away from the base of memory. This is necessary since for parsing
Patch #21 implements the actual KASLR, by randomizing the kernel physical
address, and passing entropy in x1 so that the kernel proper can relocate itself
virtually.
Ard Biesheuvel (20):
of/fdt: make memblock minimum physical address arch configurable
arm64: introduce KIMAGE_VADDR as the virtual base of the kernel region
arm64: pgtable: add dummy pud_index() and pmd_index() definitions
arm64: decouple early fixmap init from linear mapping
arm64: kvm: deal with kernel symbols outside of linear mapping
arm64: pgtable: implement static [pte|pmd|pud]_offset variants
arm64: move kernel image to base of vmalloc area
arm64: add support for module PLTs
extable: add support for relative extables to search and sort routines
arm64: switch to relative exception tables
arm64: avoid R_AARCH64_ABS64 relocations for Image header fields
arm64: avoid dynamic relocations in early boot code
arm64: allow kernel Image to be loaded anywhere in physical memory
arm64: redefine SWAPPER_TABLE_SHIFT for use in asm code
scripts/sortextable: add support for ET_DYN binaries
arm64: add support for a relocatable kernel and KASLR
efi: stub: implement efi_get_random_bytes() based on EFI_RNG_PROTOCOL
efi: stub: add implementation of efi_random_alloc()
efi: stub: use high allocation for converted command line
arm64: efi: invoke EFI_RNG_PROTOCOL to supply KASLR randomness
Mark Rutland (1):
arm64: split elf relocs into a separate header.
Documentation/arm64/booting.txt | 34 ++++-
arch/arm/include/asm/kvm_asm.h | 2 +
arch/arm/include/asm/kvm_mmu.h | 2 +
arch/arm/kvm/arm.c | 5 +-
arch/arm/kvm/mmu.c | 8 +-
arch/arm64/Kconfig | 40 +++++
arch/arm64/Makefile | 10 +-
arch/arm64/include/asm/assembler.h | 30 +++-
arch/arm64/include/asm/boot.h | 6 +
arch/arm64/include/asm/elf.h | 54 +------
arch/arm64/include/asm/elf_relocs.h | 75 ++++++++++
arch/arm64/include/asm/futex.h | 12 +-
arch/arm64/include/asm/kasan.h | 20 +--
arch/arm64/include/asm/kernel-pgtable.h | 20 ++-
arch/arm64/include/asm/kvm_asm.h | 19 ++-
arch/arm64/include/asm/kvm_host.h | 8 +-
arch/arm64/include/asm/kvm_mmu.h | 2 +
arch/arm64/include/asm/memory.h | 38 +++--
arch/arm64/include/asm/module.h | 11 ++
arch/arm64/include/asm/pgtable.h | 22 ++-
arch/arm64/include/asm/uaccess.h | 30 ++--
arch/arm64/include/asm/virt.h | 4 -
arch/arm64/include/asm/word-at-a-time.h | 7 +-
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/armv8_deprecated.c | 7 +-
arch/arm64/kernel/efi-entry.S | 9 +-
arch/arm64/kernel/head.S | 155 +++++++++++++++++---
arch/arm64/kernel/image.h | 37 ++---
arch/arm64/kernel/module-plts.c | 137 +++++++++++++++++
arch/arm64/kernel/module.c | 15 +-
arch/arm64/kernel/module.lds | 4 +
arch/arm64/kernel/setup.c | 44 +++++-
arch/arm64/kernel/vmlinux.lds.S | 13 +-
arch/arm64/kvm/debug.c | 1 +
arch/arm64/kvm/hyp.S | 6 +-
arch/arm64/mm/dump.c | 12 +-
arch/arm64/mm/extable.c | 2 +-
arch/arm64/mm/init.c | 91 ++++++++++--
arch/arm64/mm/kasan_init.c | 21 ++-
arch/arm64/mm/mmu.c | 95 +++++++-----
arch/x86/include/asm/efi.h | 2 +
drivers/firmware/efi/libstub/Makefile | 2 +-
drivers/firmware/efi/libstub/arm-stub.c | 17 ++-
drivers/firmware/efi/libstub/arm64-stub.c | 67 +++++++--
drivers/firmware/efi/libstub/efi-stub-helper.c | 24 ++-
drivers/firmware/efi/libstub/efistub.h | 9 ++
drivers/firmware/efi/libstub/random.c | 120 +++++++++++++++
drivers/of/fdt.c | 5 +-
include/linux/efi.h | 5 +-
lib/extable.c | 50 +++++--
scripts/sortextable.c | 10 +-
51 files changed, 1111 insertions(+), 309 deletions(-)
create mode 100644 arch/arm64/include/asm/elf_relocs.h
create mode 100644 arch/arm64/kernel/module-plts.c
create mode 100644 arch/arm64/kernel/module.lds
create mode 100644 drivers/firmware/efi/libstub/random.c
EFI_RNG_PROTOCOL on real hardware
=================================
To test whether your UEFI implements the EFI_RNG_PROTOCOL, download the
following executable and run it from the UEFI Shell:
http://people.linaro.org/~ard.biesheuvel/RngTest.efi
FS0:\> rngtest
UEFI RNG Protocol Testing :
----------------------------
-- Locate UEFI RNG Protocol : [Fail - Status = Not Found]
If your UEFI does not implement the EFI_RNG_PROTOCOL, you can download and
install the pseudo-random version that uses the generic timer and PMCCNTR_EL0
values and permutes them using a couple of rounds of AES.
http://people.linaro.org/~ard.biesheuvel/RngDxe.efi
NOTE: not for production!! This is a quick and dirty hack to test the KASLR
code, and is not suitable for anything else.
FS0:\> rngdxe
FS0:\> rngtest
UEFI RNG Protocol Testing :
----------------------------
-- Locate UEFI RNG Protocol : [Pass]
-- Call RNG->GetInfo() interface :
>> Supported RNG Algorithm (Count = 2) :
0) 44F0DE6E-4D8C-4045-A8C7-4DD168856B9E
1) E43176D7-B6E8-4827-B784-7FFDC4B68561
-- Call RNG->GetRNG() interface :
>> RNG with default algorithm : [Pass]
>> RNG with SP800-90-HMAC-256 : [Fail - Status = Unsupported]
>> RNG with SP800-90-Hash-256 : [Fail - Status = Unsupported]
>> RNG with SP800-90-CTR-256 : [Pass]
>> RNG with X9.31-3DES : [Fail - Status = Unsupported]
>> RNG with X9.31-AES : [Fail - Status = Unsupported]
>> RNG with RAW Entropy : [Pass]
-- Random Number Generation Test with default RNG Algorithm (20 Rounds):
01) - 27
02) - 61E8
03) - 496FD8
04) - DDD793BF
05) - B6C37C8E23
06) - 4D183C604A96
07) - 9363311DB61298
08) - 5715A7294F4E436E
09) - F0D4D7BAA0DD52318E
10) - C88C6EBCF4C0474D87C3
11) - B5594602B482A643932172
12) - CA7573F704B2089B726B9CF1
13) - A93E9451CB533DCFBA87B97C33
14) - 45AA7B83DB6044F7BBAB031F0D24
15) - 3DD7A4D61F34ADCB400B5976730DCF
16) - 4DD168D21FAB8F59708330D6A9BEB021
17) - 4BBB225E61C465F174254159467E65939F
18) - 030A156C9616337A20070941E702827DA8E1
19) - AB0FC11C9A4E225011382A9D164D9D55CA2B64
20) - 72B9B4735DC445E5DA6AF88DE965B7E87CB9A23C
Add definitions of pud_index() and pmd_index() for configurations with
fewer than 4 resp. 3 translation levels. This makes it easier to keep
the users (e.g., the fixmap init code) generic.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
This introduces the preprocessor symbol KIMAGE_VADDR which will serve as
the symbolic virtual base of the kernel region, i.e., the kernel's virtual
offset will be KIMAGE_VADDR + TEXT_OFFSET. For now, we define it as being
equal to PAGE_OFFSET, but in the future, it will be moved below it once
we move the kernel virtual mapping out of the linear mapping.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/memory.h | 10 ++++++++--
arch/arm64/kernel/head.S | 2 +-
arch/arm64/kernel/vmlinux.lds.S | 4 ++--
3 files changed, 11 insertions(+), 5 deletions(-)
KVM on arm64 uses a fixed offset between the linear mapping at EL1 and
the HYP mapping at EL2. Before we can move the kernel virtual mapping
out of the linear mapping, we have to make sure that references to kernel
symbols that are accessed via the HYP mapping are translated to their
linear equivalent.
To prevent inadvertent direct references from sneaking in later, change
the type of all extern declarations to HYP kernel symbols to the opaque
'struct kvm_ksym', which does not decay to a pointer type like char arrays
and function references. This is not bullet proof, but at least forces the
user to take the address explicitly rather than referencing it directly.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm/include/asm/kvm_asm.h | 2 ++
arch/arm/include/asm/kvm_mmu.h | 2 ++
arch/arm/kvm/arm.c | 5 +++--
arch/arm/kvm/mmu.c | 8 +++-----
arch/arm64/include/asm/kvm_asm.h | 19 ++++++++++++-------
arch/arm64/include/asm/kvm_host.h | 8 +++++---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
arch/arm64/include/asm/virt.h | 4 ----
arch/arm64/kvm/debug.c | 1 +
arch/arm64/kvm/hyp.S | 6 +++---
10 files changed, 33 insertions(+), 24 deletions(-)
This moves the module area to right before the vmalloc area, and
moves the kernel image to the base of the vmalloc area. This is
an intermediate step towards implementing kASLR, where the kernel
image can be located anywhere in the vmalloc area.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kasan.h | 20 ++++---
arch/arm64/include/asm/kernel-pgtable.h | 5 +-
arch/arm64/include/asm/memory.h | 18 ++++--
arch/arm64/include/asm/pgtable.h | 7 ---
arch/arm64/kernel/setup.c | 12 ++++
arch/arm64/mm/dump.c | 12 ++--
arch/arm64/mm/init.c | 20 +++----
arch/arm64/mm/kasan_init.c | 21 +++++--
arch/arm64/mm/mmu.c | 62 ++++++++++++++------
9 files changed, 118 insertions(+), 59 deletions(-)
@@ -349,14 +353,14 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end{unsignedlongkernel_start=__pa(_stext);-unsignedlongkernel_end=__pa(_end);+unsignedlongkernel_end=__pa(_etext);/*-*Thekernelitselfismappedatpagegranularity.Mapallother-*memory,makingsurewedon'toverwritetheexistingkernelmappings.+*Takecarenottocreateawritablealiasforthe+*read-onlytextandrodatasectionsofthekernelimage.*/-/* No overlap with the kernel. */+/* No overlap with the kernel text */if(end<kernel_start||start>=kernel_end){__create_pgd_mapping(pgd,start,__phys_to_virt(start),end-start,PAGE_KERNEL,
Instead of using absolute addresses for both the exception location
and the fixup, use offsets relative to the exception table entry values.
Not only does this cut the size of the exception table in half, it is
also a prerequisite for KASLR, since absolute exception table entries
are subject to dynamic relocation, which is incompatible with the sorting
of the exception table that occurs at build time.
This patch also introduces the _ASM_EXTABLE preprocessor macro (which
exists on x86 as well) and its _asm_extable assembly counterpart, as
shorthands to emit exception table entries.
Acked-by: Will Deacon <redacted>
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/assembler.h | 15 +++++++---
arch/arm64/include/asm/futex.h | 12 +++-----
arch/arm64/include/asm/uaccess.h | 30 +++++++++++---------
arch/arm64/include/asm/word-at-a-time.h | 7 ++---
arch/arm64/kernel/armv8_deprecated.c | 7 ++---
arch/arm64/mm/extable.c | 2 +-
scripts/sortextable.c | 2 +-
7 files changed, 38 insertions(+), 37 deletions(-)
This adds support to the generic search_extable() and sort_extable()
implementations for dealing with exception table entries whose fields
contain relative offsets rather than absolute addresses.
Acked-by: Helge Deller <deller@gmx.de>
Acked-by: Heiko Carstens <redacted>
Acked-by: H. Peter Anvin <redacted>
Acked-by: Tony Luck <tony.luck@intel.com>
Acked-by: Will Deacon <redacted>
Signed-off-by: Ard Biesheuvel <redacted>
---
lib/extable.c | 50 ++++++++++++++++----
1 file changed, 41 insertions(+), 9 deletions(-)
From: Mark Rutland <mark.rutland@arm.com>
Currently, asm/elf.h contains a mixture of simple constants, C structure
definitions, and some constants defined in terms of constants from other
headers (which are themselves mixtures).
To enable the use of AArch64 ELF reloc constants from assembly code (as
we will need for relocatable kernel support), we need an include without
C structure definitions or includes of other files with such definitions.
This patch factors out the relocs into a new header specifically for ELF
reloc types.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/elf.h | 54 +--------------
arch/arm64/include/asm/elf_relocs.h | 73 ++++++++++++++++++++
2 files changed, 74 insertions(+), 53 deletions(-)
The current definition of SWAPPER_TABLE_SHIFT can only be used in
asm code if the configured number of translation levels defines
PUD_SHIFT and/or PMD_SHIFT natively (4KB and 16KB/64KB granule,
respectively). Otherwise, it depends on the nopmd/nopud fixup
headers, which can only be included in C code.
So redefine SWAPPER_TABLE_SHIFT in a way that is independent of the
number of configured translation levels. Define SWAPPER_TABLE_SIZE
as well, we will need it later.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kernel-pgtable.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
This adds support for runtime relocation of the kernel Image, by
building it as a PIE (ET_DYN) executable and applying the dynamic
relocations in the early boot code.
On top of this, support for KASLR is implemented, based on entropy
provided by the bootloader in register x1 at kernel entry. Depending
on the size of the address space (VA_BITS) and the page size, the
entropy in the virtual displacement is up to 13 bits (16k/2 levels)
and up to 25 bits (all 4 levels), with the caveat that displacements
that result in the kernel image straddling a 1GB/32MB/512MB alignment
boundary (for 4KB/16KB/64KB granule kernels, respectively) are not
allowed.
The same virtual offset is applied to the module region: this gives
almost the same security benefits, and keeps the modules in close
proximity to the kernel so we only have to rely on branches via PLTs
once the module region is exhausted (which is slightly more likely
to occur, as the relocated module region is shared with other uses
of the vmalloc area)
Signed-off-by: Ard Biesheuvel <redacted>
---
Documentation/arm64/booting.txt | 16 +++-
arch/arm64/Kconfig | 26 ++++++
arch/arm64/Makefile | 4 +
arch/arm64/include/asm/elf_relocs.h | 2 +
arch/arm64/include/asm/memory.h | 3 +
arch/arm64/kernel/head.S | 94 +++++++++++++++++++-
arch/arm64/kernel/module.c | 3 +-
arch/arm64/kernel/setup.c | 38 ++++++--
arch/arm64/kernel/vmlinux.lds.S | 9 ++
9 files changed, 180 insertions(+), 15 deletions(-)
@@ -115,13 +115,25 @@ Header notes: accessible 1 - 2MB aligned base may be anywhere in physical memory- Bits 4-63: Reserved.+ Bit 4: Virtual address space layout randomization (KASLR)+ 0 - kernel will execute from a fixed virtual offset+ that is decided at compile time, register x1 should+ be zero at kernel entry+ 1 - kernel will execute from a virtual offset that is+ randomized based on the contents of register x1 at+ kernel entry+ Bits 5-63: Reserved. - When image_size is zero, a bootloader should attempt to keep as much memory as possible free for use by the kernel immediately after the end of the kernel image. The amount of space required will vary depending on selected features, and is effectively unbound.+- It is up to the bootloader to decide whether a KASLR capable kernel should+ boot with randomization enabled. If this is the case, register x1 should+ contain a strong random value. If the bootloader passes 'nokaslr' on the+ kernel command line to disable randomization, it must also pass 0 in x1.+ The Image must be placed text_offset bytes from a 2MB aligned base address anywhere in usable system RAM and called there. The region between the 2 MB aligned base address and the start of the image has no
@@ -145,7 +157,7 @@ Before jumping into the kernel, the following conditions must be met: - Primary CPU general-purpose register settings x0 = physical address of device tree blob (dtb) in system RAM.- x1 = 0 (reserved for future use)+ x1 = 0, unless bit 4 is set in the Image header x2 = 0 (reserved for future use) x3 = 0 (reserved for future use)
@@ -711,6 +711,32 @@ config ARM64_MODULE_PLTSselectARM64_MODULE_CMODEL_LARGEselectHAVE_MOD_ARCH_SPECIFIC+configRELOCATABLE+bool+help+ThisbuildsthekernelasaPositionIndependentExecutable(PIE),+whichretainsallrelocationmetadatarequiredtorelocatethe+kernelbinaryatruntimetoadifferentvirtualaddressthanthe+addressitwaslinkedat.+SinceAArch64usestheRELArelocationformat,thisrequiresa+relocationpassatruntimeevenifthekernelisloadedatthe+sameaddressitwaslinkedat.++configRANDOMIZE_BASE+bool"Randomize the address of the kernel image"+selectARM64_MODULE_PLTS+selectRELOCATABLE+help+Randomizesthevirtualaddressatwhichthekernelimageis+loaded,asasecurityfeaturethatdetersexploitattempts+relyingonknowledgeofthelocationofkernelinternals.++Itisthebootloader'sjobtoprovideentropy,bypassinga+randomvalueinx1atkernelentry.++Ifunsure,sayN.++endmenumenu"Boot options"
@@ -15,6 +15,10 @@ CPPFLAGS_vmlinux.lds = -DTEXT_OFFSET=$(TEXT_OFFSET)OBJCOPYFLAGS:=-Obinary-R.note-R.note.gnu.build-id-R.comment-SGZFLAGS:=-9+ifneq ($(CONFIG_RELOCATABLE),)+LDFLAGS_vmlinux+=-pie+endif+KBUILD_DEFCONFIG:=defconfig# Check for binutils support for specific extensions
@@ -122,6 +122,9 @@ extern phys_addr_t memstart_addr;/* PHYS_OFFSET - the physical address of the start of memory. */#define PHYS_OFFSET ({ memstart_addr; })+/* the virtual base of the kernel image (minus TEXT_OFFSET) */+externu64kimage_vaddr;+/* the offset between the kernel virtual and physical mappings */externu64kimage_voffset;
@@ -468,6 +552,10 @@ ENDPROC(__mmap_switched)*hotplugandneedstohavethesameprotectionsasthetextregion*/.section".text","ax"++ENTRY(kimage_vaddr)+.quad_text-TEXT_OFFSET+/**Ifwe're fortunate enough to boot@EL2, ensure that the world is*sanebeforedroppingtoEL1.
@@ -288,16 +288,41 @@ static inline void __init relocate_initrd(void)}#endif+staticboolnokaslr;+staticint__initearly_nokaslr(char*p)+{+nokaslr=true;+return0;+}+early_param("nokaslr",early_nokaslr);++staticvoidcheck_boot_args(void)+{+if((!IS_ENABLED(CONFIG_RANDOMIZE_BASE)&&boot_args[1])||+boot_args[2]||boot_args[3]){+pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"+"\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"+"This indicates a broken bootloader or old kernel\n",+boot_args[1],boot_args[2],boot_args[3]);+}+if(IS_ENABLED(CONFIG_RANDOMIZE_BASE)&&boot_args[1]&&nokaslr){+pr_err("WARNING: found KASLR entropy in x1 but 'nokaslr' was passed on the commmand line:\n"+"\tx1: %016llx\n"+"This indicates a broken bootloader\n",+boot_args[1]);+}+}+u64__cpu_logical_map[NR_CPUS]={[0...NR_CPUS-1]=INVALID_HWID};void__initsetup_arch(char**cmdline_p){staticstructvm_structvmlinux_vm;-vmlinux_vm.addr=(void*)KIMAGE_VADDR;-vmlinux_vm.size=round_up((u64)_end-KIMAGE_VADDR,+vmlinux_vm.addr=(void*)kimage_vaddr;+vmlinux_vm.size=round_up((u64)_end-kimage_vaddr,SWAPPER_BLOCK_SIZE);-vmlinux_vm.phys_addr=__pa(KIMAGE_VADDR);+vmlinux_vm.phys_addr=__pa(kimage_vaddr);vmlinux_vm.flags=VM_MAP;vmlinux_vm.caller=setup_arch;
@@ -366,12 +391,7 @@ void __init setup_arch(char **cmdline_p)conswitchp=&dummy_con;#endif#endif-if(boot_args[1]||boot_args[2]||boot_args[3]){-pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"-"\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"-"This indicates a broken bootloader or old kernel\n",-boot_args[1],boot_args[2],boot_args[3]);-}+check_boot_args();}staticint__initarm64_device_init(void)
Since arm64 does not use a decompressor that supplies an execution
environment where it is feasible to some extent to provide a source of
randomness, the arm64 KASLR kernel depends on the bootloader to supply
some random bits in register x1 upon kernel entry.
On UEFI systems, we can use the EFI_RNG_PROTOCOL, if supplied, to obtain
some random bits. At the same time, use it to randomize the offset of the
kernel Image in physical memory.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/Kconfig | 5 ++
arch/arm64/kernel/efi-entry.S | 7 +-
drivers/firmware/efi/libstub/arm-stub.c | 17 ++---
drivers/firmware/efi/libstub/arm64-stub.c | 67 +++++++++++++++-----
drivers/firmware/efi/libstub/efi-stub-helper.c | 10 +++
drivers/firmware/efi/libstub/efistub.h | 2 +
6 files changed, 82 insertions(+), 26 deletions(-)
@@ -207,14 +207,6 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,pr_efi_err(sys_table,"Failed to find DRAM base\n");gotofail;}-status=handle_kernel_image(sys_table,image_addr,&image_size,-&reserve_addr,-&reserve_size,-dram_base,image);-if(status!=EFI_SUCCESS){-pr_efi_err(sys_table,"Failed to relocate kernel\n");-gotofail;-}/**GetthecommandlinefromEFI,usingtheLOADED_IMAGE
@@ -231,6 +223,15 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,if(status!=EFI_SUCCESS)pr_efi_err(sys_table,"Failed to parse EFI cmdline options\n");+status=handle_kernel_image(sys_table,image_addr,&image_size,+&reserve_addr,+&reserve_size,+dram_base,image);+if(status!=EFI_SUCCESS){+pr_efi_err(sys_table,"Failed to relocate kernel\n");+gotofail;+}+/**Unauthenticateddevicetreedataisasecurityhazard,so*ignore'dtb='unlessUEFISecureBootisdisabled.
Before we can move the command line processing before the allocation
of the kernel, which is required for detecting the 'nokaslr' option
which controls that allocation, move the converted command line higher
up in memory, to prevent it from interfering with the kernel itself.
Since x86 needs the address to fit in 32 bits, use UINT_MAX as the upper
bound there. Otherwise, use ULONG_MAX (i.e., no limit)
Cc: Matt Fleming <redacted>
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/x86/include/asm/efi.h | 2 ++
drivers/firmware/efi/libstub/efi-stub-helper.c | 14 +++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
This implements efi_random_alloc(), which allocates a chunk of memory of
a certain size at a certain alignment, and uses the random_seed argument
it receives to randomize the offset of the allocation.
This is implemented by iterating over the UEFI memory map, counting the
number of suitable slots (aligned offsets) within each region, and picking
a random number between 0 and 'number of slots - 1' to select the slot,
This should guarantee that each possible offset is chosen equally likely.
Suggested-by: Kees Cook <redacted>
Cc: Matt Fleming <redacted>
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/efistub.h | 4 +
drivers/firmware/efi/libstub/random.c | 85 ++++++++++++++++++++
2 files changed, 89 insertions(+)
@@ -33,3 +33,88 @@ efi_status_t efi_get_random_bytes(efi_system_table_t *sys_table,returnrng->get_rng(rng,NULL,size,out);}++/*+*Returnaweightforamemoryentrydependingonhowmanyoffsetsitcovers+*thataresuitablyalignedandsupplyenoughroomfortheallocation.+*/+staticunsignedlongget_entry_weight(efi_memory_desc_t*md,unsignedlongsize,+unsignedlongalign_bits)+{+u64start,end;++if(md->type!=EFI_CONVENTIONAL_MEMORY)+return0;++if(!(md->attribute&EFI_MEMORY_WB))+return0;++start=round_up(md->phys_addr,1<<align_bits);+end=round_down(md->phys_addr+md->num_pages*EFI_PAGE_SIZE-size,+1<<align_bits);++if(start>=end)+return0;++return(end-start)>>align_bits;+}++/*+*TheUEFImemorydescriptorshaveavirtualaddressfieldthatisonlyused+*wheninstallingthevirtualmappingusingSetVirtualAddressMap().Sinceit+*isunusedhere,wecanreuseittokeeptrackofeachdescriptor'sweight.+*/+#define MD_WEIGHT(md) ((md)->virt_addr)++efi_status_tefi_random_alloc(efi_system_table_t*sys_table_arg,+unsignedlongsize,unsignedlongalign_bits,+unsignedlong*addr,unsignedlongrandom_seed)+{+unsignedlongmap_size,desc_size,max_weight=0,target;+efi_memory_desc_t*memory_map;+efi_status_tstatus=EFI_NOT_FOUND;+intl;++status=efi_get_memory_map(sys_table_arg,&memory_map,&map_size,+&desc_size,NULL,NULL);+if(status!=EFI_SUCCESS)+returnstatus;++/* assign each entry in the memory map a weight */+for(l=0;l<map_size;l+=desc_size){+efi_memory_desc_t*md=(void*)memory_map+l;+unsignedlongweight;++weight=get_entry_weight(md,size,align_bits);+MD_WEIGHT(md)=weight;+max_weight+=weight;+}++/* find a random number between 0 and max_weight */+target=(max_weight*(u16)random_seed)>>16;++/* find the entry whose accumulated weight covers the target */+for(l=0;l<map_size;l+=desc_size){+efi_memory_desc_t*md=(void*)memory_map+l;++if(target<MD_WEIGHT(md)){+unsignedlongpages;++*addr=round_up(md->phys_addr,1<<align_bits)++(target<<align_bits);+pages=round_up(size,EFI_PAGE_SIZE)/EFI_PAGE_SIZE;++status=efi_call_early(allocate_pages,+EFI_ALLOCATE_ADDRESS,+EFI_LOADER_DATA,+pages,+(efi_physical_addr_t*)addr);+break;+}+target-=MD_WEIGHT(md);+}++efi_call_early(free_pool,memory_map);++returnstatus;+}
@@ -23,7 +23,7 @@ KBUILD_CFLAGS := $(cflags-y) -DDISABLE_BRANCH_PROFILING \GCOV_PROFILE:=nKASAN_SANITIZE:=n-lib-y:=efi-stub-helper.o+lib-y:=efi-stub-helper.orandom.o# include the stub's generic dependencies from lib/ when building for ARM/arm64arm-deps:=fdt_rw.cfdt_ro.cfdt_wip.cfdt.cfdt_empty_tree.cfdt_sw.csort.c
Add support to scripts/sortextable for handling relocatable (PIE)
executables, whose ELF type is ET_DYN, not ET_EXEC. Other than adding
support for the new type, no changes are needed.
Signed-off-by: Ard Biesheuvel <redacted>
---
scripts/sortextable.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
The current definition of SWAPPER_TABLE_SHIFT can only be used in
asm code if the configured number of translation levels defines
PUD_SHIFT and/or PMD_SHIFT natively (4KB and 16KB/64KB granule,
respectively). Otherwise, it depends on the nopmd/nopud fixup
headers, which can only be included in C code.
So redefine SWAPPER_TABLE_SHIFT in a way that is independent of the
number of configured translation levels.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kernel-pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
This relaxes the kernel Image placement requirements, so that it
may be placed at any 2 MB aligned offset in physical memory.
This is accomplished by ignoring PHYS_OFFSET when installing
memblocks, and accounting for the apparent virtual offset of
the kernel Image. As a result, virtual address references
below PAGE_OFFSET are correctly mapped onto physical references
into the kernel Image regardless of where it sits in memory.
Signed-off-by: Ard Biesheuvel <redacted>
---
Documentation/arm64/booting.txt | 20 ++++--
arch/arm64/include/asm/boot.h | 6 ++
arch/arm64/include/asm/kernel-pgtable.h | 11 +++
arch/arm64/include/asm/kvm_mmu.h | 2 +-
arch/arm64/include/asm/memory.h | 15 +++--
arch/arm64/kernel/head.S | 19 ++++--
arch/arm64/mm/init.c | 71 +++++++++++++++++++-
arch/arm64/mm/mmu.c | 3 +
8 files changed, 125 insertions(+), 22 deletions(-)
@@ -109,7 +109,13 @@ Header notes: 1 - 4K 2 - 16K 3 - 64K- Bits 3-63: Reserved.+ Bit 3: Kernel physical placement+ 0 - 2MB aligned base should be as close as possible+ to the base of DRAM, since memory below it is not+ accessible+ 1 - 2MB aligned base may be anywhere in physical+ memory+ Bits 4-63: Reserved. - When image_size is zero, a bootloader should attempt to keep as much memory as possible free for use by the kernel immediately after the
@@ -117,14 +123,14 @@ Header notes: depending on selected features, and is effectively unbound. The Image must be placed text_offset bytes from a 2MB aligned base-address near the start of usable system RAM and called there. Memory-below that base address is currently unusable by Linux, and therefore it-is strongly recommended that this location is the start of system RAM.-The region between the 2 MB aligned base address and the start of the-image has no special significance to the kernel, and may be used for-other purposes.+address anywhere in usable system RAM and called there. The region+between the 2 MB aligned base address and the start of the image has no+special significance to the kernel, and may be used for other purposes. At least image_size bytes from the start of the image must be free for use by the kernel.+NOTE: versions prior to v4.6 cannot make use of memory below the+physical offset of the Image so it is recommended that the Image be+placed as close as possible to the start of system RAM. Any memory described to the kernel (even that below the start of the image) which is not marked as reserved from the kernel (e.g., with a
Before implementing KASLR for arm64 by building a self-relocating PIE
executable, we have to ensure that values we use before the relocation
routine is executed are not subject to dynamic relocation themselves.
This applies not only to virtual addresses, but also to values that are
supplied by the linker at build time and relocated using R_AARCH64_ABS64
relocations.
So instead, use assemble time constants, or force the use of static
relocations by folding the constants into the instructions.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/kernel/efi-entry.S | 2 +-
arch/arm64/kernel/head.S | 39 +++++++++++++-------
2 files changed, 27 insertions(+), 14 deletions(-)
Unfortunately, the current way of using the linker to emit build time
constants into the Image header will no longer work once we switch to
the use of PIE executables. The reason is that such constants are emitted
into the binary using R_AARCH64_ABS64 relocations, which we will resolve
at runtime, not at build time, and the places targeted by those
relocations will contain zeroes before that.
So move back to assembly time constants or R_AARCH64_ABS32 relocations
(which, interestingly enough, do get resolved at build time)
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/assembler.h | 15 ++++++++
arch/arm64/kernel/head.S | 17 +++++++--
arch/arm64/kernel/image.h | 37 ++++++--------------
3 files changed, 40 insertions(+), 29 deletions(-)
This adds support for emitting PLTs at module load time for relative
branches that are out of range. This is a prerequisite for KASLR, which
may place the kernel and the modules anywhere in the vmalloc area,
making it more likely that branch target offsets exceed the maximum
range of +/- 128 MB.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/Kconfig | 9 ++
arch/arm64/Makefile | 6 +-
arch/arm64/include/asm/module.h | 11 ++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/module-plts.c | 137 ++++++++++++++++++++
arch/arm64/kernel/module.c | 12 ++
arch/arm64/kernel/module.lds | 4 +
7 files changed, 179 insertions(+), 1 deletion(-)
@@ -363,6 +363,7 @@ config ARM64_ERRATUM_843419bool"Cortex-A53: 843419: A load or store might access an incorrect address"depends onMODULESdefaulty+selectARM64_MODULE_CMODEL_LARGEhelpThisoptionbuildskernelmodulesusingthelargememorymodelinordertoavoidtheuseoftheADRPinstruction,whichcancause
The page table accessors pte_offset(), pud_offset() and pmd_offset()
rely on __va translations, so they can only be used after the linear
mapping has been installed. For the early fixmap and kasan init routines,
whose page tables are allocated statically in the kernel image, these
functions will return bogus values. So implement pmd_offset_kimg() and
pud_offset_kimg(), which can be used instead before any page tables have
been allocated dynamically.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/pgtable.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
Please disregard this patch, I accidentally sent out two versions of
14/21, and this is the wrong one.
On 11 January 2016 at 14:19, Ard Biesheuvel [off-list ref] wrote:
quoted hunk
The current definition of SWAPPER_TABLE_SHIFT can only be used in
asm code if the configured number of translation levels defines
PUD_SHIFT and/or PMD_SHIFT natively (4KB and 16KB/64KB granule,
respectively). Otherwise, it depends on the nopmd/nopud fixup
headers, which can only be included in C code.
So redefine SWAPPER_TABLE_SHIFT in a way that is independent of the
number of configured translation levels. Define SWAPPER_TABLE_SIZE
as well, we will need it later.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kernel-pgtable.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Since the early fixmap page tables are populated using pages that are
part of the static footprint of the kernel, they are covered by the
initial kernel mapping, and we can refer to them without using __va/__pa
translations, which are tied to the linear mapping.
Since the fixmap page tables are disjoint from the kernel mapping up
to the top level pgd entry, we can refer to bm_pte[] directly, and there
is no need to walk the page tables and perform __pa()/__va() translations
at each step.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/mm/mmu.c | 32 ++++++--------------
1 file changed, 9 insertions(+), 23 deletions(-)
By default, early_init_dt_add_memory_arch() ignores memory below
the base of the kernel image since it won't be addressable via the
linear mapping. However, this is not appropriate anymore once we
decouple the kernel text mapping from the linear mapping, so archs
may want to drop the low limit entirely. So allow the minimum to be
overridden by setting MIN_MEMBLOCK_ADDR.
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/of/fdt.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 16:09:30
On Mon, Jan 11, 2016 at 02:18:57PM +0100, Ard Biesheuvel wrote:
quoted hunk
Since the early fixmap page tables are populated using pages that are
part of the static footprint of the kernel, they are covered by the
initial kernel mapping, and we can refer to them without using __va/__pa
translations, which are tied to the linear mapping.
Since the fixmap page tables are disjoint from the kernel mapping up
to the top level pgd entry, we can refer to bm_pte[] directly, and there
is no need to walk the page tables and perform __pa()/__va() translations
at each step.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/mm/mmu.c | 32 ++++++--------------
1 file changed, 9 insertions(+), 23 deletions(-)
@@ -570,38 +570,24 @@ void vmemmap_free(unsigned long start, unsigned long end)#endif /* CONFIG_SPARSEMEM_VMEMMAP */staticpte_tbm_pte[PTRS_PER_PTE]__page_aligned_bss;-#if CONFIG_PGTABLE_LEVELS > 2staticpmd_tbm_pmd[PTRS_PER_PMD]__page_aligned_bss;-#endif-#if CONFIG_PGTABLE_LEVELS > 3staticpud_tbm_pud[PTRS_PER_PUD]__page_aligned_bss;-#endifstaticinlinepud_t*fixmap_pud(unsignedlongaddr){-pgd_t*pgd=pgd_offset_k(addr);--BUG_ON(pgd_none(*pgd)||pgd_bad(*pgd));--returnpud_offset(pgd,addr);+return(CONFIG_PGTABLE_LEVELS>3)?&bm_pud[pud_index(addr)]+:(pud_t*)pgd_offset_k(addr);
If we move patch 6 earlier, we could use pud_offset_kimg here, and avoid
the cast, at the cost of passing the pgd into fixmap_pud.
Similarly for fixmap_pmd.
I assume the return type change was unintentional?
With STRICT_MM_TYPECHECKS:
arch/arm64/mm/mmu.c: In function 'fixmap_pmd':
arch/arm64/mm/mmu.c:604:9: warning: return from incompatible pointer type [-Wincompatible-pointer-types]
return (CONFIG_PGTABLE_LEVELS > 2) ? &bm_pmd[pmd_index(addr)]
^
arch/arm64/mm/mmu.c: In function 'early_fixmap_init':
arch/arm64/mm/mmu.c:635:6: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
pmd = fixmap_pmd(addr);
^
arch/arm64/mm/mmu.c:645:11: warning: comparison of distinct pointer types lacks a cast
if ((pmd != fixmap_pmd(fix_to_virt(FIX_BTMAP_BEGIN)))
^
arch/arm64/mm/mmu.c:646:14: warning: comparison of distinct pointer types lacks a cast
|| pmd != fixmap_pmd(fix_to_virt(FIX_BTMAP_END))) {
^
Side note: is there any reason we can't/shouldn't make
STRICT_MM_TYPECHECKS a common config option? Or simply have it on by
default for arm64?
Having built with and without typechecks I see that it doesn't bloat the
kernel Image size, though the binary isn't quite identical:
[mark at leverpostej:~/src/linux]% ls -al *.*checks
-rwxrwxr-x 1 mark mark 9288192 Jan 11 15:40 Image.checks
-rwxrwxr-x 1 mark mark 9288192 Jan 11 15:36 Image.nochecks
-rwxrwxr-x 1 mark mark 106782024 Jan 11 15:40 vmlinux.checks
-rwxrwxr-x 1 mark mark 106688928 Jan 11 15:35 vmlinux.nochecks
Things didn't quite line up between the two images, though I'm not sure
what the underlying difference was.
Thanks,
Mark.
On 11 January 2016 at 17:09, Mark Rutland [off-list ref] wrote:
On Mon, Jan 11, 2016 at 02:18:57PM +0100, Ard Biesheuvel wrote:
quoted
Since the early fixmap page tables are populated using pages that are
part of the static footprint of the kernel, they are covered by the
initial kernel mapping, and we can refer to them without using __va/__pa
translations, which are tied to the linear mapping.
Since the fixmap page tables are disjoint from the kernel mapping up
to the top level pgd entry, we can refer to bm_pte[] directly, and there
is no need to walk the page tables and perform __pa()/__va() translations
at each step.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/mm/mmu.c | 32 ++++++--------------
1 file changed, 9 insertions(+), 23 deletions(-)
@@ -570,38 +570,24 @@ void vmemmap_free(unsigned long start, unsigned long end)#endif /* CONFIG_SPARSEMEM_VMEMMAP */staticpte_tbm_pte[PTRS_PER_PTE]__page_aligned_bss;-#if CONFIG_PGTABLE_LEVELS > 2staticpmd_tbm_pmd[PTRS_PER_PMD]__page_aligned_bss;-#endif-#if CONFIG_PGTABLE_LEVELS > 3staticpud_tbm_pud[PTRS_PER_PUD]__page_aligned_bss;-#endifstaticinlinepud_t*fixmap_pud(unsignedlongaddr){-pgd_t*pgd=pgd_offset_k(addr);--BUG_ON(pgd_none(*pgd)||pgd_bad(*pgd));--returnpud_offset(pgd,addr);+return(CONFIG_PGTABLE_LEVELS>3)?&bm_pud[pud_index(addr)]+:(pud_t*)pgd_offset_k(addr);
If we move patch 6 earlier, we could use pud_offset_kimg here, and avoid
the cast, at the cost of passing the pgd into fixmap_pud.
Similarly for fixmap_pmd.
Is that necessarily an improvement? I know it hides the cast, but I
think having an explicit pgd_t* to pud_t* cast that so obviously
applies to CONFIG_PGTABLE_LEVELS < 4 only is fine as well.
I assume the return type change was unintentional?
Yes. Thanks for spotting that.
With STRICT_MM_TYPECHECKS:
arch/arm64/mm/mmu.c: In function 'fixmap_pmd':
arch/arm64/mm/mmu.c:604:9: warning: return from incompatible pointer type [-Wincompatible-pointer-types]
return (CONFIG_PGTABLE_LEVELS > 2) ? &bm_pmd[pmd_index(addr)]
^
arch/arm64/mm/mmu.c: In function 'early_fixmap_init':
arch/arm64/mm/mmu.c:635:6: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
pmd = fixmap_pmd(addr);
^
arch/arm64/mm/mmu.c:645:11: warning: comparison of distinct pointer types lacks a cast
if ((pmd != fixmap_pmd(fix_to_virt(FIX_BTMAP_BEGIN)))
^
arch/arm64/mm/mmu.c:646:14: warning: comparison of distinct pointer types lacks a cast
|| pmd != fixmap_pmd(fix_to_virt(FIX_BTMAP_END))) {
^
Side note: is there any reason we can't/shouldn't make
STRICT_MM_TYPECHECKS a common config option? Or simply have it on by
default for arm64?
I wouldn't mind at all.
Having built with and without typechecks I see that it doesn't bloat the
kernel Image size, though the binary isn't quite identical:
[mark at leverpostej:~/src/linux]% ls -al *.*checks
-rwxrwxr-x 1 mark mark 9288192 Jan 11 15:40 Image.checks
-rwxrwxr-x 1 mark mark 9288192 Jan 11 15:36 Image.nochecks
-rwxrwxr-x 1 mark mark 106782024 Jan 11 15:40 vmlinux.checks
-rwxrwxr-x 1 mark mark 106688928 Jan 11 15:35 vmlinux.nochecks
Things didn't quite line up between the two images, though I'm not sure
what the underlying difference was.
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 16:25:16
On Mon, Jan 11, 2016 at 02:18:59PM +0100, Ard Biesheuvel wrote:
The page table accessors pte_offset(), pud_offset() and pmd_offset()
rely on __va translations, so they can only be used after the linear
mapping has been installed. For the early fixmap and kasan init routines,
whose page tables are allocated statically in the kernel image, these
functions will return bogus values. So implement pmd_offset_kimg() and
pud_offset_kimg(), which can be used instead before any page tables have
been allocated dynamically.
Signed-off-by: Ard Biesheuvel <redacted>
This looks good to me. One possible suggsetion below, but either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
@@ -449,6 +449,9 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)#define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PHYS_MASK))+/* use ONLY for statically allocated translation tables */+#define pte_offset_kimg(dir,addr) ((pte_t *)__phys_to_kimg(pte_offset_phys((dir), (addr))))+
Given that we're probably only going to use this during one-off setup,
maybe it's worth something like:
#define IN_KERNEL_IMAGE(p) ({ \
unsigned long __p = (unsigned long)p; \
KIMAGE_VADDR <= __p && __p < _end; \
})
#define pte_offset_kimg(dir,addr) ({ \
BUG_ON(!IN_KERNEL_IMAGE(dir)); \
((pte_t *)__phys_to_kimg(pte_offset_phys((dir), (addr)))); \
})
That might be overkill, though, given all it does is turn one runtime
failure into another runtime failure.
Mark.
quoted hunk
/*
* Conversion functions: convert a page and protection to a page entry,
* and a page entry and page directory to the page they refer to.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 16:28:01
On Mon, Jan 11, 2016 at 05:15:13PM +0100, Ard Biesheuvel wrote:
On 11 January 2016 at 17:09, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:18:57PM +0100, Ard Biesheuvel wrote:
quoted
Since the early fixmap page tables are populated using pages that are
part of the static footprint of the kernel, they are covered by the
initial kernel mapping, and we can refer to them without using __va/__pa
translations, which are tied to the linear mapping.
Since the fixmap page tables are disjoint from the kernel mapping up
to the top level pgd entry, we can refer to bm_pte[] directly, and there
is no need to walk the page tables and perform __pa()/__va() translations
at each step.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/mm/mmu.c | 32 ++++++--------------
1 file changed, 9 insertions(+), 23 deletions(-)
@@ -570,38 +570,24 @@ void vmemmap_free(unsigned long start, unsigned long end)#endif /* CONFIG_SPARSEMEM_VMEMMAP */staticpte_tbm_pte[PTRS_PER_PTE]__page_aligned_bss;-#if CONFIG_PGTABLE_LEVELS > 2staticpmd_tbm_pmd[PTRS_PER_PMD]__page_aligned_bss;-#endif-#if CONFIG_PGTABLE_LEVELS > 3staticpud_tbm_pud[PTRS_PER_PUD]__page_aligned_bss;-#endifstaticinlinepud_t*fixmap_pud(unsignedlongaddr){-pgd_t*pgd=pgd_offset_k(addr);--BUG_ON(pgd_none(*pgd)||pgd_bad(*pgd));--returnpud_offset(pgd,addr);+return(CONFIG_PGTABLE_LEVELS>3)?&bm_pud[pud_index(addr)]+:(pud_t*)pgd_offset_k(addr);
If we move patch 6 earlier, we could use pud_offset_kimg here, and avoid
the cast, at the cost of passing the pgd into fixmap_pud.
Similarly for fixmap_pmd.
Is that necessarily an improvement? I know it hides the cast, but I
think having an explicit pgd_t* to pud_t* cast that so obviously
applies to CONFIG_PGTABLE_LEVELS < 4 only is fine as well.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 16:32:19
On Mon, Jan 11, 2016 at 02:18:55PM +0100, Ard Biesheuvel wrote:
This introduces the preprocessor symbol KIMAGE_VADDR which will serve as
the symbolic virtual base of the kernel region, i.e., the kernel's virtual
offset will be KIMAGE_VADDR + TEXT_OFFSET. For now, we define it as being
equal to PAGE_OFFSET, but in the future, it will be moved below it once
we move the kernel virtual mapping out of the linear mapping.
Signed-off-by: Ard Biesheuvel <redacted>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 16:51:34
On Mon, Jan 11, 2016 at 04:27:38PM +0000, Mark Rutland wrote:
On Mon, Jan 11, 2016 at 05:15:13PM +0100, Ard Biesheuvel wrote:
quoted
On 11 January 2016 at 17:09, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:18:57PM +0100, Ard Biesheuvel wrote:
quoted
Since the early fixmap page tables are populated using pages that are
part of the static footprint of the kernel, they are covered by the
initial kernel mapping, and we can refer to them without using __va/__pa
translations, which are tied to the linear mapping.
Since the fixmap page tables are disjoint from the kernel mapping up
to the top level pgd entry, we can refer to bm_pte[] directly, and there
is no need to walk the page tables and perform __pa()/__va() translations
at each step.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/mm/mmu.c | 32 ++++++--------------
1 file changed, 9 insertions(+), 23 deletions(-)
@@ -570,38 +570,24 @@ void vmemmap_free(unsigned long start, unsigned long end)#endif /* CONFIG_SPARSEMEM_VMEMMAP */staticpte_tbm_pte[PTRS_PER_PTE]__page_aligned_bss;-#if CONFIG_PGTABLE_LEVELS > 2staticpmd_tbm_pmd[PTRS_PER_PMD]__page_aligned_bss;-#endif-#if CONFIG_PGTABLE_LEVELS > 3staticpud_tbm_pud[PTRS_PER_PUD]__page_aligned_bss;-#endifstaticinlinepud_t*fixmap_pud(unsignedlongaddr){-pgd_t*pgd=pgd_offset_k(addr);--BUG_ON(pgd_none(*pgd)||pgd_bad(*pgd));--returnpud_offset(pgd,addr);+return(CONFIG_PGTABLE_LEVELS>3)?&bm_pud[pud_index(addr)]+:(pud_t*)pgd_offset_k(addr);
If we move patch 6 earlier, we could use pud_offset_kimg here, and avoid
the cast, at the cost of passing the pgd into fixmap_pud.
Similarly for fixmap_pmd.
Is that necessarily an improvement? I know it hides the cast, but I
think having an explicit pgd_t* to pud_t* cast that so obviously
applies to CONFIG_PGTABLE_LEVELS < 4 only is fine as well.
True; it's not a big thing either way.
Sorry, I'm gonig to change my mind on that again. I think using
p?d_offset_kimg is preferable. e.g.
static inline pud_t * fixmap_pud(unsigned long addr)
{
pgd_t *pgd = pgd_offset_k(addr);
BUG_ON(pgd_none(*pgd) || pgd_bad(*pgd));
return pud_offset_kimg(pgd, addr);
}
static inline pmd_t * fixmap_pmd(unsigned long addr)
{
pud_t *pud = fixmap_pud(addr);
BUG_ON(pud_none(*pud) || pud_bad(*pud));
return pmd_offset_kimg(pud, addr);
}
That avoids having to check CONFIG_PGTABLE_LEVELS check and perform a cast,
avoids duplicating details about bm_{pud,pmd}, and keeps the existing structure
so it's easier to reason about the change. I was wrong about having to pass the
pgd or pud in, so callers don't need upating.
On 11 January 2016 at 17:51, Mark Rutland [off-list ref] wrote:
On Mon, Jan 11, 2016 at 04:27:38PM +0000, Mark Rutland wrote:
quoted
On Mon, Jan 11, 2016 at 05:15:13PM +0100, Ard Biesheuvel wrote:
quoted
On 11 January 2016 at 17:09, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:18:57PM +0100, Ard Biesheuvel wrote:
quoted
Since the early fixmap page tables are populated using pages that are
part of the static footprint of the kernel, they are covered by the
initial kernel mapping, and we can refer to them without using __va/__pa
translations, which are tied to the linear mapping.
Since the fixmap page tables are disjoint from the kernel mapping up
to the top level pgd entry, we can refer to bm_pte[] directly, and there
is no need to walk the page tables and perform __pa()/__va() translations
at each step.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/mm/mmu.c | 32 ++++++--------------
1 file changed, 9 insertions(+), 23 deletions(-)
@@ -570,38 +570,24 @@ void vmemmap_free(unsigned long start, unsigned long end)#endif /* CONFIG_SPARSEMEM_VMEMMAP */staticpte_tbm_pte[PTRS_PER_PTE]__page_aligned_bss;-#if CONFIG_PGTABLE_LEVELS > 2staticpmd_tbm_pmd[PTRS_PER_PMD]__page_aligned_bss;-#endif-#if CONFIG_PGTABLE_LEVELS > 3staticpud_tbm_pud[PTRS_PER_PUD]__page_aligned_bss;-#endifstaticinlinepud_t*fixmap_pud(unsignedlongaddr){-pgd_t*pgd=pgd_offset_k(addr);--BUG_ON(pgd_none(*pgd)||pgd_bad(*pgd));--returnpud_offset(pgd,addr);+return(CONFIG_PGTABLE_LEVELS>3)?&bm_pud[pud_index(addr)]+:(pud_t*)pgd_offset_k(addr);
If we move patch 6 earlier, we could use pud_offset_kimg here, and avoid
the cast, at the cost of passing the pgd into fixmap_pud.
Similarly for fixmap_pmd.
Is that necessarily an improvement? I know it hides the cast, but I
think having an explicit pgd_t* to pud_t* cast that so obviously
applies to CONFIG_PGTABLE_LEVELS < 4 only is fine as well.
True; it's not a big thing either way.
Sorry, I'm gonig to change my mind on that again. I think using
p?d_offset_kimg is preferable. e.g.
static inline pud_t * fixmap_pud(unsigned long addr)
{
pgd_t *pgd = pgd_offset_k(addr);
BUG_ON(pgd_none(*pgd) || pgd_bad(*pgd));
return pud_offset_kimg(pgd, addr);
}
static inline pmd_t * fixmap_pmd(unsigned long addr)
{
pud_t *pud = fixmap_pud(addr);
BUG_ON(pud_none(*pud) || pud_bad(*pud));
return pmd_offset_kimg(pud, addr);
}
That avoids having to check CONFIG_PGTABLE_LEVELS check and perform a cast,
avoids duplicating details about bm_{pud,pmd}, and keeps the existing structure
so it's easier to reason about the change. I was wrong about having to pass the
pgd or pud in, so callers don't need upating.
From my PoV that is preferable.
On 11 January 2016 at 18:08, Ard Biesheuvel [off-list ref] wrote:
On 11 January 2016 at 17:51, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 04:27:38PM +0000, Mark Rutland wrote:
quoted
On Mon, Jan 11, 2016 at 05:15:13PM +0100, Ard Biesheuvel wrote:
quoted
On 11 January 2016 at 17:09, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:18:57PM +0100, Ard Biesheuvel wrote:
quoted
Since the early fixmap page tables are populated using pages that are
part of the static footprint of the kernel, they are covered by the
initial kernel mapping, and we can refer to them without using __va/__pa
translations, which are tied to the linear mapping.
Since the fixmap page tables are disjoint from the kernel mapping up
to the top level pgd entry, we can refer to bm_pte[] directly, and there
is no need to walk the page tables and perform __pa()/__va() translations
at each step.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/mm/mmu.c | 32 ++++++--------------
1 file changed, 9 insertions(+), 23 deletions(-)
@@ -570,38 +570,24 @@ void vmemmap_free(unsigned long start, unsigned long end)#endif /* CONFIG_SPARSEMEM_VMEMMAP */staticpte_tbm_pte[PTRS_PER_PTE]__page_aligned_bss;-#if CONFIG_PGTABLE_LEVELS > 2staticpmd_tbm_pmd[PTRS_PER_PMD]__page_aligned_bss;-#endif-#if CONFIG_PGTABLE_LEVELS > 3staticpud_tbm_pud[PTRS_PER_PUD]__page_aligned_bss;-#endifstaticinlinepud_t*fixmap_pud(unsignedlongaddr){-pgd_t*pgd=pgd_offset_k(addr);--BUG_ON(pgd_none(*pgd)||pgd_bad(*pgd));--returnpud_offset(pgd,addr);+return(CONFIG_PGTABLE_LEVELS>3)?&bm_pud[pud_index(addr)]+:(pud_t*)pgd_offset_k(addr);
If we move patch 6 earlier, we could use pud_offset_kimg here, and avoid
the cast, at the cost of passing the pgd into fixmap_pud.
Similarly for fixmap_pmd.
Is that necessarily an improvement? I know it hides the cast, but I
think having an explicit pgd_t* to pud_t* cast that so obviously
applies to CONFIG_PGTABLE_LEVELS < 4 only is fine as well.
True; it's not a big thing either way.
Sorry, I'm gonig to change my mind on that again. I think using
p?d_offset_kimg is preferable. e.g.
static inline pud_t * fixmap_pud(unsigned long addr)
{
pgd_t *pgd = pgd_offset_k(addr);
BUG_ON(pgd_none(*pgd) || pgd_bad(*pgd));
return pud_offset_kimg(pgd, addr);
}
static inline pmd_t * fixmap_pmd(unsigned long addr)
{
pud_t *pud = fixmap_pud(addr);
BUG_ON(pud_none(*pud) || pud_bad(*pud));
return pmd_offset_kimg(pud, addr);
}
That avoids having to check CONFIG_PGTABLE_LEVELS check and perform a cast,
avoids duplicating details about bm_{pud,pmd}, and keeps the existing structure
so it's easier to reason about the change. I was wrong about having to pass the
pgd or pud in, so callers don't need upating.
From my PoV that is preferable.
OK. I think it looks better, indeed.
... however, this does mean we have to go through a __pa() translation
and back just to get to the address of bm_pud/bm_pmd
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 17:21:33
On Mon, Jan 11, 2016 at 06:15:56PM +0100, Ard Biesheuvel wrote:
On 11 January 2016 at 18:08, Ard Biesheuvel [off-list ref] wrote:
quoted
On 11 January 2016 at 17:51, Mark Rutland [off-list ref] wrote:
quoted
Sorry, I'm gonig to change my mind on that again. I think using
p?d_offset_kimg is preferable. e.g.
static inline pud_t * fixmap_pud(unsigned long addr)
{
pgd_t *pgd = pgd_offset_k(addr);
BUG_ON(pgd_none(*pgd) || pgd_bad(*pgd));
return pud_offset_kimg(pgd, addr);
}
static inline pmd_t * fixmap_pmd(unsigned long addr)
{
pud_t *pud = fixmap_pud(addr);
BUG_ON(pud_none(*pud) || pud_bad(*pud));
return pmd_offset_kimg(pud, addr);
}
That avoids having to check CONFIG_PGTABLE_LEVELS check and perform a cast,
avoids duplicating details about bm_{pud,pmd}, and keeps the existing structure
so it's easier to reason about the change. I was wrong about having to pass the
pgd or pud in, so callers don't need upating.
From my PoV that is preferable.
OK. I think it looks better, indeed.
... however, this does mean we have to go through a __pa() translation
and back just to get to the address of bm_pud/bm_pmd
True, but we only do it in the case of a one-off init function, so I
don't think we'll notice the overhead.
Mark.
On 11 January 2016 at 17:24, Mark Rutland [off-list ref] wrote:
On Mon, Jan 11, 2016 at 02:18:59PM +0100, Ard Biesheuvel wrote:
quoted
The page table accessors pte_offset(), pud_offset() and pmd_offset()
rely on __va translations, so they can only be used after the linear
mapping has been installed. For the early fixmap and kasan init routines,
whose page tables are allocated statically in the kernel image, these
functions will return bogus values. So implement pmd_offset_kimg() and
pud_offset_kimg(), which can be used instead before any page tables have
been allocated dynamically.
Signed-off-by: Ard Biesheuvel <redacted>
This looks good to me. One possible suggsetion below, but either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
@@ -449,6 +449,9 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)#define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PHYS_MASK))+/* use ONLY for statically allocated translation tables */+#define pte_offset_kimg(dir,addr) ((pte_t *)__phys_to_kimg(pte_offset_phys((dir), (addr))))+
Given that we're probably only going to use this during one-off setup,
maybe it's worth something like:
#define IN_KERNEL_IMAGE(p) ({ \
unsigned long __p = (unsigned long)p; \
KIMAGE_VADDR <= __p && __p < _end; \
})
#define pte_offset_kimg(dir,addr) ({ \
BUG_ON(!IN_KERNEL_IMAGE(dir)); \
((pte_t *)__phys_to_kimg(pte_offset_phys((dir), (addr)))); \
})
That might be overkill, though, given all it does is turn one runtime
failure into another runtime failure.
Yes. I did consider implementing them out of line, with __init
annotations so you at least get complaints if you refer to them from
non-init code, but I don't see how we would ever need these anywhere
beyond fixmap and kasan anyway
quoted
/*
* Conversion functions: convert a page and protection to a page entry,
* and a page entry and page directory to the page they refer to.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 17:32:20
On Mon, Jan 11, 2016 at 06:28:51PM +0100, Ard Biesheuvel wrote:
On 11 January 2016 at 17:24, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:18:59PM +0100, Ard Biesheuvel wrote:
quoted
The page table accessors pte_offset(), pud_offset() and pmd_offset()
rely on __va translations, so they can only be used after the linear
mapping has been installed. For the early fixmap and kasan init routines,
whose page tables are allocated statically in the kernel image, these
functions will return bogus values. So implement pmd_offset_kimg() and
pud_offset_kimg(), which can be used instead before any page tables have
been allocated dynamically.
Signed-off-by: Ard Biesheuvel <redacted>
This looks good to me. One possible suggsetion below, but either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
@@ -449,6 +449,9 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)#define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PHYS_MASK))+/* use ONLY for statically allocated translation tables */+#define pte_offset_kimg(dir,addr) ((pte_t *)__phys_to_kimg(pte_offset_phys((dir), (addr))))+
Given that we're probably only going to use this during one-off setup,
maybe it's worth something like:
#define IN_KERNEL_IMAGE(p) ({ \
unsigned long __p = (unsigned long)p; \
KIMAGE_VADDR <= __p && __p < _end; \
})
#define pte_offset_kimg(dir,addr) ({ \
BUG_ON(!IN_KERNEL_IMAGE(dir)); \
((pte_t *)__phys_to_kimg(pte_offset_phys((dir), (addr)))); \
})
That might be overkill, though, given all it does is turn one runtime
failure into another runtime failure.
Yes. I did consider implementing them out of line, with __init
annotations so you at least get complaints if you refer to them from
non-init code, but I don't see how we would ever need these anywhere
beyond fixmap and kasan anyway
Ok. Let's forget about that for now then. :)
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-11 17:40:46
On Mon, Jan 11, 2016 at 02:18:56PM +0100, Ard Biesheuvel wrote:
quoted hunk
Add definitions of pud_index() and pmd_index() for configurations with
fewer than 4 resp. 3 translation levels. This makes it easier to keep
the users (e.g., the fixmap init code) generic.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
I think we don't need these if we use p??_ofset_kimg for the fixmap
initialisation.
Regardless, these look good conceptually, so if they're useful
elsewhere:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
On Mon, Jan 11, 2016 at 5:18 AM, Ard Biesheuvel
[off-list ref] wrote:
This series implements KASLR for arm64, by building the kernel as a PIE
executable that can relocate itself at runtime, and moving it to a random
offset in the vmalloc area. v2 and up also implement physical randomization,
i.e., it allows the kernel to deal with being loaded at any physical offset
(modulo the required alignment), and invokes the EFI_RNG_PROTOCOL from the
UEFI stub to obtain random bits and perform the actual randomization of the
physical load address.
I will continue cheering! :)
Changes since v2:
- Incorporated feedback from Marc Zyngier into the KVM patch (#5)
- Dropped the pgdir section and the patch that memblock_reserve()'s the kernel
sections at a smaller granularity. This is no longer necessary with the pgdir
section gone. This also fixes an issue spotted by James Morse where the fixmap
page tables are not zeroed correctly; these have been moved back to the .bss
section.
- Got rid of all ifdef'ery regarding the number of translation levels in the
changed .c files, by introducing new definitions in pgtable.h (#3, #6)
- Fixed KAsan support, which was broken by all earlier versions.
- Moved module region along with the virtually randomized kernel, so that module
addresses become unpredictable as well, and we only have to rely on veneers in
the PLTs when the module region is exhausted (which is somewhat more likely
since the module region is now shared with other uses of the vmalloc area)
Just to make sure I understand: this means that the offset between
kernel and modules remains static? It may still be useful to bump
modules as well, just so that leaking a module address doesn't
compromise the base kernel image address too. Don't block the series
for this, though. It's a minor nit. :)
-Kees
- Added support for the 'nokaslr' command line option. This affects the
randomization performed by the stub, and results in a warning if passed while
the bootloader also presented a random seed for virtual KASLR in register x1.
- The .text/.rodata sections of the kernel are no longer aliased in the linear
region with a writable mapping.
- Added a separate image header flag for kernel images that may be loaded at any
2 MB aligned offset (+ TEXT_OFFSET)
- The KASLR displacement is now corrected if it results in the kernel image
intersecting a PUD/PMD boundary (4k and 16k/64k granule kernels, respectively)
- Split out UEFI stub random routines into separate patches.
- Implemented a weight based EFI random allocation routine so that each suitable
offset in available memory is equally likely to be selected (as suggested by
Kees Cook)
- Reused CONFIG_RELOCATABLE and CONFIG_RANDOMIZE_BASE instead of introducing
new Kconfig symbols to describe the same functionality.
- Reimplemented mem= logic so memory is clipped from the top first.
Changes since v1/RFC:
- This series now implements fully independent virtual and physical address
randomization at load time. I have recycled some patches from this series:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/455151, and updated the
final UEFI stub patch to randomize the physical address as well.
- Added a patch to deal with the way KVM on arm64 makes assumptions about the
relation between kernel symbols and the linear mapping (on which the HYP
mapping is based), as these assumptions cease to be valid once we move the
kernel Image out of the linear mapping.
- Updated the module PLT patch so it works on BE kernels as well.
- Moved the constant Image header values to head.S, and updated the linker
script to provide the kernel size using R_AARCH64_ABS32 relocation rather
than a R_AARCH64_ABS64 relocation, since those are always resolved at build
time. This allows me to get rid of the post-build perl script to swab header
values on BE kernels.
- Minor style tweaks.
Notes:
- These patches apply on top of Mark Rutland's pagetable rework series:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/462438
- The arm64 Image is uncompressed by default, and the Elf64_Rela format uses
24 bytes per relocation entry. This results in considerable bloat (i.e., a
couple of MBs worth of relocation data in an .init section). However, no
build time postprocessing is required, we rely fully on the toolchain to
produce the image
- We have to rely on the bootloader to supply some randomness in register x1
upon kernel entry. Since we have no decompressor, it is simply not feasible
to collect randomness in the head.S code path before mapping the kernel and
enabling the MMU.
- The EFI_RNG_PROTOCOL that is invoked in patch #13 to supply randomness on
UEFI systems is not universally available. A QEMU/KVM firmware image that
implements a pseudo-random version is available here:
http://people.linaro.org/~ard.biesheuvel/QEMU_EFI.fd.aarch64-rng.bz2
(requires access to PMCCNTR_EL0 and support for AES instructions)
See below for instructions how to run the pseudo-random version on real
hardware.
- Only mildly tested. Help appreciated.
Code can be found here:
git://git.linaro.org/people/ard.biesheuvel/linux-arm.git arm64-kaslr-v3
https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/shortlog/refs/heads/arm64-kaslr-v3
Patch #1 updates the OF code to allow the minimum memblock physical address to
be overridden by the arch.
Patch #2 introduces KIMAGE_VADDR as the base of the kernel virtual region.
Patch #3 introduces dummy pud_index() and pmd_index() macros that are intended
to be optimized away if the configured number of translation levels does not
actually use them.
Patch #4 rewrites early_fixmap_init() so it does not rely on the linear mapping
(i.e., the use of phys_to_virt() is avoided)
Patch #5 updates KVM on arm64 so it can deal with kernel symbols whose addresses
are not covered by the linear mapping.
Patch #6 introduces pte_offset_kimg(), pmd_offset_kimg() and pud_offset_kimg()
that allow statically allocated page tables (i.e., by fixmap and kasan) to be
traversed before the linear mapping is installed.
Patch #7 moves the kernel virtual mapping to the vmalloc area, along with the
module region which is kept right below it, as before.
Patch #8 adds support for PLTs in modules so that relative branches can be
resolved via a PLT if the target is out of range. This is required for KASLR,
since modules may be loaded far away from the core kernel.
Patch #9 and #10 move arm64 to the a new generic relative version of the extable
implementation so that it no longer contains absolute addresses that require
fixing up at relocation time, but uses relative offsets instead.
Patch #11 reverts some changes to the Image header population code so we no
longer depend on the linker to populate the header fields. This is necessary
since the R_AARCH64_ABS64 relocations that are emitted for these fields are not
resolved at build time for PIE executables.
Patch #12 updates the code in head.S that needs to execute before relocation to
avoid the use of values that are subject to dynamic relocation. These values
will not be populated in PIE executables.
Patch #13 allows the kernel Image to be loaded anywhere in physical memory, by
decoupling PHYS_OFFSET from the base of the kernel image.
Patch #14 redefines SWAPPER_TABLE_SHIFT in a way that allows it to be used from
assembler code regardless of the number of configured translation levels.
Patch #15 (from Mark Rutland) moves the ELF relocation type #defines to a
separate file so we can use it from head.S later
Patch #16 updates scripts/sortextable.c so it accepts ET_DYN (relocatable)
executables as well as ET_EXEC (static) executables.
Patch #17 implements the core KASLR, by taking randomness supplied in register
x1 and using it to move the kernel inside the vmalloc area.
Patch #18 implements efi_get_random_bytes() based on the EFI_RNG_PROTOCOL
Patch #19 implements efi_random_alloc()
Patch #20 moves the allocation for the converted command line (UTF-16 to ASCII)
away from the base of memory. This is necessary since for parsing
Patch #21 implements the actual KASLR, by randomizing the kernel physical
address, and passing entropy in x1 so that the kernel proper can relocate itself
virtually.
Ard Biesheuvel (20):
of/fdt: make memblock minimum physical address arch configurable
arm64: introduce KIMAGE_VADDR as the virtual base of the kernel region
arm64: pgtable: add dummy pud_index() and pmd_index() definitions
arm64: decouple early fixmap init from linear mapping
arm64: kvm: deal with kernel symbols outside of linear mapping
arm64: pgtable: implement static [pte|pmd|pud]_offset variants
arm64: move kernel image to base of vmalloc area
arm64: add support for module PLTs
extable: add support for relative extables to search and sort routines
arm64: switch to relative exception tables
arm64: avoid R_AARCH64_ABS64 relocations for Image header fields
arm64: avoid dynamic relocations in early boot code
arm64: allow kernel Image to be loaded anywhere in physical memory
arm64: redefine SWAPPER_TABLE_SHIFT for use in asm code
scripts/sortextable: add support for ET_DYN binaries
arm64: add support for a relocatable kernel and KASLR
efi: stub: implement efi_get_random_bytes() based on EFI_RNG_PROTOCOL
efi: stub: add implementation of efi_random_alloc()
efi: stub: use high allocation for converted command line
arm64: efi: invoke EFI_RNG_PROTOCOL to supply KASLR randomness
Mark Rutland (1):
arm64: split elf relocs into a separate header.
Documentation/arm64/booting.txt | 34 ++++-
arch/arm/include/asm/kvm_asm.h | 2 +
arch/arm/include/asm/kvm_mmu.h | 2 +
arch/arm/kvm/arm.c | 5 +-
arch/arm/kvm/mmu.c | 8 +-
arch/arm64/Kconfig | 40 +++++
arch/arm64/Makefile | 10 +-
arch/arm64/include/asm/assembler.h | 30 +++-
arch/arm64/include/asm/boot.h | 6 +
arch/arm64/include/asm/elf.h | 54 +------
arch/arm64/include/asm/elf_relocs.h | 75 ++++++++++
arch/arm64/include/asm/futex.h | 12 +-
arch/arm64/include/asm/kasan.h | 20 +--
arch/arm64/include/asm/kernel-pgtable.h | 20 ++-
arch/arm64/include/asm/kvm_asm.h | 19 ++-
arch/arm64/include/asm/kvm_host.h | 8 +-
arch/arm64/include/asm/kvm_mmu.h | 2 +
arch/arm64/include/asm/memory.h | 38 +++--
arch/arm64/include/asm/module.h | 11 ++
arch/arm64/include/asm/pgtable.h | 22 ++-
arch/arm64/include/asm/uaccess.h | 30 ++--
arch/arm64/include/asm/virt.h | 4 -
arch/arm64/include/asm/word-at-a-time.h | 7 +-
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/armv8_deprecated.c | 7 +-
arch/arm64/kernel/efi-entry.S | 9 +-
arch/arm64/kernel/head.S | 155 +++++++++++++++++---
arch/arm64/kernel/image.h | 37 ++---
arch/arm64/kernel/module-plts.c | 137 +++++++++++++++++
arch/arm64/kernel/module.c | 15 +-
arch/arm64/kernel/module.lds | 4 +
arch/arm64/kernel/setup.c | 44 +++++-
arch/arm64/kernel/vmlinux.lds.S | 13 +-
arch/arm64/kvm/debug.c | 1 +
arch/arm64/kvm/hyp.S | 6 +-
arch/arm64/mm/dump.c | 12 +-
arch/arm64/mm/extable.c | 2 +-
arch/arm64/mm/init.c | 91 ++++++++++--
arch/arm64/mm/kasan_init.c | 21 ++-
arch/arm64/mm/mmu.c | 95 +++++++-----
arch/x86/include/asm/efi.h | 2 +
drivers/firmware/efi/libstub/Makefile | 2 +-
drivers/firmware/efi/libstub/arm-stub.c | 17 ++-
drivers/firmware/efi/libstub/arm64-stub.c | 67 +++++++--
drivers/firmware/efi/libstub/efi-stub-helper.c | 24 ++-
drivers/firmware/efi/libstub/efistub.h | 9 ++
drivers/firmware/efi/libstub/random.c | 120 +++++++++++++++
drivers/of/fdt.c | 5 +-
include/linux/efi.h | 5 +-
lib/extable.c | 50 +++++--
scripts/sortextable.c | 10 +-
51 files changed, 1111 insertions(+), 309 deletions(-)
create mode 100644 arch/arm64/include/asm/elf_relocs.h
create mode 100644 arch/arm64/kernel/module-plts.c
create mode 100644 arch/arm64/kernel/module.lds
create mode 100644 drivers/firmware/efi/libstub/random.c
EFI_RNG_PROTOCOL on real hardware
=================================
To test whether your UEFI implements the EFI_RNG_PROTOCOL, download the
following executable and run it from the UEFI Shell:
http://people.linaro.org/~ard.biesheuvel/RngTest.efi
FS0:\> rngtest
UEFI RNG Protocol Testing :
----------------------------
-- Locate UEFI RNG Protocol : [Fail - Status = Not Found]
If your UEFI does not implement the EFI_RNG_PROTOCOL, you can download and
install the pseudo-random version that uses the generic timer and PMCCNTR_EL0
values and permutes them using a couple of rounds of AES.
http://people.linaro.org/~ard.biesheuvel/RngDxe.efi
NOTE: not for production!! This is a quick and dirty hack to test the KASLR
code, and is not suitable for anything else.
FS0:\> rngdxe
FS0:\> rngtest
UEFI RNG Protocol Testing :
----------------------------
-- Locate UEFI RNG Protocol : [Pass]
-- Call RNG->GetInfo() interface :
>> Supported RNG Algorithm (Count = 2) :
0) 44F0DE6E-4D8C-4045-A8C7-4DD168856B9E
1) E43176D7-B6E8-4827-B784-7FFDC4B68561
-- Call RNG->GetRNG() interface :
>> RNG with default algorithm : [Pass]
>> RNG with SP800-90-HMAC-256 : [Fail - Status = Unsupported]
>> RNG with SP800-90-Hash-256 : [Fail - Status = Unsupported]
>> RNG with SP800-90-CTR-256 : [Pass]
>> RNG with X9.31-3DES : [Fail - Status = Unsupported]
>> RNG with X9.31-AES : [Fail - Status = Unsupported]
>> RNG with RAW Entropy : [Pass]
-- Random Number Generation Test with default RNG Algorithm (20 Rounds):
01) - 27
02) - 61E8
03) - 496FD8
04) - DDD793BF
05) - B6C37C8E23
06) - 4D183C604A96
07) - 9363311DB61298
08) - 5715A7294F4E436E
09) - F0D4D7BAA0DD52318E
10) - C88C6EBCF4C0474D87C3
11) - B5594602B482A643932172
12) - CA7573F704B2089B726B9CF1
13) - A93E9451CB533DCFBA87B97C33
14) - 45AA7B83DB6044F7BBAB031F0D24
15) - 3DD7A4D61F34ADCB400B5976730DCF
16) - 4DD168D21FAB8F59708330D6A9BEB021
17) - 4BBB225E61C465F174254159467E65939F
18) - 030A156C9616337A20070941E702827DA8E1
19) - AB0FC11C9A4E225011382A9D164D9D55CA2B64
20) - 72B9B4735DC445E5DA6AF88DE965B7E87CB9A23C
On 11 January 2016 at 23:07, Kees Cook [off-list ref] wrote:
On Mon, Jan 11, 2016 at 5:18 AM, Ard Biesheuvel
[off-list ref] wrote:
quoted
This series implements KASLR for arm64, by building the kernel as a PIE
executable that can relocate itself at runtime, and moving it to a random
offset in the vmalloc area. v2 and up also implement physical randomization,
i.e., it allows the kernel to deal with being loaded at any physical offset
(modulo the required alignment), and invokes the EFI_RNG_PROTOCOL from the
UEFI stub to obtain random bits and perform the actual randomization of the
physical load address.
I will continue cheering! :)
:-)
quoted
Changes since v2:
- Incorporated feedback from Marc Zyngier into the KVM patch (#5)
- Dropped the pgdir section and the patch that memblock_reserve()'s the kernel
sections at a smaller granularity. This is no longer necessary with the pgdir
section gone. This also fixes an issue spotted by James Morse where the fixmap
page tables are not zeroed correctly; these have been moved back to the .bss
section.
- Got rid of all ifdef'ery regarding the number of translation levels in the
changed .c files, by introducing new definitions in pgtable.h (#3, #6)
- Fixed KAsan support, which was broken by all earlier versions.
- Moved module region along with the virtually randomized kernel, so that module
addresses become unpredictable as well, and we only have to rely on veneers in
the PLTs when the module region is exhausted (which is somewhat more likely
since the module region is now shared with other uses of the vmalloc area)
Just to make sure I understand: this means that the offset between
kernel and modules remains static? It may still be useful to bump
modules as well, just so that leaking a module address doesn't
compromise the base kernel image address too. Don't block the series
for this, though. It's a minor nit. :)
Well, the module region could be any 128 MB memory region that also
covers the [_stext, _etext) interval. This would still allow all
modules to branch to all other modules and the core kernel without
resorting to indirect PLT jumps.
IOW, I think I can work around this quite easily.
quoted
- Added support for the 'nokaslr' command line option. This affects the
randomization performed by the stub, and results in a warning if passed while
the bootloader also presented a random seed for virtual KASLR in register x1.
- The .text/.rodata sections of the kernel are no longer aliased in the linear
region with a writable mapping.
- Added a separate image header flag for kernel images that may be loaded at any
2 MB aligned offset (+ TEXT_OFFSET)
- The KASLR displacement is now corrected if it results in the kernel image
intersecting a PUD/PMD boundary (4k and 16k/64k granule kernels, respectively)
- Split out UEFI stub random routines into separate patches.
- Implemented a weight based EFI random allocation routine so that each suitable
offset in available memory is equally likely to be selected (as suggested by
Kees Cook)
- Reused CONFIG_RELOCATABLE and CONFIG_RANDOMIZE_BASE instead of introducing
new Kconfig symbols to describe the same functionality.
- Reimplemented mem= logic so memory is clipped from the top first.
Changes since v1/RFC:
- This series now implements fully independent virtual and physical address
randomization at load time. I have recycled some patches from this series:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/455151, and updated the
final UEFI stub patch to randomize the physical address as well.
- Added a patch to deal with the way KVM on arm64 makes assumptions about the
relation between kernel symbols and the linear mapping (on which the HYP
mapping is based), as these assumptions cease to be valid once we move the
kernel Image out of the linear mapping.
- Updated the module PLT patch so it works on BE kernels as well.
- Moved the constant Image header values to head.S, and updated the linker
script to provide the kernel size using R_AARCH64_ABS32 relocation rather
than a R_AARCH64_ABS64 relocation, since those are always resolved at build
time. This allows me to get rid of the post-build perl script to swab header
values on BE kernels.
- Minor style tweaks.
Notes:
- These patches apply on top of Mark Rutland's pagetable rework series:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/462438
- The arm64 Image is uncompressed by default, and the Elf64_Rela format uses
24 bytes per relocation entry. This results in considerable bloat (i.e., a
couple of MBs worth of relocation data in an .init section). However, no
build time postprocessing is required, we rely fully on the toolchain to
produce the image
- We have to rely on the bootloader to supply some randomness in register x1
upon kernel entry. Since we have no decompressor, it is simply not feasible
to collect randomness in the head.S code path before mapping the kernel and
enabling the MMU.
- The EFI_RNG_PROTOCOL that is invoked in patch #13 to supply randomness on
UEFI systems is not universally available. A QEMU/KVM firmware image that
implements a pseudo-random version is available here:
http://people.linaro.org/~ard.biesheuvel/QEMU_EFI.fd.aarch64-rng.bz2
(requires access to PMCCNTR_EL0 and support for AES instructions)
See below for instructions how to run the pseudo-random version on real
hardware.
- Only mildly tested. Help appreciated.
Code can be found here:
git://git.linaro.org/people/ard.biesheuvel/linux-arm.git arm64-kaslr-v3
https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/shortlog/refs/heads/arm64-kaslr-v3
Patch #1 updates the OF code to allow the minimum memblock physical address to
be overridden by the arch.
Patch #2 introduces KIMAGE_VADDR as the base of the kernel virtual region.
Patch #3 introduces dummy pud_index() and pmd_index() macros that are intended
to be optimized away if the configured number of translation levels does not
actually use them.
Patch #4 rewrites early_fixmap_init() so it does not rely on the linear mapping
(i.e., the use of phys_to_virt() is avoided)
Patch #5 updates KVM on arm64 so it can deal with kernel symbols whose addresses
are not covered by the linear mapping.
Patch #6 introduces pte_offset_kimg(), pmd_offset_kimg() and pud_offset_kimg()
that allow statically allocated page tables (i.e., by fixmap and kasan) to be
traversed before the linear mapping is installed.
Patch #7 moves the kernel virtual mapping to the vmalloc area, along with the
module region which is kept right below it, as before.
Patch #8 adds support for PLTs in modules so that relative branches can be
resolved via a PLT if the target is out of range. This is required for KASLR,
since modules may be loaded far away from the core kernel.
Patch #9 and #10 move arm64 to the a new generic relative version of the extable
implementation so that it no longer contains absolute addresses that require
fixing up at relocation time, but uses relative offsets instead.
Patch #11 reverts some changes to the Image header population code so we no
longer depend on the linker to populate the header fields. This is necessary
since the R_AARCH64_ABS64 relocations that are emitted for these fields are not
resolved at build time for PIE executables.
Patch #12 updates the code in head.S that needs to execute before relocation to
avoid the use of values that are subject to dynamic relocation. These values
will not be populated in PIE executables.
Patch #13 allows the kernel Image to be loaded anywhere in physical memory, by
decoupling PHYS_OFFSET from the base of the kernel image.
Patch #14 redefines SWAPPER_TABLE_SHIFT in a way that allows it to be used from
assembler code regardless of the number of configured translation levels.
Patch #15 (from Mark Rutland) moves the ELF relocation type #defines to a
separate file so we can use it from head.S later
Patch #16 updates scripts/sortextable.c so it accepts ET_DYN (relocatable)
executables as well as ET_EXEC (static) executables.
Patch #17 implements the core KASLR, by taking randomness supplied in register
x1 and using it to move the kernel inside the vmalloc area.
Patch #18 implements efi_get_random_bytes() based on the EFI_RNG_PROTOCOL
Patch #19 implements efi_random_alloc()
Patch #20 moves the allocation for the converted command line (UTF-16 to ASCII)
away from the base of memory. This is necessary since for parsing
Patch #21 implements the actual KASLR, by randomizing the kernel physical
address, and passing entropy in x1 so that the kernel proper can relocate itself
virtually.
Ard Biesheuvel (20):
of/fdt: make memblock minimum physical address arch configurable
arm64: introduce KIMAGE_VADDR as the virtual base of the kernel region
arm64: pgtable: add dummy pud_index() and pmd_index() definitions
arm64: decouple early fixmap init from linear mapping
arm64: kvm: deal with kernel symbols outside of linear mapping
arm64: pgtable: implement static [pte|pmd|pud]_offset variants
arm64: move kernel image to base of vmalloc area
arm64: add support for module PLTs
extable: add support for relative extables to search and sort routines
arm64: switch to relative exception tables
arm64: avoid R_AARCH64_ABS64 relocations for Image header fields
arm64: avoid dynamic relocations in early boot code
arm64: allow kernel Image to be loaded anywhere in physical memory
arm64: redefine SWAPPER_TABLE_SHIFT for use in asm code
scripts/sortextable: add support for ET_DYN binaries
arm64: add support for a relocatable kernel and KASLR
efi: stub: implement efi_get_random_bytes() based on EFI_RNG_PROTOCOL
efi: stub: add implementation of efi_random_alloc()
efi: stub: use high allocation for converted command line
arm64: efi: invoke EFI_RNG_PROTOCOL to supply KASLR randomness
Mark Rutland (1):
arm64: split elf relocs into a separate header.
Documentation/arm64/booting.txt | 34 ++++-
arch/arm/include/asm/kvm_asm.h | 2 +
arch/arm/include/asm/kvm_mmu.h | 2 +
arch/arm/kvm/arm.c | 5 +-
arch/arm/kvm/mmu.c | 8 +-
arch/arm64/Kconfig | 40 +++++
arch/arm64/Makefile | 10 +-
arch/arm64/include/asm/assembler.h | 30 +++-
arch/arm64/include/asm/boot.h | 6 +
arch/arm64/include/asm/elf.h | 54 +------
arch/arm64/include/asm/elf_relocs.h | 75 ++++++++++
arch/arm64/include/asm/futex.h | 12 +-
arch/arm64/include/asm/kasan.h | 20 +--
arch/arm64/include/asm/kernel-pgtable.h | 20 ++-
arch/arm64/include/asm/kvm_asm.h | 19 ++-
arch/arm64/include/asm/kvm_host.h | 8 +-
arch/arm64/include/asm/kvm_mmu.h | 2 +
arch/arm64/include/asm/memory.h | 38 +++--
arch/arm64/include/asm/module.h | 11 ++
arch/arm64/include/asm/pgtable.h | 22 ++-
arch/arm64/include/asm/uaccess.h | 30 ++--
arch/arm64/include/asm/virt.h | 4 -
arch/arm64/include/asm/word-at-a-time.h | 7 +-
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/armv8_deprecated.c | 7 +-
arch/arm64/kernel/efi-entry.S | 9 +-
arch/arm64/kernel/head.S | 155 +++++++++++++++++---
arch/arm64/kernel/image.h | 37 ++---
arch/arm64/kernel/module-plts.c | 137 +++++++++++++++++
arch/arm64/kernel/module.c | 15 +-
arch/arm64/kernel/module.lds | 4 +
arch/arm64/kernel/setup.c | 44 +++++-
arch/arm64/kernel/vmlinux.lds.S | 13 +-
arch/arm64/kvm/debug.c | 1 +
arch/arm64/kvm/hyp.S | 6 +-
arch/arm64/mm/dump.c | 12 +-
arch/arm64/mm/extable.c | 2 +-
arch/arm64/mm/init.c | 91 ++++++++++--
arch/arm64/mm/kasan_init.c | 21 ++-
arch/arm64/mm/mmu.c | 95 +++++++-----
arch/x86/include/asm/efi.h | 2 +
drivers/firmware/efi/libstub/Makefile | 2 +-
drivers/firmware/efi/libstub/arm-stub.c | 17 ++-
drivers/firmware/efi/libstub/arm64-stub.c | 67 +++++++--
drivers/firmware/efi/libstub/efi-stub-helper.c | 24 ++-
drivers/firmware/efi/libstub/efistub.h | 9 ++
drivers/firmware/efi/libstub/random.c | 120 +++++++++++++++
drivers/of/fdt.c | 5 +-
include/linux/efi.h | 5 +-
lib/extable.c | 50 +++++--
scripts/sortextable.c | 10 +-
51 files changed, 1111 insertions(+), 309 deletions(-)
create mode 100644 arch/arm64/include/asm/elf_relocs.h
create mode 100644 arch/arm64/kernel/module-plts.c
create mode 100644 arch/arm64/kernel/module.lds
create mode 100644 drivers/firmware/efi/libstub/random.c
EFI_RNG_PROTOCOL on real hardware
=================================
To test whether your UEFI implements the EFI_RNG_PROTOCOL, download the
following executable and run it from the UEFI Shell:
http://people.linaro.org/~ard.biesheuvel/RngTest.efi
FS0:\> rngtest
UEFI RNG Protocol Testing :
----------------------------
-- Locate UEFI RNG Protocol : [Fail - Status = Not Found]
If your UEFI does not implement the EFI_RNG_PROTOCOL, you can download and
install the pseudo-random version that uses the generic timer and PMCCNTR_EL0
values and permutes them using a couple of rounds of AES.
http://people.linaro.org/~ard.biesheuvel/RngDxe.efi
NOTE: not for production!! This is a quick and dirty hack to test the KASLR
code, and is not suitable for anything else.
FS0:\> rngdxe
FS0:\> rngtest
UEFI RNG Protocol Testing :
----------------------------
-- Locate UEFI RNG Protocol : [Pass]
-- Call RNG->GetInfo() interface :
>> Supported RNG Algorithm (Count = 2) :
0) 44F0DE6E-4D8C-4045-A8C7-4DD168856B9E
1) E43176D7-B6E8-4827-B784-7FFDC4B68561
-- Call RNG->GetRNG() interface :
>> RNG with default algorithm : [Pass]
>> RNG with SP800-90-HMAC-256 : [Fail - Status = Unsupported]
>> RNG with SP800-90-Hash-256 : [Fail - Status = Unsupported]
>> RNG with SP800-90-CTR-256 : [Pass]
>> RNG with X9.31-3DES : [Fail - Status = Unsupported]
>> RNG with X9.31-AES : [Fail - Status = Unsupported]
>> RNG with RAW Entropy : [Pass]
-- Random Number Generation Test with default RNG Algorithm (20 Rounds):
01) - 27
02) - 61E8
03) - 496FD8
04) - DDD793BF
05) - B6C37C8E23
06) - 4D183C604A96
07) - 9363311DB61298
08) - 5715A7294F4E436E
09) - F0D4D7BAA0DD52318E
10) - C88C6EBCF4C0474D87C3
11) - B5594602B482A643932172
12) - CA7573F704B2089B726B9CF1
13) - A93E9451CB533DCFBA87B97C33
14) - 45AA7B83DB6044F7BBAB031F0D24
15) - 3DD7A4D61F34ADCB400B5976730DCF
16) - 4DD168D21FAB8F59708330D6A9BEB021
17) - 4BBB225E61C465F174254159467E65939F
18) - 030A156C9616337A20070941E702827DA8E1
19) - AB0FC11C9A4E225011382A9D164D9D55CA2B64
20) - 72B9B4735DC445E5DA6AF88DE965B7E87CB9A23C
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-12 12:37:22
On Mon, Jan 11, 2016 at 02:18:58PM +0100, Ard Biesheuvel wrote:
KVM on arm64 uses a fixed offset between the linear mapping at EL1 and
the HYP mapping at EL2. Before we can move the kernel virtual mapping
out of the linear mapping, we have to make sure that references to kernel
symbols that are accessed via the HYP mapping are translated to their
linear equivalent.
To prevent inadvertent direct references from sneaking in later, change
the type of all extern declarations to HYP kernel symbols to the opaque
'struct kvm_ksym', which does not decay to a pointer type like char arrays
and function references. This is not bullet proof, but at least forces the
user to take the address explicitly rather than referencing it directly.
Signed-off-by: Ard Biesheuvel <redacted>
So that one doesn't have to trawl git logs, it might be worth a comment
as to the purpose of struct kvm_ksym (and thus why we never need to
actually define it).
Either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Mark.
On 12 January 2016 at 13:36, Mark Rutland [off-list ref] wrote:
On Mon, Jan 11, 2016 at 02:18:58PM +0100, Ard Biesheuvel wrote:
quoted
KVM on arm64 uses a fixed offset between the linear mapping at EL1 and
the HYP mapping at EL2. Before we can move the kernel virtual mapping
out of the linear mapping, we have to make sure that references to kernel
symbols that are accessed via the HYP mapping are translated to their
linear equivalent.
To prevent inadvertent direct references from sneaking in later, change
the type of all extern declarations to HYP kernel symbols to the opaque
'struct kvm_ksym', which does not decay to a pointer type like char arrays
and function references. This is not bullet proof, but at least forces the
user to take the address explicitly rather than referencing it directly.
Signed-off-by: Ard Biesheuvel <redacted>
So that one doesn't have to trawl git logs, it might be worth a comment
as to the purpose of struct kvm_ksym (and thus why we never need to
actually define it).
Yes, I can add something
Either way:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
On 11 January 2016 at 18:40, Mark Rutland [off-list ref] wrote:
On Mon, Jan 11, 2016 at 02:18:56PM +0100, Ard Biesheuvel wrote:
quoted
Add definitions of pud_index() and pmd_index() for configurations with
fewer than 4 resp. 3 translation levels. This makes it easier to keep
the users (e.g., the fixmap init code) generic.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
I think we don't need these if we use p??_ofset_kimg for the fixmap
initialisation.
Regardless, these look good conceptually, so if they're useful
elsewhere:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks, but this can indeed be dropped after the proposed changes have
been made to the fixmap init code.
--
Ard.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-12 18:14:45
On Mon, Jan 11, 2016 at 02:19:00PM +0100, Ard Biesheuvel wrote:
quoted hunk
This moves the module area to right before the vmalloc area, and
moves the kernel image to the base of the vmalloc area. This is
an intermediate step towards implementing kASLR, where the kernel
image can be located anywhere in the vmalloc area.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kasan.h | 20 ++++---
arch/arm64/include/asm/kernel-pgtable.h | 5 +-
arch/arm64/include/asm/memory.h | 18 ++++--
arch/arm64/include/asm/pgtable.h | 7 ---
arch/arm64/kernel/setup.c | 12 ++++
arch/arm64/mm/dump.c | 12 ++--
arch/arm64/mm/init.c | 20 +++----
arch/arm64/mm/kasan_init.c | 21 +++++--
arch/arm64/mm/mmu.c | 62 ++++++++++++++------
9 files changed, 118 insertions(+), 59 deletions(-)
I couldn't immediately spot where KASAN_SHADOW_* were used in assembly.
I guess there's some other definition built atop of them that I've
missed.
Where should I be looking?
This will only affect the tables created in head.S. Before we start
userspace we'll have switched over to a new set of tables using
PAGE_KERNEL (including UXN).
Given that, this doesn't look necessary for the vmalloc area changes. Am
I missing something?
It's a shame VMALLOC_START and VMALLOC_END are now in different headers.
It would be nice if we could keep them together.
As VMEMMAP_SIZE depends on sizeof(struct page), it's not just a simple
move. We could either place that in the !__ASSEMBLY__ portion of
memory.h, or we could add S_PAGE to asm-offsets.
If that's too painful now, we can leave that for subsequent cleanup;
there's other stuff in that area I'd like to unify at some point (e.g.
the mem_init and dump.c section boundary descriptions).
With the fine grained tables we should only need to round up to
PAGE_SIZE (though _end is implicitly page-aligned anyway). Given that,
is the SWAPPER_BLOCK_SIZE rounding necessary?
@@ -126,8 +128,14 @@ static void __init clear_pgds(unsigned long start,void__initkasan_init(void){+u64kimg_shadow_start,kimg_shadow_end;structmemblock_region*reg;+kimg_shadow_start=round_down((u64)kasan_mem_to_shadow(_text),+SWAPPER_BLOCK_SIZE);+kimg_shadow_end=round_up((u64)kasan_mem_to_shadow(_end),+SWAPPER_BLOCK_SIZE);
This rounding looks suspect to me, given it's applied to the shadow
addresses rather than the kimage addresses. That's roughly equivalent to
kasan_mem_to_shadow(round_up(_end, 8 * SWAPPER_BLOCK_SIZE).
I don't think we need any rounding for the kimage addresses. The image
end is page-granular (and the fine-grained mapping will reflect that).
Any accesses between _end and roud_up(_end, SWAPPER_BLOCK_SIZE) would be
bugs (and would most likely fault) regardless of KASAN.
Or am I just being thick here?
quoted hunk
+
/*
* We are going to perform proper setup of shadow memory.
* At first we should unmap early shadow (clear_pgds() call bellow).
That virt_to_pfn doesn't look right -- kimg_shadow_start is neither a
linear address nor an image address. As pfn_to_nid is hard-coded to 0
for !NUMA this happens to be ok for us for the moment.
I think we should follow the x86 KASAN code and use NUMA_NO_NODE for
this for now.
@@ -349,14 +353,14 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end{unsignedlongkernel_start=__pa(_stext);-unsignedlongkernel_end=__pa(_end);+unsignedlongkernel_end=__pa(_etext);/*-*Thekernelitselfismappedatpagegranularity.Mapallother-*memory,makingsurewedon'toverwritetheexistingkernelmappings.+*Takecarenottocreateawritablealiasforthe+*read-onlytextandrodatasectionsofthekernelimage.*/-/* No overlap with the kernel. */+/* No overlap with the kernel text */if(end<kernel_start||start>=kernel_end){__create_pgd_mapping(pgd,start,__phys_to_virt(start),end-start,PAGE_KERNEL,
To match the style of early_fixmap_init, and given we already mapped the
kernel image, this could be:
if (pgd_none(pgd_offset_raw(pgd, FIXADDR_START))) {
Which also serves as a run-time check that the pgd entry really was
clear.
Other than that, this looks good to me!
Thanks,
Mark.
quoted hunk
+ /*
+ * The fixmap falls in a separate pgd to the kernel, and doesn't
+ * live in the carveout for the swapper_pg_dir. We can simply
+ * re-use the existing dir for the fixmap.
+ */
+ set_pgd(pgd_offset_raw(pgd, FIXADDR_START),
+ *pgd_offset_k(FIXADDR_START));
+ } else if (CONFIG_PGTABLE_LEVELS > 3) {
+ /*
+ * The fixmap shares its top level pgd entry with the kernel
+ * mapping. This can really only occur when we are running
+ * with 16k/4 levels, so we can simply reuse the pud level
+ * entry instead.
+ */
+ BUG_ON(!IS_ENABLED(CONFIG_ARM64_16K_PAGES));
+
+ set_pud(pud_set_fixmap_offset(pgd, FIXADDR_START),
+ __pud(__pa(bm_pmd) | PUD_TYPE_TABLE));
+ pud_clear_fixmap();
+ } else {
+ BUG();
+ }
kasan_copy_shadow(pgd);
}
On 12 January 2016 at 19:14, Mark Rutland [off-list ref] wrote:
On Mon, Jan 11, 2016 at 02:19:00PM +0100, Ard Biesheuvel wrote:
quoted
This moves the module area to right before the vmalloc area, and
moves the kernel image to the base of the vmalloc area. This is
an intermediate step towards implementing kASLR, where the kernel
image can be located anywhere in the vmalloc area.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kasan.h | 20 ++++---
arch/arm64/include/asm/kernel-pgtable.h | 5 +-
arch/arm64/include/asm/memory.h | 18 ++++--
arch/arm64/include/asm/pgtable.h | 7 ---
arch/arm64/kernel/setup.c | 12 ++++
arch/arm64/mm/dump.c | 12 ++--
arch/arm64/mm/init.c | 20 +++----
arch/arm64/mm/kasan_init.c | 21 +++++--
arch/arm64/mm/mmu.c | 62 ++++++++++++++------
9 files changed, 118 insertions(+), 59 deletions(-)
I couldn't immediately spot where KASAN_SHADOW_* were used in assembly.
I guess there's some other definition built atop of them that I've
missed.
Where should I be looking?
Well, the problem is that KIMAGE_VADDR will be defined in terms of
KASAN_SHADOW_END if KASAN is enabled. But since KASAN always uses the
first 1/8 of that VA space, I am going to rework this so that the
non-KASAN constants never depend on the actual values but only on
CONFIG_KASAN
This will only affect the tables created in head.S. Before we start
userspace we'll have switched over to a new set of tables using
PAGE_KERNEL (including UXN).
Given that, this doesn't look necessary for the vmalloc area changes. Am
I missing something?
No, this was carried over from an older version of the series, when
the kernel mapping, after having been moved below PAGE_OFFSET, would
not be overridden by the memblock based linear mapping routines, and
so would missing the UXN bit. But with your changes, this can indeed
be dropped.
It's a shame VMALLOC_START and VMALLOC_END are now in different headers.
It would be nice if we could keep them together.
As VMEMMAP_SIZE depends on sizeof(struct page), it's not just a simple
move. We could either place that in the !__ASSEMBLY__ portion of
memory.h, or we could add S_PAGE to asm-offsets.
If that's too painful now, we can leave that for subsequent cleanup;
there's other stuff in that area I'd like to unify at some point (e.g.
the mem_init and dump.c section boundary descriptions).
No, I think I can probably do a bit better than this. I will address it in v4
With the fine grained tables we should only need to round up to
PAGE_SIZE (though _end is implicitly page-aligned anyway). Given that,
is the SWAPPER_BLOCK_SIZE rounding necessary?
I was going to say we should set VM_KASAN also per its description in
include/vmalloc.h, though per its uses its not clear if it will ever
matter.
No, we shouldn't. Even if we are never going to unmap this vma,
setting the flag will result in the shadow area being freed using
vfree(), while it was not allocated via vmalloc() so that is likely to
cause trouble.
Do we need to register the kernel VA range quite this early, or could we
do this around paging_init/map_kernel time?
No. Locally, I moved it into map_kernel_chunk, so that we have
separate areas for _text, _init and _data, and we can unmap the _init
entirely rather than only stripping the exec bit. I haven't quite
figured out how to get rid of the vma area, but perhaps it make sense
to keep it reserved, so that modules don't end up there later (which
is possible with the module region randomization I have implemented
for v4) since I don't know how well things like kallsyms etc cope with
that.
@@ -126,8 +128,14 @@ static void __init clear_pgds(unsigned long start,void__initkasan_init(void){+u64kimg_shadow_start,kimg_shadow_end;structmemblock_region*reg;+kimg_shadow_start=round_down((u64)kasan_mem_to_shadow(_text),+SWAPPER_BLOCK_SIZE);+kimg_shadow_end=round_up((u64)kasan_mem_to_shadow(_end),+SWAPPER_BLOCK_SIZE);
This rounding looks suspect to me, given it's applied to the shadow
addresses rather than the kimage addresses. That's roughly equivalent to
kasan_mem_to_shadow(round_up(_end, 8 * SWAPPER_BLOCK_SIZE).
I don't think we need any rounding for the kimage addresses. The image
end is page-granular (and the fine-grained mapping will reflect that).
Any accesses between _end and roud_up(_end, SWAPPER_BLOCK_SIZE) would be
bugs (and would most likely fault) regardless of KASAN.
Or am I just being thick here?
Well, the problem here is that vmemmap_populate() is used as a
surrogate vmalloc() since that is not available yet, and
vmemmap_populate() allocates in SWAPPER_BLOCK_SIZE granularity.
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
I do wonder what that means for memblocks that are not multiples of 16
MB, though (below)
quoted
+
/*
* We are going to perform proper setup of shadow memory.
* At first we should unmap early shadow (clear_pgds() call bellow).
That virt_to_pfn doesn't look right -- kimg_shadow_start is neither a
linear address nor an image address. As pfn_to_nid is hard-coded to 0
for !NUMA this happens to be ok for us for the moment.
I think we should follow the x86 KASAN code and use NUMA_NO_NODE for
this for now.
@@ -349,14 +353,14 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end{unsignedlongkernel_start=__pa(_stext);-unsignedlongkernel_end=__pa(_end);+unsignedlongkernel_end=__pa(_etext);/*-*Thekernelitselfismappedatpagegranularity.Mapallother-*memory,makingsurewedon'toverwritetheexistingkernelmappings.+*Takecarenottocreateawritablealiasforthe+*read-onlytextandrodatasectionsofthekernelimage.*/-/* No overlap with the kernel. */+/* No overlap with the kernel text */if(end<kernel_start||start>=kernel_end){__create_pgd_mapping(pgd,start,__phys_to_virt(start),end-start,PAGE_KERNEL,
To match the style of early_fixmap_init, and given we already mapped the
kernel image, this could be:
if (pgd_none(pgd_offset_raw(pgd, FIXADDR_START))) {
Which also serves as a run-time check that the pgd entry really was
clear.
Yes, that looks better. I will steal that :-)
Other than that, this looks good to me!
Thanks!
quoted
+ /*
+ * The fixmap falls in a separate pgd to the kernel, and doesn't
+ * live in the carveout for the swapper_pg_dir. We can simply
+ * re-use the existing dir for the fixmap.
+ */
+ set_pgd(pgd_offset_raw(pgd, FIXADDR_START),
+ *pgd_offset_k(FIXADDR_START));
+ } else if (CONFIG_PGTABLE_LEVELS > 3) {
+ /*
+ * The fixmap shares its top level pgd entry with the kernel
+ * mapping. This can really only occur when we are running
+ * with 16k/4 levels, so we can simply reuse the pud level
+ * entry instead.
+ */
+ BUG_ON(!IS_ENABLED(CONFIG_ARM64_16K_PAGES));
+
+ set_pud(pud_set_fixmap_offset(pgd, FIXADDR_START),
+ __pud(__pa(bm_pmd) | PUD_TYPE_TABLE));
+ pud_clear_fixmap();
+ } else {
+ BUG();
+ }
kasan_copy_shadow(pgd);
}
On 13 January 2016 at 09:39, Ard Biesheuvel [off-list ref] wrote:
On 12 January 2016 at 19:14, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:00PM +0100, Ard Biesheuvel wrote:
quoted
This moves the module area to right before the vmalloc area, and
moves the kernel image to the base of the vmalloc area. This is
an intermediate step towards implementing kASLR, where the kernel
image can be located anywhere in the vmalloc area.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kasan.h | 20 ++++---
arch/arm64/include/asm/kernel-pgtable.h | 5 +-
arch/arm64/include/asm/memory.h | 18 ++++--
arch/arm64/include/asm/pgtable.h | 7 ---
arch/arm64/kernel/setup.c | 12 ++++
arch/arm64/mm/dump.c | 12 ++--
arch/arm64/mm/init.c | 20 +++----
arch/arm64/mm/kasan_init.c | 21 +++++--
arch/arm64/mm/mmu.c | 62 ++++++++++++++------
9 files changed, 118 insertions(+), 59 deletions(-)
I couldn't immediately spot where KASAN_SHADOW_* were used in assembly.
I guess there's some other definition built atop of them that I've
missed.
Where should I be looking?
Well, the problem is that KIMAGE_VADDR will be defined in terms of
KASAN_SHADOW_END if KASAN is enabled. But since KASAN always uses the
first 1/8 of that VA space, I am going to rework this so that the
non-KASAN constants never depend on the actual values but only on
CONFIG_KASAN
This will only affect the tables created in head.S. Before we start
userspace we'll have switched over to a new set of tables using
PAGE_KERNEL (including UXN).
Given that, this doesn't look necessary for the vmalloc area changes. Am
I missing something?
No, this was carried over from an older version of the series, when
the kernel mapping, after having been moved below PAGE_OFFSET, would
not be overridden by the memblock based linear mapping routines, and
so would missing the UXN bit. But with your changes, this can indeed
be dropped.
It's a shame VMALLOC_START and VMALLOC_END are now in different headers.
It would be nice if we could keep them together.
As VMEMMAP_SIZE depends on sizeof(struct page), it's not just a simple
move. We could either place that in the !__ASSEMBLY__ portion of
memory.h, or we could add S_PAGE to asm-offsets.
If that's too painful now, we can leave that for subsequent cleanup;
there's other stuff in that area I'd like to unify at some point (e.g.
the mem_init and dump.c section boundary descriptions).
No, I think I can probably do a bit better than this. I will address it in v4
With the fine grained tables we should only need to round up to
PAGE_SIZE (though _end is implicitly page-aligned anyway). Given that,
is the SWAPPER_BLOCK_SIZE rounding necessary?
I was going to say we should set VM_KASAN also per its description in
include/vmalloc.h, though per its uses its not clear if it will ever
matter.
No, we shouldn't. Even if we are never going to unmap this vma,
setting the flag will result in the shadow area being freed using
vfree(), while it was not allocated via vmalloc() so that is likely to
cause trouble.
Do we need to register the kernel VA range quite this early, or could we
do this around paging_init/map_kernel time?
No. Locally, I moved it into map_kernel_chunk, so that we have
separate areas for _text, _init and _data, and we can unmap the _init
entirely rather than only stripping the exec bit. I haven't quite
figured out how to get rid of the vma area, but perhaps it make sense
to keep it reserved, so that modules don't end up there later (which
is possible with the module region randomization I have implemented
for v4) since I don't know how well things like kallsyms etc cope with
that.
@@ -126,8 +128,14 @@ static void __init clear_pgds(unsigned long start,void__initkasan_init(void){+u64kimg_shadow_start,kimg_shadow_end;structmemblock_region*reg;+kimg_shadow_start=round_down((u64)kasan_mem_to_shadow(_text),+SWAPPER_BLOCK_SIZE);+kimg_shadow_end=round_up((u64)kasan_mem_to_shadow(_end),+SWAPPER_BLOCK_SIZE);
This rounding looks suspect to me, given it's applied to the shadow
addresses rather than the kimage addresses. That's roughly equivalent to
kasan_mem_to_shadow(round_up(_end, 8 * SWAPPER_BLOCK_SIZE).
I don't think we need any rounding for the kimage addresses. The image
end is page-granular (and the fine-grained mapping will reflect that).
Any accesses between _end and roud_up(_end, SWAPPER_BLOCK_SIZE) would be
bugs (and would most likely fault) regardless of KASAN.
Or am I just being thick here?
Well, the problem here is that vmemmap_populate() is used as a
surrogate vmalloc() since that is not available yet, and
vmemmap_populate() allocates in SWAPPER_BLOCK_SIZE granularity.
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
I do wonder what that means for memblocks that are not multiples of 16
MB, though (below)
quoted
quoted
+
/*
* We are going to perform proper setup of shadow memory.
* At first we should unmap early shadow (clear_pgds() call bellow).
That virt_to_pfn doesn't look right -- kimg_shadow_start is neither a
linear address nor an image address. As pfn_to_nid is hard-coded to 0
for !NUMA this happens to be ok for us for the moment.
I think we should follow the x86 KASAN code and use NUMA_NO_NODE for
this for now.
@@ -349,14 +353,14 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end{unsignedlongkernel_start=__pa(_stext);-unsignedlongkernel_end=__pa(_end);+unsignedlongkernel_end=__pa(_etext);/*-*Thekernelitselfismappedatpagegranularity.Mapallother-*memory,makingsurewedon'toverwritetheexistingkernelmappings.+*Takecarenottocreateawritablealiasforthe+*read-onlytextandrodatasectionsofthekernelimage.*/-/* No overlap with the kernel. */+/* No overlap with the kernel text */if(end<kernel_start||start>=kernel_end){__create_pgd_mapping(pgd,start,__phys_to_virt(start),end-start,PAGE_KERNEL,
To match the style of early_fixmap_init, and given we already mapped the
kernel image, this could be:
if (pgd_none(pgd_offset_raw(pgd, FIXADDR_START))) {
Which also serves as a run-time check that the pgd entry really was
clear.
Yes, that looks better. I will steal that :-)
OK, that doesn't work. pgd_none() is hardcoded to 'false' when running
with fewer than 4 pgtable levels, and so we always hit the BUG() here.
quoted
Other than that, this looks good to me!
Thanks!
quoted
quoted
+ /*
+ * The fixmap falls in a separate pgd to the kernel, and doesn't
+ * live in the carveout for the swapper_pg_dir. We can simply
+ * re-use the existing dir for the fixmap.
+ */
+ set_pgd(pgd_offset_raw(pgd, FIXADDR_START),
+ *pgd_offset_k(FIXADDR_START));
+ } else if (CONFIG_PGTABLE_LEVELS > 3) {
+ /*
+ * The fixmap shares its top level pgd entry with the kernel
+ * mapping. This can really only occur when we are running
+ * with 16k/4 levels, so we can simply reuse the pud level
+ * entry instead.
+ */
+ BUG_ON(!IS_ENABLED(CONFIG_ARM64_16K_PAGES));
+
+ set_pud(pud_set_fixmap_offset(pgd, FIXADDR_START),
+ __pud(__pa(bm_pmd) | PUD_TYPE_TABLE));
+ pud_clear_fixmap();
+ } else {
+ BUG();
+ }
kasan_copy_shadow(pgd);
}
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-13 11:12:17
On Wed, Jan 13, 2016 at 10:58:55AM +0100, Ard Biesheuvel wrote:
On 13 January 2016 at 09:39, Ard Biesheuvel [off-list ref] wrote:
quoted
On 12 January 2016 at 19:14, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:00PM +0100, Ard Biesheuvel wrote:
quoted
@@ -438,12 +442,29 @@ static void __init map_kernel(pgd_t *pgd) map_kernel_chunk(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC); map_kernel_chunk(pgd, _data, _end, PAGE_KERNEL);- /*- * The fixmap falls in a separate pgd to the kernel, and doesn't live- * in the carveout for the swapper_pg_dir. We can simply re-use the- * existing dir for the fixmap.- */- set_pgd(pgd_offset_raw(pgd, FIXADDR_START), *pgd_offset_k(FIXADDR_START));+ if (pgd_index(FIXADDR_START) != pgd_index((u64)_end)) {
To match the style of early_fixmap_init, and given we already mapped the
kernel image, this could be:
if (pgd_none(pgd_offset_raw(pgd, FIXADDR_START))) {
Which also serves as a run-time check that the pgd entry really was
clear.
Yes, that looks better. I will steal that :-)
OK, that doesn't work. pgd_none() is hardcoded to 'false' when running
with fewer than 4 pgtable levels, and so we always hit the BUG() here.
Ah, sorry.
We could also check CONFIG_PGTABLE_LEVELS > 3 check, as with
fixmap_init, perhaps?
Thanks,
Mark.
On 13 January 2016 at 12:11, Mark Rutland [off-list ref] wrote:
On Wed, Jan 13, 2016 at 10:58:55AM +0100, Ard Biesheuvel wrote:
quoted
On 13 January 2016 at 09:39, Ard Biesheuvel [off-list ref] wrote:
quoted
On 12 January 2016 at 19:14, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:00PM +0100, Ard Biesheuvel wrote:
quoted
@@ -438,12 +442,29 @@ static void __init map_kernel(pgd_t *pgd) map_kernel_chunk(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC); map_kernel_chunk(pgd, _data, _end, PAGE_KERNEL);- /*- * The fixmap falls in a separate pgd to the kernel, and doesn't live- * in the carveout for the swapper_pg_dir. We can simply re-use the- * existing dir for the fixmap.- */- set_pgd(pgd_offset_raw(pgd, FIXADDR_START), *pgd_offset_k(FIXADDR_START));+ if (pgd_index(FIXADDR_START) != pgd_index((u64)_end)) {
To match the style of early_fixmap_init, and given we already mapped the
kernel image, this could be:
if (pgd_none(pgd_offset_raw(pgd, FIXADDR_START))) {
Which also serves as a run-time check that the pgd entry really was
clear.
Yes, that looks better. I will steal that :-)
OK, that doesn't work. pgd_none() is hardcoded to 'false' when running
with fewer than 4 pgtable levels, and so we always hit the BUG() here.
Ah, sorry.
We could also check CONFIG_PGTABLE_LEVELS > 3 check, as with
fixmap_init, perhaps?
I'm using this now:
if (!pgd_val(*pgd_offset_raw(pgd, FIXADDR_START))) {
which I think is appropriate, since we don't expect to share any top
level entry, folded or not.
--
Ard.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-13 13:51:36
On Wed, Jan 13, 2016 at 09:39:41AM +0100, Ard Biesheuvel wrote:
On 12 January 2016 at 19:14, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:00PM +0100, Ard Biesheuvel wrote:
quoted
This moves the module area to right before the vmalloc area, and
moves the kernel image to the base of the vmalloc area. This is
an intermediate step towards implementing kASLR, where the kernel
image can be located anywhere in the vmalloc area.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kasan.h | 20 ++++---
arch/arm64/include/asm/kernel-pgtable.h | 5 +-
arch/arm64/include/asm/memory.h | 18 ++++--
arch/arm64/include/asm/pgtable.h | 7 ---
arch/arm64/kernel/setup.c | 12 ++++
arch/arm64/mm/dump.c | 12 ++--
arch/arm64/mm/init.c | 20 +++----
arch/arm64/mm/kasan_init.c | 21 +++++--
arch/arm64/mm/mmu.c | 62 ++++++++++++++------
9 files changed, 118 insertions(+), 59 deletions(-)
I couldn't immediately spot where KASAN_SHADOW_* were used in assembly.
I guess there's some other definition built atop of them that I've
missed.
Where should I be looking?
Well, the problem is that KIMAGE_VADDR will be defined in terms of
KASAN_SHADOW_END if KASAN is enabled.
Ah. I'd somehow managed to overlook that. Thanks for pointing that out!
But since KASAN always uses the first 1/8 of that VA space, I am going
to rework this so that the non-KASAN constants never depend on the
actual values but only on CONFIG_KASAN
Personally I'd prefer that they were obviously defined in terms of each
other if possible (as this means that the definitions are obviously
consistent by construction).
So if it's not too much of a pain to keep them that way it would be
nice to do so.
[...]
quoted
quoted
+ vmlinux_vm.flags = VM_MAP;
I was going to say we should set VM_KASAN also per its description in
include/vmalloc.h, though per its uses its not clear if it will ever
matter.
No, we shouldn't. Even if we are never going to unmap this vma,
setting the flag will result in the shadow area being freed using
vfree(), while it was not allocated via vmalloc() so that is likely to
cause trouble.
Ok.
quoted
quoted
+ vm_area_add_early(&vmlinux_vm);
Do we need to register the kernel VA range quite this early, or could we
do this around paging_init/map_kernel time?
No. Locally, I moved it into map_kernel_chunk, so that we have
separate areas for _text, _init and _data, and we can unmap the _init
entirely rather than only stripping the exec bit. I haven't quite
figured out how to get rid of the vma area, but perhaps it make sense
to keep it reserved, so that modules don't end up there later (which
is possible with the module region randomization I have implemented
for v4) since I don't know how well things like kallsyms etc cope with
that.
Keeping that reserved sounds reasonable to me.
[...]
This rounding looks suspect to me, given it's applied to the shadow
addresses rather than the kimage addresses. That's roughly equivalent to
kasan_mem_to_shadow(round_up(_end, 8 * SWAPPER_BLOCK_SIZE).
I don't think we need any rounding for the kimage addresses. The image
end is page-granular (and the fine-grained mapping will reflect that).
Any accesses between _end and roud_up(_end, SWAPPER_BLOCK_SIZE) would be
bugs (and would most likely fault) regardless of KASAN.
Or am I just being thick here?
Well, the problem here is that vmemmap_populate() is used as a
surrogate vmalloc() since that is not available yet, and
vmemmap_populate() allocates in SWAPPER_BLOCK_SIZE granularity.
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
Ah. :(
I'll also take a peek.
I do wonder what that means for memblocks that are not multiples of 16
MB, though (below)
Indeed.
On a related note, something I've been thinking about is PA layout
fuzzing using VMs.
It sounds like being able to test memory layouts would be useful for
cases like the above, and I suspect there are plenty of other edge cases
that we aren't yet aware of due to typical physical memory layouts being
fairly simple.
It doesn't seem to be possible to force a particular physical memory
layout (and particular kernel, dtb, etc addresses) for QEMU or KVM
tool. I started looking into adding support to KVM tool, but there's a
fair amount of refactoring needed first.
Another option might be a special EFI application that carves up memory
in a deliberate fashion to ensure particular fragmentation cases (e.g. a
bank that's SWAPPER_BLOCK_SIZE - PAGE_SIZE in length).
Thanks,
Mark.
On 13 January 2016 at 14:51, Mark Rutland [off-list ref] wrote:
On Wed, Jan 13, 2016 at 09:39:41AM +0100, Ard Biesheuvel wrote:
quoted
On 12 January 2016 at 19:14, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:00PM +0100, Ard Biesheuvel wrote:
quoted
This moves the module area to right before the vmalloc area, and
moves the kernel image to the base of the vmalloc area. This is
an intermediate step towards implementing kASLR, where the kernel
image can be located anywhere in the vmalloc area.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/include/asm/kasan.h | 20 ++++---
arch/arm64/include/asm/kernel-pgtable.h | 5 +-
arch/arm64/include/asm/memory.h | 18 ++++--
arch/arm64/include/asm/pgtable.h | 7 ---
arch/arm64/kernel/setup.c | 12 ++++
arch/arm64/mm/dump.c | 12 ++--
arch/arm64/mm/init.c | 20 +++----
arch/arm64/mm/kasan_init.c | 21 +++++--
arch/arm64/mm/mmu.c | 62 ++++++++++++++------
9 files changed, 118 insertions(+), 59 deletions(-)
I couldn't immediately spot where KASAN_SHADOW_* were used in assembly.
I guess there's some other definition built atop of them that I've
missed.
Where should I be looking?
Well, the problem is that KIMAGE_VADDR will be defined in terms of
KASAN_SHADOW_END if KASAN is enabled.
Ah. I'd somehow managed to overlook that. Thanks for pointing that out!
quoted
But since KASAN always uses the first 1/8 of that VA space, I am going
to rework this so that the non-KASAN constants never depend on the
actual values but only on CONFIG_KASAN
Personally I'd prefer that they were obviously defined in terms of each
other if possible (as this means that the definitions are obviously
consistent by construction).
So if it's not too much of a pain to keep them that way it would be
nice to do so.
[...]
I am leaning towards adding this to asm/memory.h
#ifdef CONFIG_KASAN
#define KASAN_SHADOW_SIZE (UL(1) << (VA_BITS - 3))
#else
#define KASAN_SHADOW_SIZE (0)
#endif
and remove the #ifdef CONFIG_KASAN block from asm/pgtable.h. Then
asm/kasan.h, which already includes asm/memory.h, can use it as region
size, and none of the reshuffling I had to do before is necessary.
quoted
quoted
quoted
+ vmlinux_vm.flags = VM_MAP;
I was going to say we should set VM_KASAN also per its description in
include/vmalloc.h, though per its uses its not clear if it will ever
matter.
No, we shouldn't. Even if we are never going to unmap this vma,
setting the flag will result in the shadow area being freed using
vfree(), while it was not allocated via vmalloc() so that is likely to
cause trouble.
Ok.
quoted
quoted
quoted
+ vm_area_add_early(&vmlinux_vm);
Do we need to register the kernel VA range quite this early, or could we
do this around paging_init/map_kernel time?
No. Locally, I moved it into map_kernel_chunk, so that we have
separate areas for _text, _init and _data, and we can unmap the _init
entirely rather than only stripping the exec bit. I haven't quite
figured out how to get rid of the vma area, but perhaps it make sense
to keep it reserved, so that modules don't end up there later (which
is possible with the module region randomization I have implemented
for v4) since I don't know how well things like kallsyms etc cope with
that.
Keeping that reserved sounds reasonable to me.
[...]
This rounding looks suspect to me, given it's applied to the shadow
addresses rather than the kimage addresses. That's roughly equivalent to
kasan_mem_to_shadow(round_up(_end, 8 * SWAPPER_BLOCK_SIZE).
I don't think we need any rounding for the kimage addresses. The image
end is page-granular (and the fine-grained mapping will reflect that).
Any accesses between _end and roud_up(_end, SWAPPER_BLOCK_SIZE) would be
bugs (and would most likely fault) regardless of KASAN.
Or am I just being thick here?
Well, the problem here is that vmemmap_populate() is used as a
surrogate vmalloc() since that is not available yet, and
vmemmap_populate() allocates in SWAPPER_BLOCK_SIZE granularity.
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
Ah. :(
I'll also take a peek.
Yes, please.
quoted
I do wonder what that means for memblocks that are not multiples of 16
MB, though (below)
Indeed.
On a related note, something I've been thinking about is PA layout
fuzzing using VMs.
It sounds like being able to test memory layouts would be useful for
cases like the above, and I suspect there are plenty of other edge cases
that we aren't yet aware of due to typical physical memory layouts being
fairly simple.
It doesn't seem to be possible to force a particular physical memory
layout (and particular kernel, dtb, etc addresses) for QEMU or KVM
tool. I started looking into adding support to KVM tool, but there's a
fair amount of refactoring needed first.
Another option might be a special EFI application that carves up memory
in a deliberate fashion to ensure particular fragmentation cases (e.g. a
bank that's SWAPPER_BLOCK_SIZE - PAGE_SIZE in length).
I use mem= for this, in fact, and boot most of my machines and VMs
with some value slightly below the actual available DRAM that is not a
multiple of 2M
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-13 16:26:42
On Wed, Jan 13, 2016 at 04:50:24PM +0100, Ard Biesheuvel wrote:
On 13 January 2016 at 14:51, Mark Rutland [off-list ref] wrote:
quoted
On Wed, Jan 13, 2016 at 09:39:41AM +0100, Ard Biesheuvel wrote:
quoted
But since KASAN always uses the first 1/8 of that VA space, I am going
to rework this so that the non-KASAN constants never depend on the
actual values but only on CONFIG_KASAN
Personally I'd prefer that they were obviously defined in terms of each
other if possible (as this means that the definitions are obviously
consistent by construction).
So if it's not too much of a pain to keep them that way it would be
nice to do so.
[...]
I am leaning towards adding this to asm/memory.h
#ifdef CONFIG_KASAN
#define KASAN_SHADOW_SIZE (UL(1) << (VA_BITS - 3))
#else
#define KASAN_SHADOW_SIZE (0)
#endif
and remove the #ifdef CONFIG_KASAN block from asm/pgtable.h. Then
asm/kasan.h, which already includes asm/memory.h, can use it as region
size, and none of the reshuffling I had to do before is necessary.
FWIW, that looks good to me.
[...]
quoted
quoted
I do wonder what that means for memblocks that are not multiples of 16
MB, though (below)
Indeed.
On a related note, something I've been thinking about is PA layout
fuzzing using VMs.
It sounds like being able to test memory layouts would be useful for
cases like the above, and I suspect there are plenty of other edge cases
that we aren't yet aware of due to typical physical memory layouts being
fairly simple.
It doesn't seem to be possible to force a particular physical memory
layout (and particular kernel, dtb, etc addresses) for QEMU or KVM
tool. I started looking into adding support to KVM tool, but there's a
fair amount of refactoring needed first.
Another option might be a special EFI application that carves up memory
in a deliberate fashion to ensure particular fragmentation cases (e.g. a
bank that's SWAPPER_BLOCK_SIZE - PAGE_SIZE in length).
I use mem= for this, in fact, and boot most of my machines and VMs
with some value slightly below the actual available DRAM that is not a
multiple of 2M
Sure. I do some testing with mem= to find some bugs. The problem is that
you can only vary the end address (prior to your patches), and don't get
much variation on the portions in the middle.
It's difficult to test for bugs with not-quite-adjacent regions, or
where particular sizes, alignment, or addresses are important.
For example, having memory that extends right to the end of the
kernel-supported PA range (even with gaps in the middle) is an edge case
we don't currently test. I have a suspicion that KASAN's shadow
lookahead (and allocation to account for this) wouldn't be quite right,
and I want to be able to properly test and verify that.
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-13 18:12:38
On Mon, Jan 11, 2016 at 02:19:04PM +0100, Ard Biesheuvel wrote:
Unfortunately, the current way of using the linker to emit build time
constants into the Image header will no longer work once we switch to
the use of PIE executables. The reason is that such constants are emitted
into the binary using R_AARCH64_ABS64 relocations, which we will resolve
at runtime, not at build time, and the places targeted by those
relocations will contain zeroes before that.
So move back to assembly time constants or R_AARCH64_ABS32 relocations
(which, interestingly enough, do get resolved at build time)
To me it seems very odd that ABS64 and ABS32 are treated differently,
and it makes me somewhat uncomfortable becuase it feels like a bug.
Do we know whether the inconsistency between ABS64 and ABS32 was
deliberate?
I couldn't spot anything declaring a difference in the AArch64 ELF
spec, and I'm not sure where else to look.
Thanks,
Mark.
On 13 January 2016 at 19:12, Mark Rutland [off-list ref] wrote:
On Mon, Jan 11, 2016 at 02:19:04PM +0100, Ard Biesheuvel wrote:
quoted
Unfortunately, the current way of using the linker to emit build time
constants into the Image header will no longer work once we switch to
the use of PIE executables. The reason is that such constants are emitted
into the binary using R_AARCH64_ABS64 relocations, which we will resolve
at runtime, not at build time, and the places targeted by those
relocations will contain zeroes before that.
So move back to assembly time constants or R_AARCH64_ABS32 relocations
(which, interestingly enough, do get resolved at build time)
To me it seems very odd that ABS64 and ABS32 are treated differently,
and it makes me somewhat uncomfortable becuase it feels like a bug.
Do we know whether the inconsistency between ABS64 and ABS32 was
deliberate?
I couldn't spot anything declaring a difference in the AArch64 ELF
spec, and I'm not sure where else to look.
My assumption is that PIE only defers resolving R_AARCH64_ABS64
relocations since those are the only ones that be used to refer to
memory addresses
On 13 January 2016 at 19:48, Ard Biesheuvel [off-list ref] wrote:
On 13 January 2016 at 19:12, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:04PM +0100, Ard Biesheuvel wrote:
quoted
Unfortunately, the current way of using the linker to emit build time
constants into the Image header will no longer work once we switch to
the use of PIE executables. The reason is that such constants are emitted
into the binary using R_AARCH64_ABS64 relocations, which we will resolve
at runtime, not at build time, and the places targeted by those
relocations will contain zeroes before that.
So move back to assembly time constants or R_AARCH64_ABS32 relocations
(which, interestingly enough, do get resolved at build time)
To me it seems very odd that ABS64 and ABS32 are treated differently,
and it makes me somewhat uncomfortable becuase it feels like a bug.
Do we know whether the inconsistency between ABS64 and ABS32 was
deliberate?
I couldn't spot anything declaring a difference in the AArch64 ELF
spec, and I'm not sure where else to look.
My assumption is that PIE only defers resolving R_AARCH64_ABS64
relocations since those are the only ones that can be used to refer to
memory addresses
OK, digging into the binutils source code, it turns out that indeed,
ABSnn relocations where nn equals the ELFnn memory size are treated
differently, but only if they have default visibility. This is simply
a result of the fact the code path is shared between shared libraries
and PIE executables, since PIE executable are fully linked. It also
means that we can simply work around it by emitting the linker symbols
as hidden.
On 14 January 2016 at 09:51, Ard Biesheuvel [off-list ref] wrote:
On 13 January 2016 at 19:48, Ard Biesheuvel [off-list ref] wrote:
quoted
On 13 January 2016 at 19:12, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:04PM +0100, Ard Biesheuvel wrote:
quoted
Unfortunately, the current way of using the linker to emit build time
constants into the Image header will no longer work once we switch to
the use of PIE executables. The reason is that such constants are emitted
into the binary using R_AARCH64_ABS64 relocations, which we will resolve
at runtime, not at build time, and the places targeted by those
relocations will contain zeroes before that.
So move back to assembly time constants or R_AARCH64_ABS32 relocations
(which, interestingly enough, do get resolved at build time)
To me it seems very odd that ABS64 and ABS32 are treated differently,
and it makes me somewhat uncomfortable becuase it feels like a bug.
Do we know whether the inconsistency between ABS64 and ABS32 was
deliberate?
I couldn't spot anything declaring a difference in the AArch64 ELF
spec, and I'm not sure where else to look.
My assumption is that PIE only defers resolving R_AARCH64_ABS64
relocations since those are the only ones that can be used to refer to
memory addresses
OK, digging into the binutils source code, it turns out that indeed,
ABSnn relocations where nn equals the ELFnn memory size are treated
differently, but only if they have default visibility. This is simply
a result of the fact the code path is shared between shared libraries
and PIE executables, since PIE executable are fully linked. It also
means that we can simply work around it by emitting the linker symbols
as hidden.
... and the bad news is that, while emitting the symbols as hidden
turns them from R_AARCH64_ABS64 into a R_AARCH64_RELATIVE relocations,
it does not actually force the value to be emitted at build time.
So I am going to stick with the patch, but elaborate in a comment
about why R_AARCH64_ABSnn are treated differently if nn equals the
pointer size. (look at elfNN_aarch64_final_link_relocate() in binutils
if you are keen to look at the code yourself)
--
Ard.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-14 10:47:23
On Thu, Jan 14, 2016 at 10:05:42AM +0100, Ard Biesheuvel wrote:
On 14 January 2016 at 09:51, Ard Biesheuvel [off-list ref] wrote:
quoted
On 13 January 2016 at 19:48, Ard Biesheuvel [off-list ref] wrote:
quoted
On 13 January 2016 at 19:12, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:04PM +0100, Ard Biesheuvel wrote:
quoted
Unfortunately, the current way of using the linker to emit build time
constants into the Image header will no longer work once we switch to
the use of PIE executables. The reason is that such constants are emitted
into the binary using R_AARCH64_ABS64 relocations, which we will resolve
at runtime, not at build time, and the places targeted by those
relocations will contain zeroes before that.
So move back to assembly time constants or R_AARCH64_ABS32 relocations
(which, interestingly enough, do get resolved at build time)
To me it seems very odd that ABS64 and ABS32 are treated differently,
and it makes me somewhat uncomfortable becuase it feels like a bug.
Do we know whether the inconsistency between ABS64 and ABS32 was
deliberate?
I couldn't spot anything declaring a difference in the AArch64 ELF
spec, and I'm not sure where else to look.
My assumption is that PIE only defers resolving R_AARCH64_ABS64
relocations since those are the only ones that can be used to refer to
memory addresses
OK, digging into the binutils source code, it turns out that indeed,
ABSnn relocations where nn equals the ELFnn memory size are treated
differently, but only if they have default visibility. This is simply
a result of the fact the code path is shared between shared libraries
and PIE executables, since PIE executable are fully linked. It also
means that we can simply work around it by emitting the linker symbols
as hidden.
... and the bad news is that, while emitting the symbols as hidden
turns them from R_AARCH64_ABS64 into a R_AARCH64_RELATIVE relocations,
it does not actually force the value to be emitted at build time.
So I am going to stick with the patch, but elaborate in a comment
about why R_AARCH64_ABSnn are treated differently if nn equals the
pointer size. (look at elfNN_aarch64_final_link_relocate() in binutils
if you are keen to look at the code yourself)
Ok. Thanks for digging into that.
One thing though: I would prefer if we could still keep all the LE64
image header values together, to have them dealt with consistently.
Could we hide the ABS32 usage behind some macros to do so, e.g.
in image.h:
#define DEFINE_IMAGE_LE64(sym, data) \
sym##_lo32 = DATA_LE32(data & 0xffffffff); \
sym##_hi32 = DATA_LE32(data >> 32);
#define HEAD_SYMBOLS \
DEFINE_IMAGE_LE64(_kernel_size_le, _end - _text); \
DEFINE_IMAGE_LE64(_kernel_offset_le, TEXT_OFFSET); \
DEFINE_IMAGE_LE64(_kernel_flags_le, __HEAD_FLAGS);
and in head.S:
#define IMAGE_LE64(sym) .long sym##_lo32, sym##_hi32
...
IMAGE_LE64(_kernel_size_le) // Image load offset from start of RAM, little-endian
IMAGE_LE64(_kernel_offset_le) // Effective size of kernel image, little-endian
IMAGE_LE64(_kernel_flags_le) // Informative flags, little-endian
...
Thanks,
Mark.
On 14 January 2016 at 11:46, Mark Rutland [off-list ref] wrote:
On Thu, Jan 14, 2016 at 10:05:42AM +0100, Ard Biesheuvel wrote:
quoted
On 14 January 2016 at 09:51, Ard Biesheuvel [off-list ref] wrote:
quoted
On 13 January 2016 at 19:48, Ard Biesheuvel [off-list ref] wrote:
quoted
On 13 January 2016 at 19:12, Mark Rutland [off-list ref] wrote:
quoted
On Mon, Jan 11, 2016 at 02:19:04PM +0100, Ard Biesheuvel wrote:
quoted
Unfortunately, the current way of using the linker to emit build time
constants into the Image header will no longer work once we switch to
the use of PIE executables. The reason is that such constants are emitted
into the binary using R_AARCH64_ABS64 relocations, which we will resolve
at runtime, not at build time, and the places targeted by those
relocations will contain zeroes before that.
So move back to assembly time constants or R_AARCH64_ABS32 relocations
(which, interestingly enough, do get resolved at build time)
To me it seems very odd that ABS64 and ABS32 are treated differently,
and it makes me somewhat uncomfortable becuase it feels like a bug.
Do we know whether the inconsistency between ABS64 and ABS32 was
deliberate?
I couldn't spot anything declaring a difference in the AArch64 ELF
spec, and I'm not sure where else to look.
My assumption is that PIE only defers resolving R_AARCH64_ABS64
relocations since those are the only ones that can be used to refer to
memory addresses
OK, digging into the binutils source code, it turns out that indeed,
ABSnn relocations where nn equals the ELFnn memory size are treated
differently, but only if they have default visibility. This is simply
a result of the fact the code path is shared between shared libraries
and PIE executables, since PIE executable are fully linked. It also
means that we can simply work around it by emitting the linker symbols
as hidden.
... and the bad news is that, while emitting the symbols as hidden
turns them from R_AARCH64_ABS64 into a R_AARCH64_RELATIVE relocations,
it does not actually force the value to be emitted at build time.
So I am going to stick with the patch, but elaborate in a comment
about why R_AARCH64_ABSnn are treated differently if nn equals the
pointer size. (look at elfNN_aarch64_final_link_relocate() in binutils
if you are keen to look at the code yourself)
Ok. Thanks for digging into that.
One thing though: I would prefer if we could still keep all the LE64
image header values together, to have them dealt with consistently.
Could we hide the ABS32 usage behind some macros to do so, e.g.
in image.h:
#define DEFINE_IMAGE_LE64(sym, data) \
sym##_lo32 = DATA_LE32(data & 0xffffffff); \
sym##_hi32 = DATA_LE32(data >> 32);
#define HEAD_SYMBOLS \
DEFINE_IMAGE_LE64(_kernel_size_le, _end - _text); \
DEFINE_IMAGE_LE64(_kernel_offset_le, TEXT_OFFSET); \
DEFINE_IMAGE_LE64(_kernel_flags_le, __HEAD_FLAGS);
I will steal this
and in head.S:
#define IMAGE_LE64(sym) .long sym##_lo32, sym##_hi32
...
IMAGE_LE64(_kernel_size_le) // Image load offset from start of RAM, little-endian
IMAGE_LE64(_kernel_offset_le) // Effective size of kernel image, little-endian
IMAGE_LE64(_kernel_flags_le) // Informative flags, little-endian
...
... and implement this with an asm macro.
Thanks,
Ard.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-14 17:10:25
On Mon, Jan 11, 2016 at 02:19:05PM +0100, Ard Biesheuvel wrote:
Before implementing KASLR for arm64 by building a self-relocating PIE
executable, we have to ensure that values we use before the relocation
routine is executed are not subject to dynamic relocation themselves.
This applies not only to virtual addresses, but also to values that are
supplied by the linker at build time and relocated using R_AARCH64_ABS64
relocations.
So instead, use assemble time constants, or force the use of static
relocations by folding the constants into the instructions.
Signed-off-by: Ard Biesheuvel <redacted>
I think we lose a bit of legibility due to the hoops we jump through for
the new literals. However, it is correct, and I've not managed to come
up with anything nicer.
FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
This rounding looks suspect to me, given it's applied to the shadow
addresses rather than the kimage addresses. That's roughly equivalent to
kasan_mem_to_shadow(round_up(_end, 8 * SWAPPER_BLOCK_SIZE).
I don't think we need any rounding for the kimage addresses. The image
end is page-granular (and the fine-grained mapping will reflect that).
Any accesses between _end and roud_up(_end, SWAPPER_BLOCK_SIZE) would be
bugs (and would most likely fault) regardless of KASAN.
Or am I just being thick here?
Well, the problem here is that vmemmap_populate() is used as a
surrogate vmalloc() since that is not available yet, and
vmemmap_populate() allocates in SWAPPER_BLOCK_SIZE granularity.
From a look at the git history, and a chat with Catalin, it sounds like
the SWAPPER_BLOCK_SIZE granularity is a historical artifact. It happened
to be easier to implement it that way at some point in the past, but
there's no reason the 4K/16K/64K cases can't all be handled by the same
code that would go down to PAGE_SIZE granularity, using sections if
possible.
I'll drop that on the TODO list.
quoted
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
As far as I can see, it implicitly rounds the base down and end up to
SWAPPER_BLOCK_SIZE granularity.
I can see that it might map too much memory, but I can't see why that
should trigger KASAN failures. Regardless of what was mapped KASAN
should stick to the region it cares about, and everything else should
stay out of that.
When do you see the failures, and are they in any way consistent?
Do you have an example to hand?
I'll also take a peek.
I haven't managed to trigger KASAN failures with the rounding removed.
I'm using 4K pages, and running under KVM tool (no EFI, so the memory
map is a contiguous block).
What does your memory map look like?
Thanks,
Mark.
This rounding looks suspect to me, given it's applied to the shadow
addresses rather than the kimage addresses. That's roughly equivalent to
kasan_mem_to_shadow(round_up(_end, 8 * SWAPPER_BLOCK_SIZE).
I don't think we need any rounding for the kimage addresses. The image
end is page-granular (and the fine-grained mapping will reflect that).
Any accesses between _end and roud_up(_end, SWAPPER_BLOCK_SIZE) would be
bugs (and would most likely fault) regardless of KASAN.
Or am I just being thick here?
Well, the problem here is that vmemmap_populate() is used as a
surrogate vmalloc() since that is not available yet, and
vmemmap_populate() allocates in SWAPPER_BLOCK_SIZE granularity.
From a look at the git history, and a chat with Catalin, it sounds like
the SWAPPER_BLOCK_SIZE granularity is a historical artifact. It happened
to be easier to implement it that way at some point in the past, but
there's no reason the 4K/16K/64K cases can't all be handled by the same
code that would go down to PAGE_SIZE granularity, using sections if
possible.
I'll drop that on the TODO list.
OK
quoted
quoted
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
As far as I can see, it implicitly rounds the base down and end up to
SWAPPER_BLOCK_SIZE granularity.
I can see that it might map too much memory, but I can't see why that
should trigger KASAN failures. Regardless of what was mapped KASAN
should stick to the region it cares about, and everything else should
stay out of that.
When do you see the failures, and are they in any way consistent?
Do you have an example to hand?
For some reason, this issue has evaporated, i.e., I can no longer
reproduce it on my WIP v4 branch.
So I will remove the rounding.
Thanks,
Ard.
quoted
I'll also take a peek.
I haven't managed to trigger KASAN failures with the rounding removed.
I'm using 4K pages, and running under KVM tool (no EFI, so the memory
map is a contiguous block).
What does your memory map look like?
Thanks,
Mark.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-15 11:23:32
On Fri, Jan 15, 2016 at 10:54:26AM +0100, Ard Biesheuvel wrote:
On 14 January 2016 at 19:57, Mark Rutland [off-list ref] wrote:
quoted
On Wed, Jan 13, 2016 at 01:51:10PM +0000, Mark Rutland wrote:
quoted
On Wed, Jan 13, 2016 at 09:39:41AM +0100, Ard Biesheuvel wrote:
quoted
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
As far as I can see, it implicitly rounds the base down and end up to
SWAPPER_BLOCK_SIZE granularity.
I can see that it might map too much memory, but I can't see why that
should trigger KASAN failures. Regardless of what was mapped KASAN
should stick to the region it cares about, and everything else should
stay out of that.
When do you see the failures, and are they in any way consistent?
Do you have an example to hand?
For some reason, this issue has evaporated, i.e., I can no longer
reproduce it on my WIP v4 branch.
So I will remove the rounding.
Ok.
I'll let you know if I stumble across anything that looks like a
potential cause of the KASAN failures, and I'll try to give v4 a go at
some point soon.
Thanks,
Mark.
From: Matt Fleming <hidden> Date: 2016-01-21 15:42:41
On Mon, 11 Jan, at 02:19:12PM, Ard Biesheuvel wrote:
This exposes the firmware's implementation of EFI_RNG_PROTOCOL via a new
function efi_get_random_bytes().
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/Makefile | 2 +-
drivers/firmware/efi/libstub/efistub.h | 3 ++
drivers/firmware/efi/libstub/random.c | 35 ++++++++++++++++++++
include/linux/efi.h | 5 ++-
4 files changed, 43 insertions(+), 2 deletions(-)
[...]
quoted hunk
@@ -0,0 +1,35 @@+/*+ * Copyright (C) 2016 Linaro Ltd; <ard.biesheuvel@linaro.org>+ *+ * This program is free software; you can redistribute it and/or modify+ * it under the terms of the GNU General Public License version 2 as+ * published by the Free Software Foundation.+ *+ */++#include <linux/efi.h>+#include <asm/efi.h>++#include "efistub.h"++struct efi_rng_protocol_t {+ efi_status_t (*get_info)(struct efi_rng_protocol_t *,+ unsigned long *, efi_guid_t *);+ efi_status_t (*get_rng)(struct efi_rng_protocol_t *,+ efi_guid_t *, unsigned long, u8 *out);+};
This is not the usual naming convention for EFI structs, it should
either be 'struct efi_rng_protocol' or 'efi_rng_protocol_t'.
But apart from that, this patch looks fine.
Reviewed-by: Matt Fleming <redacted>
From: Matt Fleming <hidden> Date: 2016-01-21 16:10:14
On Mon, 11 Jan, at 02:19:13PM, Ard Biesheuvel wrote:
This implements efi_random_alloc(), which allocates a chunk of memory of
a certain size at a certain alignment, and uses the random_seed argument
it receives to randomize the offset of the allocation.
s/offset/address/ ?
I see what you're getting at with the word "offset" but ultimately,
this is a memory allocation function, and it returns an address.
"offset" implies to me that the implementation allocates a larger
memory chunk than is required and returns an address that is >= the
start of the bigger-than-required-allocation.
quoted hunk
This is implemented by iterating over the UEFI memory map, counting the
number of suitable slots (aligned offsets) within each region, and picking
a random number between 0 and 'number of slots - 1' to select the slot,
This should guarantee that each possible offset is chosen equally likely.
Suggested-by: Kees Cook <redacted>
Cc: Matt Fleming <redacted>
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/efistub.h | 4 +
drivers/firmware/efi/libstub/random.c | 85 ++++++++++++++++++++
2 files changed, 89 insertions(+)
This could do with a comment. When would EFI_CONVENTIONAL_MEMORY not
have this attribute capability in the memory map?
+
+ start = round_up(md->phys_addr, 1 << align_bits);
+ end = round_down(md->phys_addr + md->num_pages * EFI_PAGE_SIZE - size,
+ 1 << align_bits);
+
+ if (start >= end)
+ return 0;
+
+ return (end - start) >> align_bits;
+}
+
+/*
+ * The UEFI memory descriptors have a virtual address field that is only used
+ * when installing the virtual mapping using SetVirtualAddressMap(). Since it
+ * is unused here, we can reuse it to keep track of each descriptor's weight.
+ */
+#define MD_WEIGHT(md) ((md)->virt_addr)
+
+efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,
+ unsigned long size, unsigned long align_bits,
+ unsigned long *addr, unsigned long random_seed)
+{
+ unsigned long map_size, desc_size, max_weight = 0, target;
+ efi_memory_desc_t *memory_map;
+ efi_status_t status = EFI_NOT_FOUND;
+ int l;
Could you pick a more descriptive variable name?
+
+ status = efi_get_memory_map(sys_table_arg, &memory_map, &map_size,
+ &desc_size, NULL, NULL);
+ if (status != EFI_SUCCESS)
+ return status;
+
+ /* assign each entry in the memory map a weight */
+ for (l = 0; l < map_size; l += desc_size) {
+ efi_memory_desc_t *md = (void *)memory_map + l;
+ unsigned long weight;
+
+ weight = get_entry_weight(md, size, align_bits);
+ MD_WEIGHT(md) = weight;
+ max_weight += weight;
+ }
+
+ /* find a random number between 0 and max_weight */
+ target = (max_weight * (u16)random_seed) >> 16;
+
+ /* find the entry whose accumulated weight covers the target */
+ for (l = 0; l < map_size; l += desc_size) {
+ efi_memory_desc_t *md = (void *)memory_map + l;
+
+ if (target < MD_WEIGHT(md)) {
+ unsigned long pages;
+
+ *addr = round_up(md->phys_addr, 1 << align_bits) +
+ (target << align_bits);
+ pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+
+ status = efi_call_early(allocate_pages,
+ EFI_ALLOCATE_ADDRESS,
+ EFI_LOADER_DATA,
+ pages,
+ (efi_physical_addr_t *)addr);
You're mixing data types here. efi_physical_addr_t is always 64-bits,
but 'addr' is unsigned long, which is 32-bits on 32-bit platforms.
This cast isn't safe.
On 21 January 2016 at 16:42, Matt Fleming [off-list ref] wrote:
On Mon, 11 Jan, at 02:19:12PM, Ard Biesheuvel wrote:
quoted
This exposes the firmware's implementation of EFI_RNG_PROTOCOL via a new
function efi_get_random_bytes().
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/Makefile | 2 +-
drivers/firmware/efi/libstub/efistub.h | 3 ++
drivers/firmware/efi/libstub/random.c | 35 ++++++++++++++++++++
include/linux/efi.h | 5 ++-
4 files changed, 43 insertions(+), 2 deletions(-)
[...]
quoted
@@ -0,0 +1,35 @@+/*+ * Copyright (C) 2016 Linaro Ltd; <ard.biesheuvel@linaro.org>+ *+ * This program is free software; you can redistribute it and/or modify+ * it under the terms of the GNU General Public License version 2 as+ * published by the Free Software Foundation.+ *+ */++#include <linux/efi.h>+#include <asm/efi.h>++#include "efistub.h"++struct efi_rng_protocol_t {+ efi_status_t (*get_info)(struct efi_rng_protocol_t *,+ unsigned long *, efi_guid_t *);+ efi_status_t (*get_rng)(struct efi_rng_protocol_t *,+ efi_guid_t *, unsigned long, u8 *out);+};
This is not the usual naming convention for EFI structs, it should
either be 'struct efi_rng_protocol' or 'efi_rng_protocol_t'.
OK, I will change that.
But apart from that, this patch looks fine.
Reviewed-by: Matt Fleming <redacted>
On 21 January 2016 at 17:10, Matt Fleming [off-list ref] wrote:
On Mon, 11 Jan, at 02:19:13PM, Ard Biesheuvel wrote:
quoted
This implements efi_random_alloc(), which allocates a chunk of memory of
a certain size at a certain alignment, and uses the random_seed argument
it receives to randomize the offset of the allocation.
s/offset/address/ ?
I see what you're getting at with the word "offset" but ultimately,
this is a memory allocation function, and it returns an address.
"offset" implies to me that the implementation allocates a larger
memory chunk than is required and returns an address that is >= the
start of the bigger-than-required-allocation.
Well, offset is horribly overloaded in our world, so let's stick with 'address'
quoted
This is implemented by iterating over the UEFI memory map, counting the
number of suitable slots (aligned offsets) within each region, and picking
a random number between 0 and 'number of slots - 1' to select the slot,
This should guarantee that each possible offset is chosen equally likely.
Suggested-by: Kees Cook <redacted>
Cc: Matt Fleming <redacted>
Signed-off-by: Ard Biesheuvel <redacted>
---
drivers/firmware/efi/libstub/efistub.h | 4 +
drivers/firmware/efi/libstub/random.c | 85 ++++++++++++++++++++
2 files changed, 89 insertions(+)
This could do with a comment. When would EFI_CONVENTIONAL_MEMORY not
have this attribute capability in the memory map?
Actually, I think I should drop it instead. The other alloc functions
only check for EFI_CONVENTIONAL_MEMORY, and this is intended to be
generic code. Also, I have never seen a system with
EFI_CONVENTIONAL_MEMORY with the WB bit cleared.
quoted
+
+ start = round_up(md->phys_addr, 1 << align_bits);
+ end = round_down(md->phys_addr + md->num_pages * EFI_PAGE_SIZE - size,
+ 1 << align_bits);
+
+ if (start >= end)
+ return 0;
+
+ return (end - start) >> align_bits;
+}
+
+/*
+ * The UEFI memory descriptors have a virtual address field that is only used
+ * when installing the virtual mapping using SetVirtualAddressMap(). Since it
+ * is unused here, we can reuse it to keep track of each descriptor's weight.
+ */
+#define MD_WEIGHT(md) ((md)->virt_addr)
+
+efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,
+ unsigned long size, unsigned long align_bits,
+ unsigned long *addr, unsigned long random_seed)
+{
+ unsigned long map_size, desc_size, max_weight = 0, target;
+ efi_memory_desc_t *memory_map;
+ efi_status_t status = EFI_NOT_FOUND;
+ int l;
Could you pick a more descriptive variable name?
Sure :-)
quoted
+
+ status = efi_get_memory_map(sys_table_arg, &memory_map, &map_size,
+ &desc_size, NULL, NULL);
+ if (status != EFI_SUCCESS)
+ return status;
+
+ /* assign each entry in the memory map a weight */
+ for (l = 0; l < map_size; l += desc_size) {
+ efi_memory_desc_t *md = (void *)memory_map + l;
+ unsigned long weight;
+
+ weight = get_entry_weight(md, size, align_bits);
+ MD_WEIGHT(md) = weight;
+ max_weight += weight;
+ }
+
+ /* find a random number between 0 and max_weight */
+ target = (max_weight * (u16)random_seed) >> 16;
+
+ /* find the entry whose accumulated weight covers the target */
+ for (l = 0; l < map_size; l += desc_size) {
+ efi_memory_desc_t *md = (void *)memory_map + l;
+
+ if (target < MD_WEIGHT(md)) {
+ unsigned long pages;
+
+ *addr = round_up(md->phys_addr, 1 << align_bits) +
+ (target << align_bits);
+ pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+
+ status = efi_call_early(allocate_pages,
+ EFI_ALLOCATE_ADDRESS,
+ EFI_LOADER_DATA,
+ pages,
+ (efi_physical_addr_t *)addr);
You're mixing data types here. efi_physical_addr_t is always 64-bits,
but 'addr' is unsigned long, which is 32-bits on 32-bit platforms.
This cast isn't safe.
OK, I will fix that.
quoted
+ break;
+ }
+ target -= MD_WEIGHT(md);
I think this needs a comment.
Sure. Note that in my local version, I already replaced max_weight
with total_weight since it wasn't entirely accurate. So I'll try to
pick a better name for target as well.
Thanks,
Ard.
From: Matt Fleming <hidden> Date: 2016-01-21 16:21:05
On Mon, 11 Jan, at 02:19:14PM, Ard Biesheuvel wrote:
Before we can move the command line processing before the allocation
of the kernel, which is required for detecting the 'nokaslr' option
which controls that allocation, move the converted command line higher
up in memory, to prevent it from interfering with the kernel itself.
Since x86 needs the address to fit in 32 bits, use UINT_MAX as the upper
bound there. Otherwise, use ULONG_MAX (i.e., no limit)
Cc: Matt Fleming <redacted>
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/x86/include/asm/efi.h | 2 ++
drivers/firmware/efi/libstub/efi-stub-helper.c | 14 +++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
From: Matt Fleming <hidden> Date: 2016-01-21 16:31:56
On Mon, 11 Jan, at 02:19:15PM, Ard Biesheuvel wrote:
Since arm64 does not use a decompressor that supplies an execution
environment where it is feasible to some extent to provide a source of
randomness, the arm64 KASLR kernel depends on the bootloader to supply
some random bits in register x1 upon kernel entry.
On UEFI systems, we can use the EFI_RNG_PROTOCOL, if supplied, to obtain
some random bits. At the same time, use it to randomize the offset of the
kernel Image in physical memory.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/Kconfig | 5 ++
arch/arm64/kernel/efi-entry.S | 7 +-
drivers/firmware/efi/libstub/arm-stub.c | 17 ++---
drivers/firmware/efi/libstub/arm64-stub.c | 67 +++++++++++++++-----
drivers/firmware/efi/libstub/efi-stub-helper.c | 10 +++
drivers/firmware/efi/libstub/efistub.h | 2 +
6 files changed, 82 insertions(+), 26 deletions(-)
Could we not keep the "nokaslr" parsing inside of arm-stub.c? It's not
really specific to EFI and doesn't make use of any of the code in
efi_parse_options() anyhow.
As an added bonus, __nokaslr could then become static.
From: Mark Rutland <mark.rutland@arm.com> Date: 2016-01-22 16:55:48
Hi Ard,
This looks good.
My comments below are mostly nits, and much of the rest probably betrays
my lack of familiarity with ELF.
On Mon, Jan 11, 2016 at 02:19:01PM +0100, Ard Biesheuvel wrote:
quoted hunk
This adds support for emitting PLTs at module load time for relative
branches that are out of range. This is a prerequisite for KASLR, which
may place the kernel and the modules anywhere in the vmalloc area,
making it more likely that branch target offsets exceed the maximum
range of +/- 128 MB.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/Kconfig | 9 ++
arch/arm64/Makefile | 6 +-
arch/arm64/include/asm/module.h | 11 ++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/module-plts.c | 137 ++++++++++++++++++++
arch/arm64/kernel/module.c | 12 ++
arch/arm64/kernel/module.lds | 4 +
7 files changed, 179 insertions(+), 1 deletion(-)
@@ -363,6 +363,7 @@ config ARM64_ERRATUM_843419bool"Cortex-A53: 843419: A load or store might access an incorrect address"depends onMODULESdefaulty+selectARM64_MODULE_CMODEL_LARGEhelpThisoptionbuildskernelmodulesusingthelargememorymodelinordertoavoidtheuseoftheADRPinstruction,whichcancause
We only need natural alignment for the instructions, so what's the
alignment for? I can't see that anything else cares.
It might be worth a comment regarding why why use x16 (i.e. because the
AAPCS says that as IP0 it is valid for veneers/PLTs to clobber).
It would be nice if we could un-magic this, though I see that reusing
the existing insn or reloc_insn code is painful here.
+ int i, *count;
+
+ if (in_init(mod, loc)) {
+ plt = (struct plt_entry *)mod->arch.init_plt->sh_addr;
+ count = &mod->arch.init_plt_count;
+ } else {
+ plt = (struct plt_entry *)mod->arch.core_plt->sh_addr;
+ count = &mod->arch.core_plt_count;
+ }
+
+ /* Look for an existing entry pointing to 'val' */
+ for (i = 0; i < *count; i++)
+ if (plt[i].mov0 == entry.mov0 &&
+ plt[i].mov1 == entry.mov1 &&
+ plt[i].mov2 == entry.mov2)
+ return (u64)&plt[i];
I think that at the cost of redundantly comparing the br x16, you could
simplify this by comparing the whole struct, e.g.
for (i = 0; i < *count; i++)
if (plt[i] == entry)
return (u64)&plt[i];
Which would also work if we change the veneer for some reason.
+
+ i = (*count)++;
given i == *count at the end of the loop, you could just increment
*count here.
+ plt[i] = entry;
+ return (u64)&plt[i];
+}
+
+static int duplicate_rel(Elf64_Addr base, const Elf64_Rela *rela, int num)
Perhaps: static bool is_duplicate_rel
+{
+ int i;
+
+ for (i = 0; i < num; i++) {
+ if (rela[i].r_info == rela[num].r_info &&
+ rela[i].r_addend == rela[num].r_addend)
+ return 1;
+ }
+ return 0;
+}
+
+/* Count how many PLT entries we may need */
+static unsigned int count_plts(Elf64_Addr base, const Elf64_Rela *rela, int num)
+{
+ unsigned int ret = 0;
+ int i;
+
+ /*
+ * Sure, this is order(n^2), but it's usually short, and not
+ * time critical
+ */
+ for (i = 0; i < num; i++)
+ switch (ELF64_R_TYPE(rela[i].r_info)) {
+ case R_AARCH64_JUMP26:
+ case R_AARCH64_CALL26:
+ if (!duplicate_rel(base, rela, i))
+ ret++;
+ break;
+ }
While braces aren't strictly required on the for loop, i think it would
look better with them given the contained logic is non-trivial.
+ return ret;
+}
+
+int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
+ char *secstrings, struct module *mod)
+{
+ unsigned long core_plts = 0, init_plts = 0;
+ Elf64_Shdr *s, *sechdrs_end = sechdrs + ehdr->e_shnum;
+
+ /*
+ * To store the PLTs, we expand the .text section for core module code
+ * and the .init.text section for initialization code.
+ */
That comment is a bit misleading, given we don't touch .text and
.init.text, but rather .core.plt and .init.plt, relying on
layout_sections to group those with .text and .init.text.
+ for (s = sechdrs; s < sechdrs_end; ++s)
+ if (strcmp(".core.plt", secstrings + s->sh_name) == 0)
+ mod->arch.core_plt = s;
+ else if (strcmp(".init.plt", secstrings + s->sh_name) == 0)
+ mod->arch.init_plt = s;
This would be nicer with braces.
+
+ if (!mod->arch.core_plt || !mod->arch.init_plt) {
+ pr_err("%s: sections missing\n", mod->name);
+ return -ENOEXEC;
+ }
+
+ for (s = sechdrs + 1; s < sechdrs_end; ++s) {
Could we have a comment as to why we skip the first Shdr? I recall it's
in some way special, but I can't recall why/how.
On 22 January 2016 at 17:55, Mark Rutland [off-list ref] wrote:
Hi Ard,
This looks good.
Thanks for taking a look. I must say that this looks slightly
different now in my upcoming v4: I got rid of the O(n^2) loops in
favor of sorting the RELA section (iff it relocates an executable
section)
My comments below are mostly nits, and much of the rest probably betrays
my lack of familiarity with ELF.
On Mon, Jan 11, 2016 at 02:19:01PM +0100, Ard Biesheuvel wrote:
quoted
This adds support for emitting PLTs at module load time for relative
branches that are out of range. This is a prerequisite for KASLR, which
may place the kernel and the modules anywhere in the vmalloc area,
making it more likely that branch target offsets exceed the maximum
range of +/- 128 MB.
Signed-off-by: Ard Biesheuvel <redacted>
---
arch/arm64/Kconfig | 9 ++
arch/arm64/Makefile | 6 +-
arch/arm64/include/asm/module.h | 11 ++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/module-plts.c | 137 ++++++++++++++++++++
arch/arm64/kernel/module.c | 12 ++
arch/arm64/kernel/module.lds | 4 +
7 files changed, 179 insertions(+), 1 deletion(-)
@@ -363,6 +363,7 @@ config ARM64_ERRATUM_843419bool"Cortex-A53: 843419: A load or store might access an incorrect address"depends onMODULESdefaulty+selectARM64_MODULE_CMODEL_LARGEhelpThisoptionbuildskernelmodulesusingthelargememorymodelinordertoavoidtheuseoftheADRPinstruction,whichcancause
We only need natural alignment for the instructions, so what's the
alignment for? I can't see that anything else cares.
This allows the compiler to emit a single load for the first two
fields when performing the comparison in the loop below. All of this
is somewhat moot now, since the sorting of the section causes the
duplicates to be adjacent, and I only have to compare against the last
veneer that was emitted.
It might be worth a comment regarding why why use x16 (i.e. because the
AAPCS says that as IP0 it is valid for veneers/PLTs to clobber).
It would be nice if we could un-magic this, though I see that reusing
the existing insn or reloc_insn code is painful here.
Well, I could #define PLT0 PLT1 PLT2 etc, and document them a bit
better, but having all the instruction machinery for emitting the
exact same instructions each time seems a bit overkill imo.
quoted
+ int i, *count;
+
+ if (in_init(mod, loc)) {
+ plt = (struct plt_entry *)mod->arch.init_plt->sh_addr;
+ count = &mod->arch.init_plt_count;
+ } else {
+ plt = (struct plt_entry *)mod->arch.core_plt->sh_addr;
+ count = &mod->arch.core_plt_count;
+ }
+
+ /* Look for an existing entry pointing to 'val' */
+ for (i = 0; i < *count; i++)
+ if (plt[i].mov0 == entry.mov0 &&
+ plt[i].mov1 == entry.mov1 &&
+ plt[i].mov2 == entry.mov2)
+ return (u64)&plt[i];
I think that at the cost of redundantly comparing the br x16, you could
simplify this by comparing the whole struct, e.g.
for (i = 0; i < *count; i++)
if (plt[i] == entry)
You can use struct types in assignments, but not in comparisons,
strangely enough
return (u64)&plt[i];
Which would also work if we change the veneer for some reason.
quoted
+
+ i = (*count)++;
given i == *count at the end of the loop, you could just increment
*count here.
quoted
+ plt[i] = entry;
+ return (u64)&plt[i];
+}
+
+static int duplicate_rel(Elf64_Addr base, const Elf64_Rela *rela, int num)
Perhaps: static bool is_duplicate_rel
quoted
+{
+ int i;
+
+ for (i = 0; i < num; i++) {
+ if (rela[i].r_info == rela[num].r_info &&
+ rela[i].r_addend == rela[num].r_addend)
+ return 1;
+ }
+ return 0;
+}
+
+/* Count how many PLT entries we may need */
+static unsigned int count_plts(Elf64_Addr base, const Elf64_Rela *rela, int num)
+{
+ unsigned int ret = 0;
+ int i;
+
+ /*
+ * Sure, this is order(n^2), but it's usually short, and not
+ * time critical
+ */
+ for (i = 0; i < num; i++)
+ switch (ELF64_R_TYPE(rela[i].r_info)) {
+ case R_AARCH64_JUMP26:
+ case R_AARCH64_CALL26:
+ if (!duplicate_rel(base, rela, i))
+ ret++;
+ break;
+ }
While braces aren't strictly required on the for loop, i think it would
look better with them given the contained logic is non-trivial.
Indeed. I will add them
quoted
+ return ret;
+}
+
+int module_frob_arch_sections(Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
+ char *secstrings, struct module *mod)
+{
+ unsigned long core_plts = 0, init_plts = 0;
+ Elf64_Shdr *s, *sechdrs_end = sechdrs + ehdr->e_shnum;
+
+ /*
+ * To store the PLTs, we expand the .text section for core module code
+ * and the .init.text section for initialization code.
+ */
That comment is a bit misleading, given we don't touch .text and
.init.text, but rather .core.plt and .init.plt, relying on
layout_sections to group those with .text and .init.text.
ok
quoted
+ for (s = sechdrs; s < sechdrs_end; ++s)
+ if (strcmp(".core.plt", secstrings + s->sh_name) == 0)
+ mod->arch.core_plt = s;
+ else if (strcmp(".init.plt", secstrings + s->sh_name) == 0)
+ mod->arch.init_plt = s;
This would be nicer with braces.
ok
quoted
+
+ if (!mod->arch.core_plt || !mod->arch.init_plt) {
+ pr_err("%s: sections missing\n", mod->name);
+ return -ENOEXEC;
+ }
+
+ for (s = sechdrs + 1; s < sechdrs_end; ++s) {
Could we have a comment as to why we skip the first Shdr? I recall it's
in some way special, but I can't recall why/how.
I don't remember exactly, and some of this code originated on ia64 IIRC.
Probably better to simply start from [0]
Nope.
arch/arm64/Kconfig:86: select MODULES_USE_ELF_RELA
As I said, this code will look different in the next version, but I
will make sure to take your review points.
Thanks,
Ard.
It would be nice if we could un-magic this, though I see that reusing
the existing insn or reloc_insn code is painful here.
Well, I could #define PLT0 PLT1 PLT2 etc, and document them a bit
better, but having all the instruction machinery for emitting the
exact same instructions each time seems a bit overkill imo.
Well, almost the same (the target address does change after all).
I agree that this looks more complicated using the insn machinery, based
on local experimentation. Oh well...
quoted
quoted
+ int i, *count;
+
+ if (in_init(mod, loc)) {
+ plt = (struct plt_entry *)mod->arch.init_plt->sh_addr;
+ count = &mod->arch.init_plt_count;
+ } else {
+ plt = (struct plt_entry *)mod->arch.core_plt->sh_addr;
+ count = &mod->arch.core_plt_count;
+ }
+
+ /* Look for an existing entry pointing to 'val' */
+ for (i = 0; i < *count; i++)
+ if (plt[i].mov0 == entry.mov0 &&
+ plt[i].mov1 == entry.mov1 &&
+ plt[i].mov2 == entry.mov2)
+ return (u64)&plt[i];
I think that at the cost of redundantly comparing the br x16, you could
simplify this by comparing the whole struct, e.g.
for (i = 0; i < *count; i++)
if (plt[i] == entry)
You can use struct types in assignments, but not in comparisons,
strangely enough
Ah, sorry for the noise.
quoted
quoted
+ for (s = sechdrs + 1; s < sechdrs_end; ++s) {
Could we have a comment as to why we skip the first Shdr? I recall it's
in some way special, but I can't recall why/how.
I don't remember exactly, and some of this code originated on ia64 IIRC.
Probably better to simply start from [0]
On 15 January 2016 at 12:23, Mark Rutland [off-list ref] wrote:
On Fri, Jan 15, 2016 at 10:54:26AM +0100, Ard Biesheuvel wrote:
quoted
On 14 January 2016 at 19:57, Mark Rutland [off-list ref] wrote:
quoted
On Wed, Jan 13, 2016 at 01:51:10PM +0000, Mark Rutland wrote:
quoted
On Wed, Jan 13, 2016 at 09:39:41AM +0100, Ard Biesheuvel wrote:
quoted
If I remove the rounding, I get false positive kasan errors which I
have not quite diagnosed yet, but are probably due to the fact that
the rounding performed by vmemmap_populate() goes in the wrong
direction.
As far as I can see, it implicitly rounds the base down and end up to
SWAPPER_BLOCK_SIZE granularity.
I can see that it might map too much memory, but I can't see why that
should trigger KASAN failures. Regardless of what was mapped KASAN
should stick to the region it cares about, and everything else should
stay out of that.
When do you see the failures, and are they in any way consistent?
Do you have an example to hand?
For some reason, this issue has evaporated, i.e., I can no longer
reproduce it on my WIP v4 branch.
So I will remove the rounding.
Ok.
I'll let you know if I stumble across anything that looks like a
potential cause of the KASAN failures, and I'll try to give v4 a go at
some point soon.
OK, I managed to track this down (I think). The issue here is that,
while vmemmap_populate() does the right thing wrt the start and end
boundaries, populate_zero_shadow() will map the adjoining regions down
to page granularity, replacing vmemmap_populate()'s PMD block mappings
with PMD table mappings. So I need to put back the rounding (I removed
it in v4)
Thanks,
Ard.