Skip to content

Commit 0b99b55

Browse files
joshuaseatonCQ Bot
authored andcommitted
[kernel][vm][arm64] Disallow executable + uncached mappings
This doesn't seem sensible and the related executable-gated code assumes cached (which was tickled in a recent bug where MMIO was wrongly marked as executable). Change-Id: Ia90361c14b448e4adca8cb7ab3e50c906600e182 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1279924 Fuchsia-Auto-Submit: Joshua Seaton <[email protected]> Commit-Queue: Auto-Submit <auto-submit-builder@fuchsia-internal-service-accts.iam.gserviceaccount.com> Reviewed-by: Adrian Danis <[email protected]>
1 parent 7843cc7 commit 0b99b55

File tree

3 files changed

+14
-1
lines changed

3 files changed

+14
-1
lines changed

zircon/kernel/arch/arm64/mmu.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1455,6 +1455,9 @@ zx_status_t ArmArchVmAspace::MapContiguous(vaddr_t vaddr, paddr_t paddr, size_t
14551455
if (!(mmu_flags & ARCH_MMU_FLAG_PERM_READ)) {
14561456
return ZX_ERR_INVALID_ARGS;
14571457
}
1458+
if ((mmu_flags & ARCH_MMU_FLAG_PERM_EXECUTE) && arch_mmu_flags_uncached(mmu_flags)) {
1459+
return ZX_ERR_INVALID_ARGS;
1460+
}
14581461

14591462
// paddr and vaddr must be aligned.
14601463
DEBUG_ASSERT(IS_PAGE_ALIGNED(vaddr));
@@ -1543,6 +1546,9 @@ zx_status_t ArmArchVmAspace::Map(vaddr_t vaddr, paddr_t* phys, size_t count, uin
15431546
if (!(mmu_flags & ARCH_MMU_FLAG_PERM_READ)) {
15441547
return ZX_ERR_INVALID_ARGS;
15451548
}
1549+
if ((mmu_flags & ARCH_MMU_FLAG_PERM_EXECUTE) && arch_mmu_flags_uncached(mmu_flags)) {
1550+
return ZX_ERR_INVALID_ARGS;
1551+
}
15461552

15471553
// vaddr must be aligned.
15481554
DEBUG_ASSERT(IS_PAGE_ALIGNED(vaddr));
@@ -1664,6 +1670,9 @@ zx_status_t ArmArchVmAspace::Protect(vaddr_t vaddr, size_t count, uint mmu_flags
16641670
if (!(mmu_flags & ARCH_MMU_FLAG_PERM_READ)) {
16651671
return ZX_ERR_INVALID_ARGS;
16661672
}
1673+
if ((mmu_flags & ARCH_MMU_FLAG_PERM_EXECUTE) && arch_mmu_flags_uncached(mmu_flags)) {
1674+
return ZX_ERR_INVALID_ARGS;
1675+
}
16671676

16681677
// The stage 2 data and instructions aborts do not contain sufficient information for us to
16691678
// resolve permission faults, and these kinds of faults generate a hard error. As such we cannot

zircon/kernel/vm/include/vm/arch_vm_aspace.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ const uint ARCH_MMU_FLAG_INVALID = (1u << 7); // Indicates that flags are not s
3434
const uint ARCH_ASPACE_FLAG_KERNEL = (1u << 0);
3535
const uint ARCH_ASPACE_FLAG_GUEST = (1u << 1);
3636

37+
constexpr bool arch_mmu_flags_uncached(uint mmu_flags) {
38+
return (mmu_flags & (ARCH_MMU_FLAG_UNCACHED | ARCH_MMU_FLAG_UNCACHED_DEVICE)) != 0;
39+
}
40+
3741
// per arch base class api to encapsulate the mmu routines on an aspace
3842
//
3943
// Beyond construction/destruction lifetimes users of this object must ensure that none of the

zircon/kernel/vm/vm.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ constexpr uint32_t ToVmarFlags(PhysMapping::Permissions perms) {
6565
}
6666

6767
constexpr uint ToArchMmuFlags(PhysMapping::Permissions perms) {
68-
uint flags = 0;
68+
uint flags = ARCH_MMU_FLAG_CACHED;
6969
if (perms.readable()) {
7070
flags |= ARCH_MMU_FLAG_PERM_READ;
7171
}

0 commit comments

Comments
 (0)