File provider IPC API requires UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC

ldorau · ldorau · commit b726d3e94596 · 2024-09-26T18:14:02.000+02:00
IPC API of file memory provider requires the `UMF_MEM_MAP_SHARED`
or `UMF_MEM_MAP_SYNC` memory `visibility` mode
(`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise).

Signed-off-by: Lukasz Dorau &lt;lukasz.dorau@intel.com&gt;
diff --git a/README.md b/README.md
@@ -203,6 +203,9 @@ so it should be used with a pool manager that will take over
 the managing of the provided memory - for example the jemalloc pool
 with the `disable_provider_free` parameter set to true.
 
+IPC API requires the `UMF_MEM_MAP_SHARED` or `UMF_MEM_MAP_SYNC` memory `visibility` mode
+(`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise).
+
 The memory visibility mode parameter must be set to `UMF_MEM_MAP_SYNC` in case of FSDAX.
 
 ##### Requirements
diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c
@@ -8,6 +8,7 @@
 #include <assert.h>
 #include <errno.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -41,6 +42,9 @@ typedef struct file_memory_provider_t {
     unsigned visibility; // memory visibility mode
     size_t page_size;    // minimum page size
 
+    // IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
+    bool IPC_enabled;
+
     critnib *mmaps; // a critnib map storing mmap mappings (addr, size)
 
     // A critnib map storing (ptr, fd_offset + 1) pairs. We add 1 to fd_offset
@@ -101,6 +105,10 @@ file_translate_params(umf_file_memory_provider_params_t *in_params,
         return result;
     }
 
+    // IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
+    provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED ||
+                             in_params->visibility == UMF_MEM_MAP_SYNC);
+
     return UMF_RESULT_SUCCESS;
 }
 
