From b9fb35204055571682c765b15ff65a468b924db5 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 17 Oct 2024 11:23:17 +0200 Subject: [PATCH 1/2] Fix updating offset_mmap in file_alloc_aligned() Fixes: #796 Signed-off-by: Lukasz Dorau --- src/provider/provider_file_memory.c | 58 ++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index bb8cad2e3..274aab0b2 100644 --- a/src/provider/provider_file_memory.c +++ b/src/provider/provider_file_memory.c @@ -263,22 +263,38 @@ static umf_result_t file_mmap_aligned(file_memory_provider_t *file_provider, return UMF_RESULT_ERROR_INVALID_ARGUMENT; // arithmetic overflow } - if (offset_fd + extended_size > size_fd) { - if (utils_fallocate(fd, offset_fd, extended_size)) { + // offset_fd has to be also page-aligned since it is the offset of mmap() + size_t aligned_offset_fd = offset_fd; + rest = aligned_offset_fd & (page_size - 1); + if (rest) { + aligned_offset_fd += page_size - rest; + } + if (aligned_offset_fd < offset_fd) { + LOG_ERR("arithmetic overflow of file offset"); + return UMF_RESULT_ERROR_INVALID_ARGUMENT; // arithmetic overflow + } + + if (aligned_offset_fd + extended_size > size_fd) { + size_t new_size_fd = aligned_offset_fd + extended_size; + if (utils_fallocate(fd, size_fd, new_size_fd - size_fd)) { LOG_ERR("cannot grow the file size from %zu to %zu", size_fd, - offset_fd + extended_size); + new_size_fd); return UMF_RESULT_ERROR_UNKNOWN; } - LOG_DEBUG("file size grown from %zu to %zu", size_fd, - offset_fd + extended_size); - file_provider->size_fd = size_fd = offset_fd + extended_size; + LOG_DEBUG("file size grown from %zu to %zu", size_fd, new_size_fd); + file_provider->size_fd = new_size_fd; + } + + if (aligned_offset_fd > offset_fd) { + file_provider->offset_fd = aligned_offset_fd; } ASSERT_IS_ALIGNED(extended_size, page_size); - ASSERT_IS_ALIGNED(offset_fd, page_size); + ASSERT_IS_ALIGNED(aligned_offset_fd, page_size); - void *ptr = utils_mmap_file(NULL, extended_size, prot, flag, fd, offset_fd); + void *ptr = + utils_mmap_file(NULL, extended_size, prot, flag, fd, aligned_offset_fd); if (ptr == NULL) { LOG_PERR("memory mapping failed"); return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; @@ -292,6 +308,10 @@ static umf_result_t file_mmap_aligned(file_memory_provider_t *file_provider, ptr, extended_size); } + LOG_DEBUG( + "inserted a value to the map of memory mapping (addr=%p, size=%zu)", + ptr, extended_size); + file_provider->base_mmap = ptr; file_provider->size_mmap = extended_size; file_provider->offset_mmap = 0; @@ -335,19 +355,31 @@ static umf_result_t file_alloc_aligned(file_memory_provider_t *file_provider, } size_t new_offset_mmap = new_aligned_ptr - (uintptr_t)base_mmap; + size_t new_offset_fd = + file_provider->offset_fd + new_offset_mmap - file_provider->offset_mmap; + if (file_provider->size_mmap - new_offset_mmap < size) { umf_result = file_mmap_aligned(file_provider, size, alignment); if (umf_result != UMF_RESULT_SUCCESS) { utils_mutex_unlock(&file_provider->lock); return umf_result; } + + assert(file_provider->base_mmap); + + // file_provider-> base_mmap, offset_mmap, offset_fd + // were updated by file_mmap_aligned(): + new_aligned_ptr = (uintptr_t)file_provider->base_mmap; + new_offset_mmap = 0; // == file_provider->offset_mmap + new_offset_fd = file_provider->offset_fd; + + ASSERT_IS_ALIGNED(new_aligned_ptr, alignment); } - size_t old_offset_mmap = file_provider->offset_mmap; - file_provider->offset_mmap = new_offset_mmap; - *alloc_offset_fd = - file_provider->offset_fd + new_offset_mmap - old_offset_mmap; - file_provider->offset_fd = *alloc_offset_fd + size; + *alloc_offset_fd = new_offset_fd; + + file_provider->offset_fd = new_offset_fd + size; + file_provider->offset_mmap = new_offset_mmap + size; *out_addr = (void *)new_aligned_ptr; From 62444d34cf8600bd4ba27a308d53b8f1c5c9bd45 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 17 Oct 2024 11:25:39 +0200 Subject: [PATCH 2/2] Add the two_allocations test Signed-off-by: Lukasz Dorau --- test/provider_file_memory.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/provider_file_memory.cpp b/test/provider_file_memory.cpp index a00f31adc..b2c71635d 100644 --- a/test/provider_file_memory.cpp +++ b/test/provider_file_memory.cpp @@ -183,6 +183,38 @@ INSTANTIATE_TEST_SUITE_P(fileProviderTest, FileProviderParamsDefault, TEST_P(FileProviderParamsDefault, create_destroy) {} +TEST_P(FileProviderParamsDefault, two_allocations) { + umf_result_t umf_result; + void *ptr1 = nullptr; + void *ptr2 = nullptr; + size_t size = page_plus_64; + size_t alignment = page_size; + + umf_result = umfMemoryProviderAlloc(provider.get(), size, alignment, &ptr1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + ASSERT_NE(ptr1, nullptr); + + umf_result = umfMemoryProviderAlloc(provider.get(), size, alignment, &ptr2); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + ASSERT_NE(ptr2, nullptr); + + ASSERT_NE(ptr1, ptr2); + if ((uintptr_t)ptr1 > (uintptr_t)ptr2) { + ASSERT_GT((uintptr_t)ptr1 - (uintptr_t)ptr2, size); + } else { + ASSERT_GT((uintptr_t)ptr2 - (uintptr_t)ptr1, size); + } + + memset(ptr1, 0x11, size); + memset(ptr2, 0x22, size); + + umf_result = umfMemoryProviderFree(provider.get(), ptr1, size); + ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + + umf_result = umfMemoryProviderFree(provider.get(), ptr2, size); + ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); +} + TEST_P(FileProviderParamsDefault, alloc_page64_align_0) { test_alloc_free_success(provider.get(), page_plus_64, 0, PURGE_NONE); }