Skip to content

Commit 52e7da9

Browse files
lukaszstolarczuklukaszstolarczuk
authored
Merge pull request #760 from ldorau/IPC_API_of_OS_memory_provider_requires_UMF_MEM_MAP_SHARED_visibility
Fix arguments checking in IPC API of OS and file memory providers
2 parents 38e1c83 + b726d3e commit 52e7da9

File tree

6 files changed

+185
-26
lines changed

6 files changed

+185
-26
lines changed

‎README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ OS memory provider supports two types of memory mappings (set by the `visibility
143143
1) private memory mapping (`UMF_MEM_MAP_PRIVATE`)
144144
2) shared memory mapping (`UMF_MEM_MAP_SHARED` - supported on Linux only yet)
145145

146+
IPC API requires the `UMF_MEM_MAP_SHARED` memory `visibility` mode
147+
(`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise).
148+
146149
There are available two mechanisms for the shared memory mapping:
147150
1) a named shared memory object (used if the `shm_name` parameter is not NULL) or
148151
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
200203
the managing of the provided memory - for example the jemalloc pool
201204
with the `disable_provider_free` parameter set to true.
202205

206+
IPC API requires the `UMF_MEM_MAP_SHARED` or `UMF_MEM_MAP_SYNC` memory `visibility` mode
207+
(`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise).
208+
203209
The memory visibility mode parameter must be set to `UMF_MEM_MAP_SYNC` in case of FSDAX.
204210

205211
##### Requirements

‎src/provider/provider_file_memory.c

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <assert.h>
99
#include <errno.h>
1010
#include <limits.h>
11+
#include <stdbool.h>
1112
#include <stddef.h>
1213
#include <stdio.h>
1314
#include <stdlib.h>
@@ -41,6 +42,9 @@ typedef struct file_memory_provider_t {
4142
unsigned visibility; // memory visibility mode
4243
size_t page_size; // minimum page size
4344

45+
// IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
46+
bool IPC_enabled;
47+
4448
critnib *mmaps; // a critnib map storing mmap mappings (addr, size)
4549

4650
// 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,
101105
return result;
102106
}
103107

108+
// IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
109+
provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED ||
110+
in_params->visibility == UMF_MEM_MAP_SYNC);
111+
104112
return UMF_RESULT_SUCCESS;
105113
}
106114

@@ -545,6 +553,13 @@ static umf_result_t file_get_ipc_handle_size(void *provider, size_t *size) {
545553
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
546554
}
547555

556+
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
557+
if (!file_provider->IPC_enabled) {
558+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
559+
"UMF_MEM_MAP_SYNC")
560+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
561+
}
562+
548563
*size = sizeof(file_ipc_data_t);
549564

550565
return UMF_RESULT_SUCCESS;
@@ -557,6 +572,11 @@ static umf_result_t file_get_ipc_handle(void *provider, const void *ptr,
557572
}
558573

559574
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
575+
if (!file_provider->IPC_enabled) {
576+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
577+
"UMF_MEM_MAP_SYNC")
578+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
579+
}
560580

561581
void *value = critnib_get(file_provider->fd_offset_map, (uintptr_t)ptr);
562582
if (value == NULL) {
@@ -581,6 +601,12 @@ static umf_result_t file_put_ipc_handle(void *provider, void *providerIpcData) {
581601
}
582602

583603
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
604+
if (!file_provider->IPC_enabled) {
605+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
606+
"UMF_MEM_MAP_SYNC")
607+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
608+
}
609+
584610
file_ipc_data_t *file_ipc_data = (file_ipc_data_t *)providerIpcData;
585611

586612
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,
597623
}
598624

599625
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
626+
if (!file_provider->IPC_enabled) {
627+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
628+
"UMF_MEM_MAP_SYNC")
629+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
630+
}
631+
600632
file_ipc_data_t *file_ipc_data = (file_ipc_data_t *)providerIpcData;
601633
umf_result_t ret = UMF_RESULT_SUCCESS;
602634
int fd;
@@ -631,6 +663,13 @@ static umf_result_t file_close_ipc_handle(void *provider, void *ptr,
631663
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
632664
}
633665

666+
file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
667+
if (!file_provider->IPC_enabled) {
668+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
669+
"UMF_MEM_MAP_SYNC")
670+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
671+
}
672+
634673
errno = 0;
635674
int ret = utils_munmap(ptr, size);
636675
// ignore error when size == 0

‎src/provider/provider_os_memory.c

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,9 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
402402
return result;
403403
}
404404

405+
// IPC API requires in_params->visibility == UMF_MEM_MAP_SHARED
406+
provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED);
407+
405408
// NUMA config
406409
int emptyNodeset = in_params->numa_list_len == 0;
407410
result = validate_numa_mode(in_params->numa_mode, emptyNodeset);
@@ -1089,7 +1092,7 @@ static umf_result_t os_allocation_split(void *provider, void *ptr,
10891092
(void)totalSize;
10901093

10911094
os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
1092-
if (os_provider->fd <= 0) {
1095+
if (os_provider->fd < 0) {
10931096
return UMF_RESULT_SUCCESS;
10941097
}
10951098

@@ -1122,7 +1125,7 @@ static umf_result_t os_allocation_merge(void *provider, void *lowPtr,
11221125
(void)totalSize;
11231126

11241127
os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
1125-
if (os_provider->fd <= 0) {
1128+
if (os_provider->fd < 0) {
11261129
return UMF_RESULT_SUCCESS;
11271130
}
11281131

@@ -1152,6 +1155,10 @@ static umf_result_t os_get_ipc_handle_size(void *provider, size_t *size) {
11521155
}
11531156

11541157
os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
1158+
if (!os_provider->IPC_enabled) {
1159+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
1160+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
1161+
}
11551162

11561163
if (os_provider->shm_name[0]) {
11571164
// 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,
11711178
}
11721179

11731180
os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
1174-
if (os_provider->fd <= 0) {
1181+
if (!os_provider->IPC_enabled) {
1182+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
11751183
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
11761184
}
11771185

@@ -1203,6 +1211,11 @@ static umf_result_t os_put_ipc_handle(void *provider, void *providerIpcData) {
12031211
}
12041212

12051213
os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
1214+
if (!os_provider->IPC_enabled) {
1215+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
1216+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
1217+
}
1218+
12061219
os_ipc_data_t *os_ipc_data = (os_ipc_data_t *)providerIpcData;
12071220

12081221
if (os_ipc_data->pid != utils_getpid()) {
@@ -1229,6 +1242,11 @@ static umf_result_t os_open_ipc_handle(void *provider, void *providerIpcData,
12291242
}
12301243

12311244
os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
1245+
if (!os_provider->IPC_enabled) {
1246+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
1247+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
1248+
}
1249+
12321250
os_ipc_data_t *os_ipc_data = (os_ipc_data_t *)providerIpcData;
12331251
umf_result_t ret = UMF_RESULT_SUCCESS;
12341252
int fd;
@@ -1269,6 +1287,12 @@ static umf_result_t os_close_ipc_handle(void *provider, void *ptr,
12691287
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
12701288
}
12711289

1290+
os_memory_provider_t *os_provider = (os_memory_provider_t *)provider;
1291+
if (!os_provider->IPC_enabled) {
1292+
LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
1293+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
1294+
}
1295+
12721296
errno = 0;
12731297
int ret = utils_munmap(ptr, size);
12741298
// ignore error when size == 0

‎src/provider/provider_os_memory_internal.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#define UMF_OS_MEMORY_PROVIDER_INTERNAL_H
1010

1111
#include <limits.h>
12+
#include <stdbool.h>
1213

1314
#if defined(_WIN32) && !defined(NAME_MAX)
1415
#include <stdlib.h>
@@ -29,8 +30,13 @@ extern "C" {
2930
typedef struct os_memory_provider_t {
3031
unsigned protection; // combination of OS-specific protection flags
3132
unsigned visibility; // memory visibility mode
33+
34+
// IPC is enabled only if (in_params->visibility == UMF_MEM_MAP_SHARED)
35+
bool IPC_enabled;
36+
3237
// a name of a shared memory file (valid only in case of the shared memory visibility)
3338
char shm_name[NAME_MAX];
39+
3440
int fd; // file descriptor for memory mapping
3541
size_t size_fd; // size of file used for memory mapping
3642
size_t max_size_fd; // maximum size of file used for memory mapping

0 commit comments

Comments
 (0)