Merge pull request oneapi-src#760 from ldorau/IPC_API_of_OS_memory_provider_requires_UMF_MEM_MAP_SHARED_visibility

lukaszstolarczuk · web-flow · commit 52e7da99b8a7 · 2024-09-30T15:45:31.000+02:00
Fix arguments checking in IPC API of OS and file memory providers
diff --git a/README.md b/README.md
@@ -143,6 +143,9 @@ OS memory provider supports two types of memory mappings (set by the `visibility
 1) private memory mapping (`UMF_MEM_MAP_PRIVATE`)
 2) shared memory mapping (`UMF_MEM_MAP_SHARED` - supported on Linux only yet)
 
+IPC API requires the `UMF_MEM_MAP_SHARED` memory `visibility` mode
+(`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise).
+
 There are available two mechanisms for the shared memory mapping:
 1) a named shared memory object (used if the `shm_name` parameter is not NULL) or
 2) an anonymous file descriptor (used if the `shm_name` parameter is NULL)
@@ -200,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/src/provider/provider_os_memory.c b/src/provider/provider_os_memory.c
@@ -402,6 +402,9 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
         return result;
     }
 
+    // IPC API requires in_params->visibility == UMF_MEM_MAP_SHARED
+    provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED);
+
     // NUMA config
     int emptyNodeset = in_params->numa_list_len == 0;
     result = validate_numa_mode(in_params->numa_mode, emptyNodeset);
@@ -1089,7 +1092,7 @@ static umf_result_t os_allocation_split(void *provider, void *ptr,
     (void)totalSize;
 
     os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
-    if (os_provider->fd <= 0) {
+    if (os_provider->fd < 0) {
         return UMF_RESULT_SUCCESS;
     }
 
@@ -1122,7 +1125,7 @@ static umf_result_t os_allocation_merge(void *provider, void *lowPtr,
     (void)totalSize;
 
     os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
-    if (os_provider->fd <= 0) {
+    if (os_provider->fd < 0) {
         return UMF_RESULT_SUCCESS;
     }
 
@@ -1152,6 +1155,10 @@ static umf_result_t os_get_ipc_handle_size(void *provider, size_t *size) {
     }
 
     os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
+    if (!os_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
 
     if (os_provider->shm_name[0]) {
         // os_ipc_data_t->shm_name will be used
@@ -1171,7 +1178,8 @@ static umf_result_t os_get_ipc_handle(void *provider, const void *ptr,
     }
 
     os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
-    if (os_provider->fd <= 0) {
+    if (!os_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
@@ -1203,6 +1211,11 @@ static umf_result_t os_put_ipc_handle(void *provider, void *providerIpcData) {
     }
 
     os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
+    if (!os_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     os_ipc_data_t *os_ipc_data = (os_ipc_data_t *)providerIpcData;
 
     if (os_ipc_data->pid != utils_getpid()) {
@@ -1229,6 +1242,11 @@ static umf_result_t os_open_ipc_handle(void *provider, void *providerIpcData,
     }
 
     os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
+    if (!os_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     os_ipc_data_t *os_ipc_data = (os_ipc_data_t *)providerIpcData;
     umf_result_t ret = UMF_RESULT_SUCCESS;
     int fd;
@@ -1269,6 +1287,12 @@ static umf_result_t os_close_ipc_handle(void *provider, void *ptr,
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
+    os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
+    if (!os_provider->IPC_enabled) {
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
+        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
+    }
+
     errno = 0;
     int ret = utils_munmap(ptr, size);
     // ignore error when size == 0
diff --git a/src/provider/provider_os_memory_internal.h b/src/provider/provider_os_memory_internal.h
@@ -9,6 +9,7 @@
 #define UMF_OS_MEMORY_PROVIDER_INTERNAL_H
 
 #include <limits.h>
+#include <stdbool.h>
 
 #if defined(_WIN32) && !defined(NAME_MAX)
 #include <stdlib.h>
@@ -29,8 +30,13 @@ extern "C" {
 typedef struct os_memory_provider_t {
     unsigned protection; // combination of OS-specific protection flags
     unsigned visibility; // memory visibility mode
+
+    // IPC is enabled only if (in_params->visibility == UMF_MEM_MAP_SHARED)
+    bool IPC_enabled;
+
     // a name of a shared memory file (valid only in case of the shared memory visibility)
     char shm_name[NAME_MAX];
+
     int fd;                // file descriptor for memory mapping
     size_t size_fd;        // size of file used for memory mapping
     size_t max_size_fd;    // maximum size of file used for memory mapping
diff --git a/test/provider_file_memory.cpp b/test/provider_file_memory.cpp
diff --git a/test/provider_os_memory.cpp b/test/provider_os_memory.cpp
