From 7be371792f03735519174b773450546e6bf5376e Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Sun, 21 May 2023 10:15:27 +1000 Subject: [PATCH] kernel/vm: unlock cache before unmapping addresses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unlock the current cache in `map_backing_store` before `unmap_address_range` is called, since `unmap_address_range` may call `delete_area` which would then also attempt to lock the same cache if that cache has already been mapped to an area in the conflicting address range. Fixes #18422. Change-Id: I6fc5301c43d11bb6df489a2e6d6bdcd6cd80d2b7 Reviewed-on: https://review.haiku-os.org/c/haiku/+/6392 Reviewed-by: Jérôme Duval --- src/system/kernel/vm/vm.cpp | 7 +++- src/tests/system/kernel/Jamfile | 1 + src/tests/system/kernel/mmap_fixed_test.cpp | 41 +++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 src/tests/system/kernel/mmap_fixed_test.cpp diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index 292b3826ac..f323583d42 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -918,7 +918,7 @@ discard_address_range(VMAddressSpace* addressSpace, addr_t address, addr_t size, If \a addressSpec is \c B_EXACT_ADDRESS and the \c CREATE_AREA_UNMAP_ADDRESS_RANGE flag is specified, the caller must ensure that no part of the specified address range (base \c *_virtualAddress, size - \a size) is wired. + \a size) is wired. The cache will also be temporarily unlocked. */ static status_t map_backing_store(VMAddressSpace* addressSpace, VMCache* cache, off_t offset, @@ -1005,8 +1005,13 @@ map_backing_store(VMAddressSpace* addressSpace, VMCache* cache, off_t offset, if (addressRestrictions->address_specification == B_EXACT_ADDRESS && (flags & CREATE_AREA_UNMAP_ADDRESS_RANGE) != 0) { + // temporarily unlock the current cache since it might be mapped to + // some existing area, and unmap_address_range also needs to lock that + // cache to delete the area. + cache->Unlock(); status = unmap_address_range(addressSpace, (addr_t)addressRestrictions->address, size, kernel); + cache->Lock(); if (status != B_OK) goto err2; } diff --git a/src/tests/system/kernel/Jamfile b/src/tests/system/kernel/Jamfile index afb3ec116d..04a6754874 100644 --- a/src/tests/system/kernel/Jamfile +++ b/src/tests/system/kernel/Jamfile @@ -56,6 +56,7 @@ SimpleTest port_wakeup_test_9 : port_wakeup_test_9.cpp ; SimpleTest mmap_resize_test : mmap_resize_test.cpp ; SimpleTest mmap_cut_tests : mmap_cut_tests.cpp ; +SimpleTest mmap_fixed_test : mmap_fixed_test.cpp ; SimpleTest null_poll_test : null_poll_test.cpp ; diff --git a/src/tests/system/kernel/mmap_fixed_test.cpp b/src/tests/system/kernel/mmap_fixed_test.cpp new file mode 100644 index 0000000000..76acac5956 --- /dev/null +++ b/src/tests/system/kernel/mmap_fixed_test.cpp @@ -0,0 +1,41 @@ +/* + * Copyright 2023, Trung Nguyen, trungnt282910@gmail.com. + * Distributed under the terms of the MIT License. + */ + +#include +#include +#include +#include +#include + +int +main() +{ + char tmpfile[] = "/tmp/mmap_fixed_test_XXXXXX"; + int fd = mkstemp(tmpfile); + + if (fd < 0) { + printf("cannot create temporary file.\n"); + return -1; + } + + unlink(tmpfile); + ftruncate(fd, 4096); + + // Should not crash the kernel (#18422) + void* addr = mmap(NULL, 4096, PROT_NONE, MAP_SHARED, fd, 0); + void* addr1 = mmap(addr, 4096, PROT_NONE, MAP_SHARED | MAP_FIXED, fd, 0); + + if (addr == MAP_FAILED || addr1 == MAP_FAILED) { + printf("mmap failed.\n"); + return -1; + } + + if (addr != addr1) { + printf("MAP_FIXED did not return same address.\n"); + return -1; + } + + return 0; +}