From 3f103151b1d54d76bb46179a3dac05e918983e79 Mon Sep 17 00:00:00 2001 From: "Neil.wrz" Date: Sun, 7 May 2023 02:37:30 -0700 Subject: [PATCH 62/79] fix security warning Signed-off-by: Neil.wrz --- .../oci/storage/image_store/image_store.c | 2 +- .../remote_layer_support/image_remote_impl.c | 36 +++++++++++++--- .../remote_layer_support/layer_remote_impl.c | 41 +++++++++++++++---- .../overlay_remote_impl.c | 38 ++++++++++++++--- .../remote_layer_support/remote_support.c | 19 ++++++++- .../ro_symlink_maintain.c | 2 - .../modules/image/oci/storage/storage.c | 2 + 7 files changed, 116 insertions(+), 24 deletions(-) diff --git a/src/daemon/modules/image/oci/storage/image_store/image_store.c b/src/daemon/modules/image/oci/storage/image_store/image_store.c index 7837f9db..d436eba2 100644 --- a/src/daemon/modules/image/oci/storage/image_store/image_store.c +++ b/src/daemon/modules/image/oci/storage/image_store/image_store.c @@ -3647,7 +3647,7 @@ int image_store_init(struct storage_module_init_options *opts) } #ifdef ENABLE_REMOTE_LAYER_STORE - remote_image_init(g_image_store->dir); + ret = remote_image_init(g_image_store->dir); #endif out: diff --git a/src/daemon/modules/image/oci/storage/remote_layer_support/image_remote_impl.c b/src/daemon/modules/image/oci/storage/remote_layer_support/image_remote_impl.c index a822ea81..1ac0139f 100644 --- a/src/daemon/modules/image/oci/storage/remote_layer_support/image_remote_impl.c +++ b/src/daemon/modules/image/oci/storage/remote_layer_support/image_remote_impl.c @@ -36,10 +36,28 @@ struct remote_image_data *remote_image_create(const char *remote_home, const cha ERROR("Out of memory"); return NULL; } + data->image_home = remote_home; image_byid_old = map_new(MAP_STR_BOOL, MAP_DEFAULT_CMP_FUNC, MAP_DEFAULT_FREE_FUNC); + if (image_byid_old == NULL) { + ERROR("Failed to cerate image_byid_old"); + goto free_out; + } + image_byid_new = map_new(MAP_STR_BOOL, MAP_DEFAULT_CMP_FUNC, MAP_DEFAULT_FREE_FUNC); + if (image_byid_new == NULL) { + ERROR("Failed to cerate image_byid_new"); + goto free_out; + } + return data; + +free_out: + map_free(image_byid_old); + map_free(image_byid_new); + free(data); + + return NULL; } void remote_image_destroy(struct remote_image_data *data) @@ -97,7 +115,9 @@ static int remote_dir_scan(void *data) // for refresh, we don't care v1 image, cause image should be handled by master isulad // when master isulad pull images if (!is_v1_image) { - map_insert(image_byid_new, util_strdup_s(image_dirs[i]), (void *)&exist); + if (!map_insert(image_byid_new, image_dirs[i], (void *)&exist)) { + WARN("Failed to insert image %s to map", image_dirs[i]); + } } } @@ -113,7 +133,7 @@ static int remote_image_add(void *data) char *top_layer = NULL; map_t *tmp_map = NULL; bool exist = true; - int i = 0; + size_t i = 0; int ret = 0; if (data == NULL) { @@ -127,13 +147,17 @@ static int remote_image_add(void *data) top_layer = remote_image_get_top_layer_from_json(array_added[i]); if (top_layer != NULL && !remote_layer_layer_valid(top_layer)) { WARN("Current not find valid under layer, remoet image:%s not added", array_added[i]); - map_remove(image_byid_new, (void *)array_added[i]); + if (!map_remove(image_byid_new, (void *)array_added[i])) { + WARN("image %s will not be loaded from remote.", array_added[i]); + } continue; } if (remote_append_image_by_directory_with_lock(array_added[i]) != 0) { ERROR("Failed to load image into memrory: %s", array_added[i]); - map_remove(image_byid_new, (void *)array_added[i]); + if (!map_remove(image_byid_new, (void *)array_added[i])) { + WARN("image %s will not be loaded from remote", array_added[i]); + } ret = -1; } } @@ -141,7 +165,9 @@ static int remote_image_add(void *data) for (i = 0; i < util_array_len((const char **)array_deleted); i++) { if (remote_remove_image_from_memory_with_lock(array_deleted[i]) != 0) { ERROR("Failed to remove remote memory store"); - map_insert(image_byid_new, array_deleted[i], (void *)&exist); + if (!map_insert(image_byid_new, array_deleted[i], (void *)&exist)) { + WARN("image %s will not be removed from local", array_deleted[i]); + } ret = -1; } } diff --git a/src/daemon/modules/image/oci/storage/remote_layer_support/layer_remote_impl.c b/src/daemon/modules/image/oci/storage/remote_layer_support/layer_remote_impl.c index 3e3afff6..b1a1e944 100644 --- a/src/daemon/modules/image/oci/storage/remote_layer_support/layer_remote_impl.c +++ b/src/daemon/modules/image/oci/storage/remote_layer_support/layer_remote_impl.c @@ -36,13 +36,30 @@ struct remote_layer_data *remote_layer_create(const char *layer_home, const char ERROR("Out of memory"); return NULL; } - data->layer_home = util_strdup_s(layer_home); - data->layer_ro = util_strdup_s(layer_ro); + data->layer_home = layer_home; + data->layer_ro = layer_ro; + layer_byid_old = map_new(MAP_STR_BOOL, MAP_DEFAULT_CMP_FUNC, MAP_DEFAULT_FREE_FUNC); + if (layer_byid_old == NULL) { + ERROR("Failed to cerate layer_byid_old"); + goto free_out; + } + layer_byid_new = map_new(MAP_STR_BOOL, MAP_DEFAULT_CMP_FUNC, MAP_DEFAULT_FREE_FUNC); + if (layer_byid_new == NULL) { + ERROR("Failed to cerate layer_byid_new"); + goto free_out; + } return data; -}; + +free_out: + map_free(layer_byid_old); + map_free(layer_byid_new); + free(data); + + return NULL; +} void remote_layer_destroy(struct remote_layer_data *data) { @@ -59,7 +76,7 @@ static bool layer_walk_dir_cb(const char *path_name, const struct dirent *sub_di { bool exist = true; - if (!map_insert(layer_byid_new, util_strdup_s(sub_dir->d_name), (void *)&exist)) { + if (!map_insert(layer_byid_new, (void *)sub_dir->d_name, (void *)&exist)) { ERROR("can't insert remote layer into map"); return false; } @@ -165,7 +182,7 @@ static int remote_layer_add(struct remote_layer_data *data) char **array_deleted = NULL; map_t *tmp_map = NULL; bool exist = true; - int i = 0; + size_t i = 0; if (data == NULL) { return -1; @@ -175,15 +192,19 @@ static int remote_layer_add(struct remote_layer_data *data) array_deleted = remote_deleted_layers(layer_byid_old, layer_byid_new); for (i = 0; i < util_array_len((const char **)array_added); i++) { - if (!remote_overlay_layer_valid(array_added[i]) != 0) { + if (!remote_overlay_layer_valid(array_added[i])) { WARN("remote overlay layer current not valid: %s", array_added[i]); - map_remove(layer_byid_new, (void *)array_added[i]); + if (!map_remove(layer_byid_new, (void *)array_added[i])) { + WARN("layer %s will not be loaded from remote", array_added[i]); + } continue; } if (add_one_remote_layer(data, array_added[i]) != 0) { ERROR("Failed to add remote layer: %s", array_added[i]); - map_remove(layer_byid_new, (void *)array_added[i]); + if (!map_remove(layer_byid_new, (void *)array_added[i])) { + WARN("layer %s will not be loaded from remote", array_added[i]); + } ret = -1; } } @@ -191,7 +212,9 @@ static int remote_layer_add(struct remote_layer_data *data) for (i = 0; i < util_array_len((const char **)array_deleted); i++) { if (remove_one_remote_layer(data, array_deleted[i]) != 0) { ERROR("Failed to delete remote overlay layer: %s", array_deleted[i]); - map_insert(layer_byid_new, array_deleted[i], (void *)&exist); + if (!map_insert(layer_byid_new, array_deleted[i], (void *)&exist)) { + WARN("layer %s will not be removed from local", array_deleted[i]); + } ret = -1; } } diff --git a/src/daemon/modules/image/oci/storage/remote_layer_support/overlay_remote_impl.c b/src/daemon/modules/image/oci/storage/remote_layer_support/overlay_remote_impl.c index de2e583c..30caf175 100644 --- a/src/daemon/modules/image/oci/storage/remote_layer_support/overlay_remote_impl.c +++ b/src/daemon/modules/image/oci/storage/remote_layer_support/overlay_remote_impl.c @@ -42,13 +42,37 @@ struct remote_overlay_data *remote_overlay_create(const char *remote_home, const ERROR("Out of memory"); return NULL; } + data->overlay_home = remote_home; data->overlay_ro = remote_ro; + overlay_byid_old = map_new(MAP_STR_BOOL, MAP_DEFAULT_CMP_FUNC, MAP_DEFAULT_FREE_FUNC); + if (overlay_byid_old == NULL) { + ERROR("Failed to cerate overlay_byid_old"); + goto free_out; + } + overlay_byid_new = map_new(MAP_STR_BOOL, MAP_DEFAULT_CMP_FUNC, MAP_DEFAULT_FREE_FUNC); + if (overlay_byid_new == NULL) { + ERROR("Failed to cerate overlay_byid_new"); + goto free_out; + } + overlay_id_link = map_new(MAP_STR_STR, MAP_DEFAULT_CMP_FUNC, MAP_DEFAULT_FREE_FUNC); + if (overlay_id_link == NULL) { + ERROR("Failed to cerate overlay_id_link"); + goto free_out; + } return data; + +free_out: + map_free(overlay_byid_old); + map_free(overlay_byid_new); + map_free(overlay_id_link); + free(data); + + return NULL; } void remote_overlay_destroy(struct remote_overlay_data *data) @@ -66,7 +90,7 @@ void remote_overlay_destroy(struct remote_overlay_data *data) static bool overlay_walk_dir_cb(const char *path_name, const struct dirent *sub_dir, void *context) { bool exist = true; - if (!map_insert(overlay_byid_new, util_strdup_s(sub_dir->d_name), (void *)&exist)) { + if (!map_insert(overlay_byid_new, (void *)sub_dir->d_name, (void *)&exist)) { ERROR("can't insert remote layer into map"); return false; } @@ -254,7 +278,7 @@ static int add_one_remote_overlay_layer(struct remote_overlay_data *data, const ret = -1; } - if (!map_insert(overlay_id_link, util_strdup_s(overlay_id), (void *)diff_symlink)) { + if (!map_insert(overlay_id_link, (void *)overlay_id, (void *)diff_symlink)) { ERROR("can't insert remote layer into map"); ret = -1; } @@ -275,7 +299,7 @@ static int remote_image_add(struct remote_overlay_data *data) char **array_deleted = NULL; map_t *tmp_map = NULL; bool exist = true; - int i = 0; + size_t i = 0; if (data == NULL) { return -1; @@ -287,7 +311,9 @@ static int remote_image_add(struct remote_overlay_data *data) for (i = 0; i < util_array_len((const char **)array_added); i++) { if (add_one_remote_overlay_layer(data, array_added[i]) != 0) { ERROR("Failed to add remote overlay layer: %s", array_added[i]); - map_remove(overlay_byid_new, (void *)array_added[i]); + if (!map_remove(overlay_byid_new, (void *)array_added[i])) { + WARN("overlay layer %s will not be loaded from remote", array_added[i]); + } ret = -1; } } @@ -295,7 +321,9 @@ static int remote_image_add(struct remote_overlay_data *data) for (i = 0; i < util_array_len((const char **)array_deleted); i++) { if (remove_one_remote_overlay_layer(data, array_deleted[i]) != 0) { ERROR("Failed to delete remote overlay layer: %s", array_deleted[i]); - map_insert(overlay_byid_new, array_deleted[i], (void *)&exist); + if (!map_insert(overlay_byid_new, array_deleted[i], (void *)&exist)) { + WARN("overlay layer %s will be deleted from local", array_deleted[i]); + } ret = -1; } } diff --git a/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c b/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c index 7d457755..748298cb 100644 --- a/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c +++ b/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c @@ -65,7 +65,10 @@ static void *remote_refresh_ro_symbol_link(void *arg) util_usleep_nointerupt(5 * 1000 * 1000); DEBUG("remote refresh start\n"); - remote_refresh_lock(supporters.remote_lock, true); + if (!remote_refresh_lock(supporters.remote_lock, true)) { + WARN("Failed to lock remote store failed, try to lock after 5 seconds"); + continue; + } remote_overlay_refresh(refresh_supporters->overlay_data); remote_layer_refresh(refresh_supporters->layer_data); remote_image_refresh(refresh_supporters->image_data); @@ -127,18 +130,30 @@ static char **map_diff(const map_t *map_a, const map_t *map_b) char **array = NULL; map_itor *itor = map_itor_new(map_a); bool *found = NULL; + int ret = 0; // iter new_map, every item not in old, append them to new_layers for (; map_itor_valid(itor); map_itor_next(itor)) { char *id = map_itor_key(itor); found = map_search(map_b, id); if (found == NULL) { - util_array_append(&array, util_strdup_s(id)); + ret = util_array_append(&array, id); + if (ret != 0) { + ERROR("Failed to add diff item %s to array", id); + break; + } } } map_itor_free(itor); + // if array is null then return directly + // if array is not null, free array and return NULL + if (ret != 0 && array != NULL) { + util_free_array(array); + array = NULL; + } + return array; } diff --git a/src/daemon/modules/image/oci/storage/remote_layer_support/ro_symlink_maintain.c b/src/daemon/modules/image/oci/storage/remote_layer_support/ro_symlink_maintain.c index a3aa3aa4..0e2b671b 100644 --- a/src/daemon/modules/image/oci/storage/remote_layer_support/ro_symlink_maintain.c +++ b/src/daemon/modules/image/oci/storage/remote_layer_support/ro_symlink_maintain.c @@ -27,8 +27,6 @@ #include "utils.h" #include "utils_file.h" -#define REMOTE_RO_LAYER_DIR "RO" - // overlay-layers and overlay-layers/RO static char *image_home; diff --git a/src/daemon/modules/image/oci/storage/storage.c b/src/daemon/modules/image/oci/storage/storage.c index 836ccf4d..07c3830b 100644 --- a/src/daemon/modules/image/oci/storage/storage.c +++ b/src/daemon/modules/image/oci/storage/storage.c @@ -1876,6 +1876,8 @@ int storage_module_init(struct storage_module_init_options *opts) #ifdef ENABLE_REMOTE_LAYER_STORE if (opts->enable_remote_layer && remote_start_refresh_thread(opts->remote_lock) != 0) { ERROR("Failed to start remote refresh thread"); + ret = -1; + goto out; } #endif -- 2.25.1