@@ -545,6 +553,13 @@ static umf_result_t file_get_ipc_handle_size(void *provider, size_t *size) {
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
+    file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
+    if (!file_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
+                "UMF_MEM_MAP_SYNC")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     *size = sizeof(file_ipc_data_t);
 
     return UMF_RESULT_SUCCESS;
@@ -557,6 +572,11 @@ static umf_result_t file_get_ipc_handle(void *provider, const void *ptr,
     }
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
+    if (!file_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
+                "UMF_MEM_MAP_SYNC")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
 
     void *value = critnib_get(file_provider->fd_offset_map, (uintptr_t)ptr);
     if (value == NULL) {
@@ -581,6 +601,12 @@ static umf_result_t file_put_ipc_handle(void *provider, void *providerIpcData) {
     }
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
+    if (!file_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
+                "UMF_MEM_MAP_SYNC")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     file_ipc_data_t *file_ipc_data = (file_ipc_data_t *)providerIpcData;
 
     if (strncmp(file_ipc_data->path, file_provider->path, PATH_MAX)) {
@@ -597,6 +623,12 @@ static umf_result_t file_open_ipc_handle(void *provider, void *providerIpcData,
     }
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
+    if (!file_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
+                "UMF_MEM_MAP_SYNC")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     file_ipc_data_t *file_ipc_data = (file_ipc_data_t *)providerIpcData;
     umf_result_t ret = UMF_RESULT_SUCCESS;
     int fd;
@@ -631,6 +663,13 @@ static umf_result_t file_close_ipc_handle(void *provider, void *ptr,
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
+    file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
+    if (!file_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
+                "UMF_MEM_MAP_SYNC")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     errno = 0;
     int ret = utils_munmap(ptr, size);
     // ignore error when size == 0
diff --git a/test/provider_file_memory.cpp b/test/provider_file_memory.cpp
@@ -55,7 +55,7 @@ static void providerCreateExt(providerCreateExtParams params,
         umf::provider_unique_handle_t(hProvider, &umfMemoryProviderDestroy);
 }
 
-struct umfProviderTest
+struct FileProviderParamsDefault
     : umf_test::test,
       ::testing::WithParamInterface<providerCreateExtParams> {
     void SetUp() override {
@@ -75,6 +75,8 @@ struct umfProviderTest
     size_t page_plus_64;
 };
 
+struct FileProviderParamsShared : FileProviderParamsDefault {};
+
 static void test_alloc_free_success(umf_memory_provider_handle_t provider,
                                     size_t size, size_t alignment,
                                     purge_t purge) {
@@ -161,6 +163,9 @@ TEST_F(test, test_if_mapped_with_MAP_SYNC) {
 
 // positive tests using test_alloc_free_success
 
+umf_file_memory_provider_params_t file_params_default =
+    umfFileMemoryProviderParamsDefault(FILE_PATH);
+
 umf_file_memory_provider_params_t get_file_params_shared(char *path) {
     umf_file_memory_provider_params_t file_params =
         umfFileMemoryProviderParamsDefault(path);
@@ -171,68 +176,108 @@ umf_file_memory_provider_params_t get_file_params_shared(char *path) {
 umf_file_memory_provider_params_t file_params_shared =
     get_file_params_shared(FILE_PATH);
 
-INSTANTIATE_TEST_SUITE_P(fileProviderTest, umfProviderTest,
+INSTANTIATE_TEST_SUITE_P(fileProviderTest, FileProviderParamsDefault,
                          ::testing::Values(providerCreateExtParams{
-                             umfFileMemoryProviderOps(), &file_params_shared}));
+                             umfFileMemoryProviderOps(),
+                             &file_params_default}));
 
-TEST_P(umfProviderTest, create_destroy) {}
+TEST_P(FileProviderParamsDefault, create_destroy) {}
 
-TEST_P(umfProviderTest, alloc_page64_align_0) {
+TEST_P(FileProviderParamsDefault, alloc_page64_align_0) {
     test_alloc_free_success(provider.get(), page_plus_64, 0, PURGE_NONE);
 }
 
-TEST_P(umfProviderTest, alloc_page64_align_page_div_2) {
+TEST_P(FileProviderParamsDefault, alloc_page64_align_page_div_2) {
     test_alloc_free_success(provider.get(), page_plus_64, page_size / 2,
                             PURGE_NONE);
 }
 
-TEST_P(umfProviderTest, purge_lazy) {
+TEST_P(FileProviderParamsDefault, purge_lazy) {
     test_alloc_free_success(provider.get(), page_plus_64, 0, PURGE_LAZY);
 }
 
-TEST_P(umfProviderTest, purge_force) {
+TEST_P(FileProviderParamsDefault, purge_force) {
     test_alloc_free_success(provider.get(), page_plus_64, 0, PURGE_FORCE);
 }
 
 // negative tests using test_alloc_failure
 
-TEST_P(umfProviderTest, alloc_WRONG_SIZE) {
+TEST_P(FileProviderParamsDefault, alloc_WRONG_SIZE) {
     test_alloc_failure(provider.get(), -1, 0, UMF_RESULT_ERROR_INVALID_ARGUMENT,
                        0);
 }
 
-TEST_P(umfProviderTest, alloc_page64_WRONG_ALIGNMENT_3_pages) {
+TEST_P(FileProviderParamsDefault, alloc_page64_WRONG_ALIGNMENT_3_pages) {
     test_alloc_failure(provider.get(), page_plus_64, 3 * page_size,
                        UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0);
 }
 
-TEST_P(umfProviderTest, alloc_3pages_WRONG_ALIGNMENT_3pages) {
+TEST_P(FileProviderParamsDefault, alloc_3pages_WRONG_ALIGNMENT_3pages) {
     test_alloc_failure(provider.get(), 3 * page_size, 3 * page_size,
                        UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0);
 }
 
-TEST_P(umfProviderTest, alloc_page64_align_page_minus_1_WRONG_ALIGNMENT_1) {
+TEST_P(FileProviderParamsDefault,
+       alloc_page64_align_page_minus_1_WRONG_ALIGNMENT_1) {
     test_alloc_failure(provider.get(), page_plus_64, page_size - 1,
                        UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0);
 }
 
-TEST_P(umfProviderTest, alloc_page64_align_one_half_pages_WRONG_ALIGNMENT_2) {
+TEST_P(FileProviderParamsDefault,
+       alloc_page64_align_one_half_pages_WRONG_ALIGNMENT_2) {
     test_alloc_failure(provider.get(), page_plus_64,
                        page_size + (page_size / 2),
                        UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0);
 }
 
+// negative IPC tests
+
+TEST_P(FileProviderParamsDefault, get_ipc_handle_size_wrong_visibility) {
+    size_t size;
+    umf_result_t umf_result =
+        umfMemoryProviderGetIPCHandleSize(provider.get(), &size);
+    ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_P(FileProviderParamsDefault, get_ipc_handle_wrong_visibility) {
+    char providerIpcData;
+    umf_result_t umf_result = umfMemoryProviderGetIPCHandle(
+        provider.get(), INVALID_PTR, 1, &providerIpcData);
+    ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_P(FileProviderParamsDefault, put_ipc_handle_wrong_visibility) {
+    char providerIpcData;
+    umf_result_t umf_result =
+        umfMemoryProviderPutIPCHandle(provider.get(), &providerIpcData);
+    ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_P(FileProviderParamsDefault, open_ipc_handle_wrong_visibility) {
+    char providerIpcData;
+    void *ptr;
+    umf_result_t umf_result =
+        umfMemoryProviderOpenIPCHandle(provider.get(), &providerIpcData, &ptr);
+    ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_P(FileProviderParamsDefault, close_ipc_handle_wrong_visibility) {
+    umf_result_t umf_result =
+        umfMemoryProviderCloseIPCHandle(provider.get(), INVALID_PTR, 1);
+    ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
 // other positive tests
 
-TEST_P(umfProviderTest, get_min_page_size) {
+TEST_P(FileProviderParamsDefault, get_min_page_size) {
     size_t min_page_size;
     umf_result_t umf_result = umfMemoryProviderGetMinPageSize(
         provider.get(), nullptr, &min_page_size);
     ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS);
     ASSERT_LE(min_page_size, page_size);
 }
 
-TEST_P(umfProviderTest, get_recommended_page_size) {
+TEST_P(FileProviderParamsDefault, get_recommended_page_size) {
     size_t min_page_size;
     umf_result_t umf_result = umfMemoryProviderGetMinPageSize(
         provider.get(), nullptr, &min_page_size);
@@ -246,18 +291,18 @@ TEST_P(umfProviderTest, get_recommended_page_size) {
     ASSERT_GE(recommended_page_size, min_page_size);
 }
 
-TEST_P(umfProviderTest, get_name) {
+TEST_P(FileProviderParamsDefault, get_name) {
     const char *name = umfMemoryProviderGetName(provider.get());
     ASSERT_STREQ(name, "FILE");
 }
 
-TEST_P(umfProviderTest, free_size_0_ptr_not_null) {
+TEST_P(FileProviderParamsDefault, free_size_0_ptr_not_null) {
     umf_result_t umf_result =
         umfMemoryProviderFree(provider.get(), INVALID_PTR, 0);
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED);
 }
 
-TEST_P(umfProviderTest, free_NULL) {
+TEST_P(FileProviderParamsDefault, free_NULL) {
     umf_result_t umf_result = umfMemoryProviderFree(provider.get(), nullptr, 0);
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED);
 }
@@ -274,19 +319,19 @@ TEST_F(test, create_empty_path) {
     EXPECT_EQ(hProvider, nullptr);
 }
 
-TEST_P(umfProviderTest, free_INVALID_POINTER_SIZE_GT_0) {
+TEST_P(FileProviderParamsDefault, free_INVALID_POINTER_SIZE_GT_0) {
     umf_result_t umf_result =
         umfMemoryProviderFree(provider.get(), INVALID_PTR, page_plus_64);
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED);
 }
 
-TEST_P(umfProviderTest, purge_lazy_INVALID_POINTER) {
+TEST_P(FileProviderParamsDefault, purge_lazy_INVALID_POINTER) {
     umf_result_t umf_result =
         umfMemoryProviderPurgeLazy(provider.get(), INVALID_PTR, 1);
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED);
 }
 
-TEST_P(umfProviderTest, purge_force_INVALID_POINTER) {
+TEST_P(FileProviderParamsDefault, purge_force_INVALID_POINTER) {
     umf_result_t umf_result =
         umfMemoryProviderPurgeForce(provider.get(), INVALID_PTR, 1);
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC);
@@ -297,7 +342,11 @@ TEST_P(umfProviderTest, purge_force_INVALID_POINTER) {
 
 // IPC tests
 
-TEST_P(umfProviderTest, IPC_base_success_test) {
+INSTANTIATE_TEST_SUITE_P(fileProviderTest, FileProviderParamsShared,
+                         ::testing::Values(providerCreateExtParams{
+                             umfFileMemoryProviderOps(), &file_params_shared}));
+
+TEST_P(FileProviderParamsShared, IPC_base_success_test) {
     umf_result_t umf_result;
     void *ptr = nullptr;
     size_t size = page_size;
@@ -338,7 +387,7 @@ TEST_P(umfProviderTest, IPC_base_success_test) {
     ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED);
 }
 
-TEST_P(umfProviderTest, IPC_file_not_exist) {
+TEST_P(FileProviderParamsShared, IPC_file_not_exist) {
     umf_result_t umf_result;
     void *ptr = nullptr;
     size_t size = page_size;
