Merge pull request #800 from ldorau/Fix_updating_offset_mmap_in_file_alloc_aligned

Fix `file_alloc()` of the file memory provider

Author: lplewa

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;
 

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);
 }