From 9d40492efa467095340cf3dca5860880aa441472 Mon Sep 17 00:00:00 2001 From: Andrew Turner Date: Mon, 8 Apr 2024 10:44:33 +0000 Subject: [PATCH] arm64: Check DMAP address is valid in PHYS_IN_DMAP When checking if a physical address is in the DMAP region we assume all physical addresses between DMAP_MIN_PHYSADDR and DMAP_MAX_PHYSADDR are able to be accesses through the DMAP. It may be the case that there is device memory in this range that shouldn't be accessed through the DMAP mappings. Add a check to PHYS_IN_DMAP that the translated virtual address is a valid kernel address. To support code that already checks the address is valid add PHYS_IN_DMAP_RANGE. PR: 278233 Reviewed by: alc, markj Sponsored by: Arm Ltd Differential Revision: https://reviews.freebsd.org/D44677 --- sys/arm64/arm64/efirt_machdep.c | 9 ++------- sys/arm64/arm64/machdep.c | 2 +- sys/arm64/arm64/minidump_machdep.c | 7 ++++--- sys/arm64/include/vmparam.h | 18 +++++++++++++++--- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/sys/arm64/arm64/efirt_machdep.c b/sys/arm64/arm64/efirt_machdep.c index 52906779dfb..0c46d2e6dcc 100644 --- a/sys/arm64/arm64/efirt_machdep.c +++ b/sys/arm64/arm64/efirt_machdep.c @@ -144,13 +144,8 @@ efi_1t1_l3(vm_offset_t va) vm_offset_t efi_phys_to_kva(vm_paddr_t paddr) { - vm_offset_t vaddr; - - if (PHYS_IN_DMAP(paddr)) { - vaddr = PHYS_TO_DMAP(paddr); - if (pmap_klookup(vaddr, NULL)) - return (vaddr); - } + if (PHYS_IN_DMAP(paddr)) + return (PHYS_TO_DMAP(paddr)); /* TODO: Map memory not in the DMAP */ diff --git a/sys/arm64/arm64/machdep.c b/sys/arm64/arm64/machdep.c index d566a8e2298..517f4e7c2e2 100644 --- a/sys/arm64/arm64/machdep.c +++ b/sys/arm64/arm64/machdep.c @@ -444,7 +444,7 @@ arm64_get_writable_addr(void *addr, void **out) /* * If it is within the DMAP region and is writable use that. */ - if (PHYS_IN_DMAP(pa)) { + if (PHYS_IN_DMAP_RANGE(pa)) { addr = (void *)PHYS_TO_DMAP(pa); if (PAR_SUCCESS(arm64_address_translate_s1e1w( (vm_offset_t)addr))) { diff --git a/sys/arm64/arm64/minidump_machdep.c b/sys/arm64/arm64/minidump_machdep.c index 87bf41b27fd..8ee626953ae 100644 --- a/sys/arm64/arm64/minidump_machdep.c +++ b/sys/arm64/arm64/minidump_machdep.c @@ -202,7 +202,8 @@ cpu_minidumpsys(struct dumperinfo *di, const struct minidumpstate *state) if ((l3e & ATTR_DESCR_MASK) != L3_PAGE) continue; pa = PTE_TO_PHYS(l3e); - if (PHYS_IN_DMAP(pa) && vm_phys_is_dumpable(pa)) + if (PHYS_IN_DMAP_RANGE(pa) && + vm_phys_is_dumpable(pa)) vm_page_dump_add(state->dump_bitset, pa); } @@ -216,7 +217,7 @@ cpu_minidumpsys(struct dumperinfo *di, const struct minidumpstate *state) dumpsize += round_page(sizeof(dump_avail)); dumpsize += round_page(BITSET_SIZE(vm_page_dump_pages)); VM_PAGE_DUMP_FOREACH(state->dump_bitset, pa) { - if (PHYS_IN_DMAP(pa) && vm_phys_is_dumpable(pa)) + if (PHYS_IN_DMAP_RANGE(pa) && vm_phys_is_dumpable(pa)) dumpsize += PAGE_SIZE; else vm_page_dump_drop(state->dump_bitset, pa); @@ -347,7 +348,7 @@ cpu_minidumpsys(struct dumperinfo *di, const struct minidumpstate *state) * We always write a page, even if it is zero. If pa * is malformed, write the zeroed tmpbuffer. */ - if (PHYS_IN_DMAP(pa) && vm_phys_is_dumpable(pa)) + if (PHYS_IN_DMAP_RANGE(pa) && vm_phys_is_dumpable(pa)) error = blk_write(di, NULL, pa, PAGE_SIZE); else error = blk_write(di, (char *)&tmpbuffer, 0, diff --git a/sys/arm64/include/vmparam.h b/sys/arm64/include/vmparam.h index d5d4a5691f3..83c55913f56 100644 --- a/sys/arm64/include/vmparam.h +++ b/sys/arm64/include/vmparam.h @@ -223,9 +223,21 @@ #define DMAP_MIN_PHYSADDR (dmap_phys_base) #define DMAP_MAX_PHYSADDR (dmap_phys_max) -/* True if pa is in the dmap range */ -#define PHYS_IN_DMAP(pa) ((pa) >= DMAP_MIN_PHYSADDR && \ +/* + * Checks to see if a physical address is in the DMAP range. + * - PHYS_IN_DMAP_RANGE will return true that may be within the DMAP range + * but not accessible through the DMAP, e.g. device memory between two + * DMAP physical address regions. + * - PHYS_IN_DMAP will check if DMAP address is mapped before returning true. + * + * PHYS_IN_DMAP_RANGE should only be used when a check on the address is + * performed, e.g. by checking the physical address is within phys_avail, + * or checking the virtual address is mapped. + */ +#define PHYS_IN_DMAP_RANGE(pa) ((pa) >= DMAP_MIN_PHYSADDR && \ (pa) < DMAP_MAX_PHYSADDR) +#define PHYS_IN_DMAP(pa) (PHYS_IN_DMAP_RANGE(pa) && \ + pmap_klookup(PHYS_TO_DMAP(pa), NULL)) /* True if va is in the dmap range */ #define VIRT_IN_DMAP(va) ((va) >= DMAP_MIN_ADDRESS && \ (va) < (dmap_max_addr)) @@ -233,7 +245,7 @@ #define PMAP_HAS_DMAP 1 #define PHYS_TO_DMAP(pa) \ ({ \ - KASSERT(PHYS_IN_DMAP(pa), \ + KASSERT(PHYS_IN_DMAP_RANGE(pa), \ ("%s: PA out of range, PA: 0x%lx", __func__, \ (vm_paddr_t)(pa))); \ ((pa) - dmap_phys_base) + DMAP_MIN_ADDRESS; \