Skip to content

Fix arguments checking in IPC API of OS and file memory providers #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions src/provider/provider_file_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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)) {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
30 changes: 27 additions & 3 deletions src/provider/provider_os_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -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()) {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/provider/provider_os_memory_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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
Expand Down
Loading
Loading