From 7706431123d5c714492654ca810814177928a970 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 24 Oct 2024 15:12:49 +0200 Subject: [PATCH] Fix devdax_open_ipc_handle() and devdax_close_ipc_handle() devdax_open_ipc_handle() has to use the path of the remote /dev/dax got from the IPC handle, not the local one. devdax_open_ipc_handle() has to use also the memory protection got from the IPC handle, so let's add the memory protection to the IPC handle. devdax_open_ipc_handle() should mmap only the required range of memory, not the whole /dev/dax device, so let's add the length of the allocation to the IPC handle. devdax_close_ipc_handle() should unmap only the required range of memory, not the whole /dev/dax device. Signed-off-by: Lukasz Dorau --- src/provider/provider_devdax_memory.c | 72 +++++++++------------------ 1 file changed, 23 insertions(+), 49 deletions(-) diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index 544960995..0e4c68820 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -369,9 +369,10 @@ static umf_result_t devdax_allocation_merge(void *provider, void *lowPtr, } typedef struct devdax_ipc_data_t { - char dd_path[PATH_MAX]; // path to the /dev/dax - size_t dd_size; // size of the /dev/dax - size_t offset; // offset of the data + char path[PATH_MAX]; // path to the /dev/dax + unsigned protection; // combination of OS-specific memory protection flags + size_t offset; // offset of the data + size_t length; // length of the data } devdax_ipc_data_t; static umf_result_t devdax_get_ipc_handle_size(void *provider, size_t *size) { @@ -386,8 +387,6 @@ static umf_result_t devdax_get_ipc_handle_size(void *provider, size_t *size) { static umf_result_t devdax_get_ipc_handle(void *provider, const void *ptr, size_t size, void *providerIpcData) { - (void)size; // unused - if (provider == NULL || ptr == NULL || providerIpcData == NULL) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } @@ -396,11 +395,12 @@ static umf_result_t devdax_get_ipc_handle(void *provider, const void *ptr, (devdax_memory_provider_t *)provider; devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData; + strncpy(devdax_ipc_data->path, devdax_provider->path, PATH_MAX - 1); + devdax_ipc_data->path[PATH_MAX - 1] = '\0'; + devdax_ipc_data->protection = devdax_provider->protection; devdax_ipc_data->offset = (size_t)((uintptr_t)ptr - (uintptr_t)devdax_provider->base); - strncpy(devdax_ipc_data->dd_path, devdax_provider->path, PATH_MAX - 1); - devdax_ipc_data->dd_path[PATH_MAX - 1] = '\0'; - devdax_ipc_data->dd_size = devdax_provider->size; + devdax_ipc_data->length = size; return UMF_RESULT_SUCCESS; } @@ -416,16 +416,9 @@ static umf_result_t devdax_put_ipc_handle(void *provider, devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData; // verify the path of the /dev/dax - if (strncmp(devdax_ipc_data->dd_path, devdax_provider->path, PATH_MAX)) { + if (strncmp(devdax_ipc_data->path, devdax_provider->path, PATH_MAX)) { LOG_ERR("devdax path mismatch (local: %s, ipc: %s)", - devdax_provider->path, devdax_ipc_data->dd_path); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - // verify the size of the /dev/dax - if (devdax_ipc_data->dd_size != devdax_provider->size) { - LOG_ERR("devdax size mismatch (local: %zu, ipc: %zu)", - devdax_provider->size, devdax_ipc_data->dd_size); + devdax_provider->path, devdax_ipc_data->path); return UMF_RESULT_ERROR_INVALID_ARGUMENT; } @@ -438,28 +431,12 @@ static umf_result_t devdax_open_ipc_handle(void *provider, return UMF_RESULT_ERROR_INVALID_ARGUMENT; } - devdax_memory_provider_t *devdax_provider = - (devdax_memory_provider_t *)provider; devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData; - // verify it is the same devdax - first verify the path - if (strncmp(devdax_ipc_data->dd_path, devdax_provider->path, PATH_MAX)) { - LOG_ERR("devdax path mismatch (local: %s, ipc: %s)", - devdax_provider->path, devdax_ipc_data->dd_path); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - // verify the size of the /dev/dax - if (devdax_ipc_data->dd_size != devdax_provider->size) { - LOG_ERR("devdax size mismatch (local: %zu, ipc: %zu)", - devdax_provider->size, devdax_ipc_data->dd_size); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - umf_result_t ret = UMF_RESULT_SUCCESS; - int fd = utils_devdax_open(devdax_provider->path); + int fd = utils_devdax_open(devdax_ipc_data->path); if (fd == -1) { - LOG_PERR("opening a devdax (%s) failed", devdax_provider->path); + LOG_PERR("opening the devdax (%s) failed", devdax_ipc_data->path); return UMF_RESULT_ERROR_INVALID_ARGUMENT; } @@ -467,27 +444,27 @@ static umf_result_t devdax_open_ipc_handle(void *provider, utils_translate_mem_visibility_flag(UMF_MEM_MAP_SYNC, &map_sync_flag); // mmap /dev/dax with the MAP_SYNC xor MAP_SHARED flag (if MAP_SYNC fails) - char *base = utils_mmap_file(NULL, devdax_provider->size, - devdax_provider->protection, map_sync_flag, fd, - 0 /* offset */); - if (base == NULL) { + char *addr = utils_mmap_file(NULL, devdax_ipc_data->length, + devdax_ipc_data->protection, map_sync_flag, fd, + devdax_ipc_data->offset); + if (addr == NULL) { devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED, errno); LOG_PERR("devdax mapping failed (path: %s, size: %zu, protection: %i, " - "fd: %i)", - devdax_provider->path, devdax_provider->size, - devdax_provider->protection, fd); + "fd: %i, offset: %zu)", + devdax_ipc_data->path, devdax_ipc_data->length, + devdax_ipc_data->protection, fd, devdax_ipc_data->offset); ret = UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; } LOG_DEBUG("devdax mapped (path: %s, size: %zu, protection: %i, fd: %i, " "offset: %zu)", - devdax_provider->path, devdax_provider->size, - devdax_provider->protection, fd, devdax_ipc_data->offset); + devdax_ipc_data->path, devdax_ipc_data->length, + devdax_ipc_data->protection, fd, devdax_ipc_data->offset); (void)utils_close_fd(fd); - *ptr = base + devdax_ipc_data->offset; + *ptr = addr; return ret; } @@ -498,11 +475,8 @@ static umf_result_t devdax_close_ipc_handle(void *provider, void *ptr, return UMF_RESULT_ERROR_INVALID_ARGUMENT; } - devdax_memory_provider_t *devdax_provider = - (devdax_memory_provider_t *)provider; - errno = 0; - int ret = utils_munmap(devdax_provider->base, devdax_provider->size); + int ret = utils_munmap(ptr, size); // ignore error when size == 0 if (ret && (size > 0)) { devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_FREE_FAILED,