arm64: kernel: restrict /dev/mem read() calls to linear region
PATCH v2 1/1 arm64: kernel: restrict /dev/mem read() calls to linear region
When running lscpu on an AArch64 system that has SMBIOS version 2.0 tables, it will segfault in the following way:
Unable to handle kernel paging request at virtual address ffff8000bfff0000 pgd = ffff8000f9615000 [ffff8000bfff0000] *pgd=0000000000000000 Internal error: Oops: 96000007 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 1284 Comm: lscpu Not tainted 4.11.0-rc3+ #103 Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 task: ffff8000fa78e800 task.stack: ffff8000f9780000 PC is at __arch_copy_to_user+0x90/0x220 LR is at read_mem+0xcc/0x140
This is caused by the fact that lspci issues a read() on /dev/mem at the offset where it expects to find the SMBIOS structure array. However, this region is classified as EFI_RUNTIME_SERVICE_DATA (as per the UEFI spec), and so it is omitted from the linear mapping.
So let's restrict /dev/mem read/write access to those areas that are covered by the linear region.
---
v2: check whether the entire region is covered by the same memblock that has
the MEMBLOCK_NOMAP attribute cleared
arch/arm64/mm/mmap.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
arch/arm64/mm/mmap.c +13 -6
@@ -18,6 +18,7 @@#include <linux/elf.h> #include <linux/fs.h> +#include <linux/memblock.h> #include <linux/mm.h> #include <linux/mman.h> #include <linux/export.h>
@@ -103,12 +104,18 @@ void arch_pick_mmap_layout(struct mm_struct *mm)*/ int valid_phys_addr_range (phys_addr_t addr,size_t size) { -if (addr <PHYS_OFFSET ) -return 0 ; -if (addr + size >__pa (high_memory -1 ) +1 ) -return 0 ; - -return 1 ; +/* +* Check whether addr is covered by a memory region without the +* MEMBLOCK_NOMAP attribute, and whether that region covers the +* entire range. In theory, this could lead to false negatives +* if the range is covered by distinct but adjacent memory regions +* that only differ in other attributes. However, few of such +* attributes have been defined, and it is debatable whether it +* follows that /dev/mem read() calls should be able traverse +* such boundaries. +*/ +return memblock_is_region_memory (addr, size) && +memblock_is_map_memory (addr); }/*
Signature · 1 line
2.9.3
PATCH v2 1/1 arm64: kernel: restrict /dev/mem read() calls to linear region
Quoted 14 lines · Ard Biesheuvel
When running lscpu on an AArch64 system that has SMBIOS version 2.0 tables, it will segfault in the following way:
Unable to handle kernel paging request at virtual address ffff8000bfff0000 pgd = ffff8000f9615000 [ffff8000bfff0000] *pgd=0000000000000000 Internal error: Oops: 96000007 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 1284 Comm: lscpu Not tainted 4.11.0-rc3+ #103 Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 task: ffff8000fa78e800 task.stack: ffff8000f9780000 PC is at __arch_copy_to_user+0x90/0x220 LR is at read_mem+0xcc/0x140
Quoted 7 lines
This is caused by the fact that lspci issues a
read()on/dev/memat the offset where it expects to find the SMBIOS structure array. However, this region is classified asEFI_RUNTIME_SERVICE_DATA(as per the UEFI spec), and so it is omitted from the linear mapping.So let's restrict
/dev/memread/write access to those areas that are covered by the linear region.
So, I'm still of the opinion that /dev/mem simply should not be made available on systems where people care about accidentally hard-locking their systems from userland.
To that extent, this workaround takes the pressure off people to configure their kernels properly.
On the other hand, it probably removes 90% of the risk cases.
I guess the solution depends on whether people think the remaining 10% matter.
/
Leif
Quoted 50 lines
Reported-by Alexander Graf <[email protected]>Fixes 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")Signed-off-by Ard Biesheuvel <[email protected]>---
v2: check whether the entire region is covered by the same memblock that has
theMEMBLOCK_NOMAPattribute clearedarch/arm64/mm/mmap.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)arch/arm64/mm/mmap.c +13
-6@@ -18,6 +18,7 @@#include <linux/elf.h> #include <linux/fs.h> +#include <linux/memblock.h> #include <linux/mm.h> #include <linux/mman.h> #include <linux/export.h> @@ -103,12 +104,18 @@ void arch_pick_mmap_layout(struct mm_struct *mm)*/ int valid_phys_addr_range (phys_addr_t addr,size_t size) { -if (addr <PHYS_OFFSET ) -return 0 ; -if (addr + size >__pa (high_memory -1 ) +1 ) -return 0 ; - -return 1 ; +/* +* Check whether addr is covered by a memory region without the +* MEMBLOCK_NOMAP attribute, and whether that region covers the +* entire range. In theory, this could lead to false negatives +* if the range is covered by distinct but adjacent memory regions +* that only differ in other attributes. However, few of such +* attributes have been defined, and it is debatable whether it +* follows that /dev/mem read() calls should be able traverse +* such boundaries. +*/ +return memblock_is_region_memory (addr, size) && +memblock_is_map_memory (addr); }/* Signature · 1 line
2.9.3
PATCH v2 1/1 arm64: kernel: restrict /dev/mem read() calls to linear region
Quoted 23 lines · Leif Lindholm
Quoted 14 lines · Ard Biesheuvel
When running lscpu on an AArch64 system that has SMBIOS version 2.0 tables, it will segfault in the following way:
Unable to handle kernel paging request at virtual address ffff8000bfff0000 pgd = ffff8000f9615000 [ffff8000bfff0000] *pgd=0000000000000000 Internal error: Oops: 96000007 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 1284 Comm: lscpu Not tainted 4.11.0-rc3+ #103 Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 task: ffff8000fa78e800 task.stack: ffff8000f9780000 PC is at __arch_copy_to_user+0x90/0x220 LR is at read_mem+0xcc/0x140Quoted 7 lines
This is caused by the fact that lspci issues a
read()on/dev/memat the offset where it expects to find the SMBIOS structure array. However, this region is classified asEFI_RUNTIME_SERVICE_DATA(as per the UEFI spec), and so it is omitted from the linear mapping.So let's restrict
/dev/memread/write access to those areas that are covered by the linear region.
Quoted 9 lines
So, I'm still of the opinion that
/dev/memsimply should not be made available on systems where people care about accidentally hard-locking their systems from userland.To that extent, this workaround takes the pressure off people to configure their kernels properly.
On the other hand, it probably removes 90% of the risk cases.
Sadly, no. This change only fixes a kernel bug where we access a region using read()/write() that is known to be memory, but may have been left unmapped. It does not affect /dev/mem uses involving mmap() at all.
Quoted 3 lines
I guess the solution depends on whether people think the remaining 10% matter.
I think there are two different aspects that we need to distinguish:
- on ARM, you cannot use /dev/mem to go looking for magic numbers in memory to locate certain data structures left by the firmware,
- on ARM, you should [must] not use /dev/mem to map regions that are already mapped with different attributes.
This fix is in neither category, it just fails read()s on /dev/mem gracefully if they attempt to access NOMAP memory.
So the only moderately sane use case for /dev/mem on ARM is to do cached accesses to DRAM regions that are covered by the linear mapping. Interestingly, these are exactly the ones RESTRICT_DEVMEM usually disallows IIRC, given that you may be poking into kernel internals.
My vote would be to deprecate other /dev/mem uses on arm64, so that we can start getting a handle on things before the cat is too far out of the bag. (and you can't 'break' a x86 userspace this way on arm64 if it was simply recompiled but pokes memory for legacy BIOS strings etc)
PATCH v2 1/1 arm64: kernel: restrict /dev/mem read() calls to linear region
Quoted 7 lines · Ard Biesheuvel
Quoted 7 lines
So, I'm still of the opinion that
/dev/memsimply should not be made available on systems where people care about accidentally hard-locking their systems from userland.To that extent, this workaround takes the pressure off people to configure their kernels properly.
Quoted 6 lines
Quoted 1 line
On the other hand, it probably removes 90% of the risk cases.
Sadly, no. This change only fixes a kernel bug where we access a region using read()/write() that is known to be memory, but may have been left unmapped. It does not affect
/dev/memuses involvingmmap()at all.
Ah. Well, in that case I don't have much of an objection.
Quoted 8 lines
Quoted 2 lines
I guess the solution depends on whether people think the remaining 10% matter.
I think there are two different aspects that we need to distinguish:
- on ARM, you cannot use/dev/memto go looking for magic numbers in memory to locate certain data structures left by the firmware,
- on ARM, you should [must] not use/dev/memto map regions that are already mapped with different attributes.
And don't leave out the whopper:
- on ARM, running userland applications written to use /dev/mem to do specific things on x86 is unlikely to do what you expected it to do, and is likely to crash your system.
Being architecturally safe does not mean your system is not vulnerable.
Quoted 2 lines
This fix is in neither category, it just fails read()s on
/dev/memgracefully if they attempt to access NOMAP memory.
Understood.
Quoted 3 lines
So the only moderately sane use case for
/dev/memon ARM is to do cached accesses to DRAM regions that are covered by the linear mapping.
Aside from "writing proper kernel interfaces takes effort", is there a valid reason to do so?
Even if we did restrict it down to cached known DRAM regions, we would still need to turn it read-only. Or a badly written program (I've heard a couple exist) can start trampling whatever.
If people need a generic way to easily give access to certain defined memory regions, let's write a driver that chomps kernel command line or DT and provides simple mmap access to those specific regions.
If people want arbitrary userspace access to any region in memory at any given time, why did we waste the gates and energy of implementing an MMU and managing permissions?
Quoted 3 lines
Interestingly, these are exactly the ones
RESTRICT_DEVMEMusually disallows IIRC, given that you may be poking into kernel internals.
Yes, well, given its history as being PROMISC_DEVMEM renamed and flipped logically, I doubt that't the only inconsistency...
Quoted 4 lines
My vote would be to deprecate other
/dev/memuses on arm64, so that we can start getting a handle on things before the cat is too far out of the bag. (and you can't 'break' a x86 userspace this way on arm64 if it was simply recompiled but pokes memory for legacy BIOS strings etc)
To no one's surprise, my vote is to deprecate all uses of /dev/mem on arm64.
/
Leif
PATCH v2 1/1 arm64: kernel: restrict /dev/mem read() calls to linear region
In reply to Ard BiesheuvelHi Ard,
Quoted 25 lines · Ard Biesheuvel
When running lscpu on an AArch64 system that has SMBIOS version 2.0 tables, it will segfault in the following way:
Unable to handle kernel paging request at virtual address ffff8000bfff0000 pgd = ffff8000f9615000 [ffff8000bfff0000] *pgd=0000000000000000 Internal error: Oops: 96000007 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 1284 Comm: lscpu Not tainted 4.11.0-rc3+ #103 Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 task: ffff8000fa78e800 task.stack: ffff8000f9780000 PC is at __arch_copy_to_user+0x90/0x220 LR is at read_mem+0xcc/0x140This is caused by the fact that lspci issues a
read()on/dev/memat the offset where it expects to find the SMBIOS structure array. However, this region is classified asEFI_RUNTIME_SERVICE_DATA(as per the UEFI spec), and so it is omitted from the linear mapping.So let's restrict
/dev/memread/write access to those areas that are covered by the linear region.Reported-by Alexander Graf <[email protected]>Fixes 4dffbfc48d65 ("arm64/efi: mark UEFI reserved regions as MEMBLOCK_NOMAP")Signed-off-by Ard Biesheuvel <[email protected]>
Quoted 6 lines
---
v2: check whether the entire region is covered by the same memblock that has
theMEMBLOCK_NOMAPattribute clearedarch/arm64/mm/mmap.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
Thanks. I'll stick this into -next and see how we do.
Will