From 92cbc7b18ca1d6847947a7359a47ccf62740ac0e Mon Sep 17 00:00:00 2001 From: zhongtao Date: Tue, 11 Jun 2024 20:02:15 +0800 Subject: [PATCH] bugfix for env/cri/fork Signed-off-by: zhongtao (cherry picked from commit bbcaf5a7227e418497e56c9f3495457e8cf7c652) --- ...Use-user-defined-shm-for-CRI-request.patch | 145 ++++++++++++++++ ...-in-set_connected_container_shm_path.patch | 51 ++++++ ...-status-codes-of-cri-in-case-of-erro.patch | 164 ++++++++++++++++++ ...re-created-pipe-was-not-closed-when-.patch | 27 +++ ...-add-debug-msg-info-in-image_load.sh.patch | 26 +++ ...llow-env-variable-has-an-empty-value.patch | 69 ++++++++ ...ugfix-for-hostname-env-set-only-once.patch | 148 ++++++++++++++++ 0212-fix-bug-for-invalid-env-write.patch | 51 ++++++ 0213-change-fork-process-exit-mode.patch | 64 +++++++ iSulad.spec | 17 +- 10 files changed, 761 insertions(+), 1 deletion(-) create mode 100644 0205-Use-user-defined-shm-for-CRI-request.patch create mode 100644 0206-Fix-memory-leak-in-set_connected_container_shm_path.patch create mode 100644 0207-modify-some-grpc-status-codes-of-cri-in-case-of-erro.patch create mode 100644 0208-bugfix-for-the-pre-created-pipe-was-not-closed-when-.patch create mode 100644 0209-add-debug-msg-info-in-image_load.sh.patch create mode 100644 0210-allow-env-variable-has-an-empty-value.patch create mode 100644 0211-bugfix-for-hostname-env-set-only-once.patch create mode 100644 0212-fix-bug-for-invalid-env-write.patch create mode 100644 0213-change-fork-process-exit-mode.patch diff --git a/0205-Use-user-defined-shm-for-CRI-request.patch b/0205-Use-user-defined-shm-for-CRI-request.patch new file mode 100644 index 0000000..eaef6d1 --- /dev/null +++ b/0205-Use-user-defined-shm-for-CRI-request.patch @@ -0,0 +1,145 @@ +From bd8d77e88717394f2904809a8868c1d6228dccae Mon Sep 17 00:00:00 2001 +From: xuxuepeng +Date: Mon, 8 Apr 2024 20:53:57 +0800 +Subject: [PATCH 205/213] Use user defined shm for CRI request + +Signed-off-by: xuxuepeng +--- + src/daemon/modules/spec/specs_mount.c | 104 +++++++++++++++++--------- + 1 file changed, 69 insertions(+), 35 deletions(-) + +diff --git a/src/daemon/modules/spec/specs_mount.c b/src/daemon/modules/spec/specs_mount.c +index 8bff6cda..4b56200e 100644 +--- a/src/daemon/modules/spec/specs_mount.c ++++ b/src/daemon/modules/spec/specs_mount.c +@@ -2840,58 +2840,92 @@ out_free: + return ret; + } + +-#define SHM_MOUNT_POINT "/dev/shm" +-static int set_shm_path(host_config *host_spec, container_config_v2_common_config *v2_spec) ++static inline int set_sharable_ipc_mode(host_config *host_spec, container_config_v2_common_config *v2_spec) ++{ ++ free(v2_spec->shm_path); ++ v2_spec->shm_path = get_prepare_share_shm_path(host_spec->runtime, v2_spec->id); ++ if (v2_spec->shm_path == NULL) { ++ ERROR("Failed to get prepare share shm path"); ++ return -1; ++ } ++ ++ return 0; ++} ++ ++static inline int set_connected_container_shm_path(host_config *host_spec, container_config_v2_common_config *v2_spec) + { +- int ret = 0; + container_t *cont = NULL; + char *tmp_cid = NULL; + char *right_path = NULL; + ++ tmp_cid = namespace_get_connected_container(host_spec->ipc_mode); ++ cont = containers_store_get(tmp_cid); ++ if (cont == NULL) { ++ ERROR("Invalid share path: %s", host_spec->ipc_mode); ++ return -1; ++ } ++ right_path = util_strdup_s(cont->common_config->shm_path); ++ container_unref(cont); ++ ++ free(v2_spec->shm_path); ++ v2_spec->shm_path = right_path; ++ ++ return 0; ++} ++ ++#define SHM_MOUNT_POINT "/dev/shm" ++static inline int set_host_ipc_shm_path(container_config_v2_common_config *v2_spec) ++{ ++ if (!util_file_exists(SHM_MOUNT_POINT)) { ++ ERROR("/dev/shm is not mounted, but must be for --ipc=host"); ++ return -1; ++ } ++ free(v2_spec->shm_path); ++ v2_spec->shm_path = util_strdup_s(SHM_MOUNT_POINT); ++ return 0; ++} ++ ++/** ++ * There are 4 cases for setting shm path: ++ * 1. The user defined /dev/shm in mounts, which takes the first priority ++ * 2. If sharable is set in ipc mode (or by default ipc_mode is null), the container provides shm path, ++ * in the case of sandbox API is used, the sandbox module has already provided shm path ++ * 3. Use the connected container's shm path if ipc_mode is set to container:, ++ * if connected containerd is a sandbox, use the sandbox's shm path ++ * 4. Use /dev/shm if ipc_mode is set to host ++ */ ++static int set_shm_path(host_config *host_spec, container_config_v2_common_config *v2_spec) ++{ + // ignore shm of system container + if (host_spec->system_container) { + return 0; + } +- // setup shareable dirs +- if (is_shareable_ipc(host_spec->ipc_mode)) { +- // has mount for /dev/shm +- if (has_mount_shm(host_spec, v2_spec)) { +- return 0; +- } +- +- v2_spec->shm_path = get_prepare_share_shm_path(host_spec->runtime, v2_spec->id); +- if (v2_spec->shm_path == NULL) { +- ERROR("Failed to get prepare share shm path"); +- return -1; +- } + ++ // case 1: Defined in mounts already ++ if (has_mount_shm(host_spec, v2_spec)) { + return 0; + } + ++ // case 2: Container has its own IPC namespace ++ if (is_shareable_ipc(host_spec->ipc_mode)) { ++ return set_sharable_ipc_mode(host_spec, v2_spec); ++ } ++ ++ // case 3: Connected container + if (namespace_is_container(host_spec->ipc_mode)) { +- tmp_cid = namespace_get_connected_container(host_spec->ipc_mode); +- cont = containers_store_get(tmp_cid); +- if (cont == NULL) { +- ERROR("Invalid share path: %s", host_spec->ipc_mode); +- ret = -1; +- goto out; +- } +- right_path = util_strdup_s(cont->common_config->shm_path); +- container_unref(cont); +- } else if (namespace_is_host(host_spec->ipc_mode)) { +- if (!util_file_exists(SHM_MOUNT_POINT)) { +- ERROR("/dev/shm is not mounted, but must be for --ipc=host"); +- ret = -1; +- goto out; +- } +- right_path = util_strdup_s(SHM_MOUNT_POINT); ++ return set_connected_container_shm_path(host_spec, v2_spec); + } + ++ // case 4: Host IPC namespace ++ if (namespace_is_host(host_spec->ipc_mode)) { ++ return set_host_ipc_shm_path(v2_spec); ++ } ++ ++ // Otherwise, the case is unknown, nothing is set + free(v2_spec->shm_path); +- v2_spec->shm_path = right_path; +-out: +- free(tmp_cid); +- return ret; ++ v2_spec->shm_path = NULL; ++ ++ return 0; + } + + int destination_compare(const void *p1, const void *p2) +-- +2.25.1 + diff --git a/0206-Fix-memory-leak-in-set_connected_container_shm_path.patch b/0206-Fix-memory-leak-in-set_connected_container_shm_path.patch new file mode 100644 index 0000000..1d37bf4 --- /dev/null +++ b/0206-Fix-memory-leak-in-set_connected_container_shm_path.patch @@ -0,0 +1,51 @@ +From 05cb3b55d4b7fcde0e8f5eb7a2fb72983d127c64 Mon Sep 17 00:00:00 2001 +From: xuxuepeng +Date: Tue, 9 Apr 2024 06:35:03 +0800 +Subject: [PATCH 206/213] Fix memory leak in set_connected_container_shm_path + +Signed-off-by: xuxuepeng +--- + src/daemon/modules/spec/specs_mount.c | 18 ++++++++++-------- + 1 file changed, 10 insertions(+), 8 deletions(-) + +diff --git a/src/daemon/modules/spec/specs_mount.c b/src/daemon/modules/spec/specs_mount.c +index 4b56200e..6adb6a5b 100644 +--- a/src/daemon/modules/spec/specs_mount.c ++++ b/src/daemon/modules/spec/specs_mount.c +@@ -2854,23 +2854,25 @@ static inline int set_sharable_ipc_mode(host_config *host_spec, container_config + + static inline int set_connected_container_shm_path(host_config *host_spec, container_config_v2_common_config *v2_spec) + { ++ int ret = 0; + container_t *cont = NULL; + char *tmp_cid = NULL; +- char *right_path = NULL; + + tmp_cid = namespace_get_connected_container(host_spec->ipc_mode); + cont = containers_store_get(tmp_cid); + if (cont == NULL) { + ERROR("Invalid share path: %s", host_spec->ipc_mode); +- return -1; +- } +- right_path = util_strdup_s(cont->common_config->shm_path); +- container_unref(cont); ++ ret = -1; ++ } else { ++ char *right_path = util_strdup_s(cont->common_config->shm_path); ++ container_unref(cont); + +- free(v2_spec->shm_path); +- v2_spec->shm_path = right_path; ++ free(v2_spec->shm_path); ++ v2_spec->shm_path = right_path; ++ } + +- return 0; ++ free(tmp_cid); ++ return ret; + } + + #define SHM_MOUNT_POINT "/dev/shm" +-- +2.25.1 + diff --git a/0207-modify-some-grpc-status-codes-of-cri-in-case-of-erro.patch b/0207-modify-some-grpc-status-codes-of-cri-in-case-of-erro.patch new file mode 100644 index 0000000..75f26bb --- /dev/null +++ b/0207-modify-some-grpc-status-codes-of-cri-in-case-of-erro.patch @@ -0,0 +1,164 @@ +From 5795e14d45ac750dfddf25299f6b7dd2b4deebae Mon Sep 17 00:00:00 2001 +From: jikai +Date: Mon, 8 Apr 2024 16:55:29 +0800 +Subject: [PATCH 207/213] modify some grpc status codes of cri in case of error + +Signed-off-by: jikai +--- + .../connect/grpc/runtime_runtime_service.cc | 41 +++++++++++++------ + .../connect/grpc/runtime_runtime_service.h | 3 ++ + 2 files changed, 32 insertions(+), 12 deletions(-) + +diff --git a/src/daemon/entry/connect/grpc/runtime_runtime_service.cc b/src/daemon/entry/connect/grpc/runtime_runtime_service.cc +index 5b4adc3f..b0acf4a3 100644 +--- a/src/daemon/entry/connect/grpc/runtime_runtime_service.cc ++++ b/src/daemon/entry/connect/grpc/runtime_runtime_service.cc +@@ -28,6 +28,23 @@ + + using namespace CRI; + ++grpc::Status RuntimeRuntimeServiceImpl::ToGRPCStatus(Errors &error) ++{ ++ if (error.Empty()) { ++ return grpc::Status::OK; ++ } ++ if (error.GetMessage().find("Failed to find") != std::string::npos) { ++ return grpc::Status(grpc::StatusCode::NOT_FOUND, error.GetMessage()); ++ } ++ ++ // Attach exceeded timeout for lxc and Exec container error;exec timeout for runc ++ if (error.GetMessage().find("Attach exceeded timeout") != std::string::npos ++ || error.GetMessage().find("Exec container error;exec timeout") != std::string::npos) { ++ return grpc::Status(grpc::StatusCode::DEADLINE_EXCEEDED, error.GetMessage()); ++ } ++ return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++} ++ + void RuntimeRuntimeServiceImpl::Init(Network::NetworkPluginConf mConf, isulad_daemon_configs *config, Errors &err) + { + std::string podSandboxImage; +@@ -124,7 +141,7 @@ grpc::Status RuntimeRuntimeServiceImpl::CreateContainer(grpc::ServerContext *con + rService->CreateContainer(request->pod_sandbox_id(), request->config(), request->sandbox_config(), error); + if (!error.Empty() || responseID.empty()) { + ERROR("Object: CRI, Type: Failed to create container"); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + reply->set_container_id(responseID); + +@@ -149,7 +166,7 @@ grpc::Status RuntimeRuntimeServiceImpl::StartContainer(grpc::ServerContext *cont + rService->StartContainer(request->container_id(), error); + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to start container %s", request->container_id().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + EVENT("Event: {Object: CRI, Type: Started Container: %s}", request->container_id().c_str()); +@@ -173,7 +190,7 @@ grpc::Status RuntimeRuntimeServiceImpl::StopContainer(grpc::ServerContext *conte + rService->StopContainer(request->container_id(), (int64_t)request->timeout(), error); + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to stop container %s", request->container_id().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + EVENT("Event: {Object: CRI, Type: Stopped Container: %s}", request->container_id().c_str()); +@@ -197,7 +214,7 @@ grpc::Status RuntimeRuntimeServiceImpl::RemoveContainer(grpc::ServerContext *con + rService->RemoveContainer(request->container_id(), error); + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to remove container %s", request->container_id().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + EVENT("Event: {Object: CRI, Type: Removed Container: %s}", request->container_id().c_str()); +@@ -316,7 +333,7 @@ grpc::Status RuntimeRuntimeServiceImpl::ContainerStatus(grpc::ServerContext *con + rService->ContainerStatus(request->container_id(), error); + if (!error.Empty() || !contStatus) { + ERROR("Object: CRI, Type: Failed to get container status %s", request->container_id().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + *(reply->mutable_status()) = *contStatus; + +@@ -341,7 +358,7 @@ grpc::Status RuntimeRuntimeServiceImpl::ExecSync(grpc::ServerContext *context, + rService->ExecSync(request->container_id(), request->cmd(), request->timeout(), reply, error); + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to sync exec container: %s", request->container_id().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + WARN("Event: {Object: CRI, Type: sync execed Container: %s}", request->container_id().c_str()); +@@ -395,7 +412,7 @@ grpc::Status RuntimeRuntimeServiceImpl::StopPodSandbox(grpc::ServerContext *cont + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to stop pod:%s due to %s", request->pod_sandbox_id().c_str(), + error.GetMessage().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + EVENT("Event: {Object: CRI, Type: Stopped Pod: %s}", request->pod_sandbox_id().c_str()); +@@ -420,7 +437,7 @@ grpc::Status RuntimeRuntimeServiceImpl::RemovePodSandbox(grpc::ServerContext *co + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to remove pod:%s due to %s", request->pod_sandbox_id().c_str(), + error.GetMessage().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + EVENT("Event: {Object: CRI, Type: Removed Pod: %s}", request->pod_sandbox_id().c_str()); +@@ -446,7 +463,7 @@ grpc::Status RuntimeRuntimeServiceImpl::PodSandboxStatus(grpc::ServerContext *co + if (!error.Empty() || !podStatus) { + ERROR("Object: CRI, Type: Failed to status pod:%s due to %s", request->pod_sandbox_id().c_str(), + error.GetMessage().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + *(reply->mutable_status()) = *podStatus; + +@@ -506,7 +523,7 @@ RuntimeRuntimeServiceImpl::UpdateContainerResources(grpc::ServerContext *context + if (error.NotEmpty()) { + ERROR("Object: CRI, Type: Failed to update container:%s due to %s", request->container_id().c_str(), + error.GetMessage().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + WARN("Event: {Object: CRI, Type: Updated container resources: %s}", request->container_id().c_str()); +@@ -531,7 +548,7 @@ grpc::Status RuntimeRuntimeServiceImpl::Exec(grpc::ServerContext *context, + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to exec container:%s due to %s", request->container_id().c_str(), + error.GetMessage().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + EVENT("Event: {Object: CRI, Type: execed Container: %s}", request->container_id().c_str()); +@@ -556,7 +573,7 @@ grpc::Status RuntimeRuntimeServiceImpl::Attach(grpc::ServerContext *context, + if (!error.Empty()) { + ERROR("Object: CRI, Type: Failed to attach container:%s due to %s", request->container_id().c_str(), + error.GetMessage().c_str()); +- return grpc::Status(grpc::StatusCode::UNKNOWN, error.GetMessage()); ++ return ToGRPCStatus(error); + } + + EVENT("Event: {Object: CRI, Type: attched Container: %s}", request->container_id().c_str()); +diff --git a/src/daemon/entry/connect/grpc/runtime_runtime_service.h b/src/daemon/entry/connect/grpc/runtime_runtime_service.h +index 94543793..8b5f96ad 100644 +--- a/src/daemon/entry/connect/grpc/runtime_runtime_service.h ++++ b/src/daemon/entry/connect/grpc/runtime_runtime_service.h +@@ -107,6 +107,9 @@ public: + runtime::v1alpha2::StatusResponse *reply) override; + + private: ++ ++ grpc::Status ToGRPCStatus(Errors &err); ++ + std::unique_ptr rService; + }; + +-- +2.25.1 + diff --git a/0208-bugfix-for-the-pre-created-pipe-was-not-closed-when-.patch b/0208-bugfix-for-the-pre-created-pipe-was-not-closed-when-.patch new file mode 100644 index 0000000..27fe499 --- /dev/null +++ b/0208-bugfix-for-the-pre-created-pipe-was-not-closed-when-.patch @@ -0,0 +1,27 @@ +From 2acc11797139e6524c1994196e7cd99d35b1a8b3 Mon Sep 17 00:00:00 2001 +From: zhongtao +Date: Thu, 11 Apr 2024 15:48:43 +0800 +Subject: [PATCH 208/213] bugfix for the pre-created pipe was not closed when + the pipe creation failed + +Signed-off-by: zhongtao +--- + src/daemon/modules/runtime/isula/isula_rt_ops.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/daemon/modules/runtime/isula/isula_rt_ops.c b/src/daemon/modules/runtime/isula/isula_rt_ops.c +index 6a5e0125..949bce7a 100644 +--- a/src/daemon/modules/runtime/isula/isula_rt_ops.c ++++ b/src/daemon/modules/runtime/isula/isula_rt_ops.c +@@ -792,6 +792,8 @@ static int shim_create(bool fg, const char *id, const char *workdir, const char + + if (pipe2(shim_stdout_pipe, O_CLOEXEC) != 0) { + ERROR("Failed to create pipe for shim stdout"); ++ close(shim_stderr_pipe[0]); ++ close(shim_stderr_pipe[1]); + return -1; + } + +-- +2.25.1 + diff --git a/0209-add-debug-msg-info-in-image_load.sh.patch b/0209-add-debug-msg-info-in-image_load.sh.patch new file mode 100644 index 0000000..4fc039b --- /dev/null +++ b/0209-add-debug-msg-info-in-image_load.sh.patch @@ -0,0 +1,26 @@ +From da08a25e0aa82424a6da679a7b0ec22ffb43cdf4 Mon Sep 17 00:00:00 2001 +From: zhongtao +Date: Fri, 12 Apr 2024 15:23:07 +0800 +Subject: [PATCH 209/213] add debug msg info in image_load.sh + +Signed-off-by: zhongtao +--- + CI/test_cases/image_cases/image_load.sh | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/CI/test_cases/image_cases/image_load.sh b/CI/test_cases/image_cases/image_load.sh +index a2cada5f..d50b3203 100755 +--- a/CI/test_cases/image_cases/image_load.sh ++++ b/CI/test_cases/image_cases/image_load.sh +@@ -103,6 +103,8 @@ function test_concurrent_load() + [[ $? -ne 0 ]] && msg_err "${FUNCNAME[0]}:${LINENO} - fail to do isulad load $i" && ((ret++)) + done + ++ tail -n 50 /var/lib/isulad/isulad.log ++ + ubuntu_id=`isula inspect -f '{{.image.id}}' ubuntu` + [[ $? -ne 0 ]] && msg_err "${FUNCNAME[0]}:${LINENO} - fail to inspect image: ubuntu" && ((ret++)) + +-- +2.25.1 + diff --git a/0210-allow-env-variable-has-an-empty-value.patch b/0210-allow-env-variable-has-an-empty-value.patch new file mode 100644 index 0000000..05e4098 --- /dev/null +++ b/0210-allow-env-variable-has-an-empty-value.patch @@ -0,0 +1,69 @@ +From 5446c18fe129fa81a9a05c294fbc4e5d7c1c71a5 Mon Sep 17 00:00:00 2001 +From: jikai +Date: Sat, 27 Apr 2024 14:32:19 +0800 +Subject: [PATCH 210/213] allow env variable has an empty value + +Signed-off-by: jikai +--- + src/daemon/modules/spec/specs_extend.c | 10 ++++++---- + src/utils/cutils/utils_string.c | 3 +++ + 2 files changed, 9 insertions(+), 4 deletions(-) + +diff --git a/src/daemon/modules/spec/specs_extend.c b/src/daemon/modules/spec/specs_extend.c +index 5ede7936..a9e8142e 100644 +--- a/src/daemon/modules/spec/specs_extend.c ++++ b/src/daemon/modules/spec/specs_extend.c +@@ -196,6 +196,7 @@ static int generate_env_map_from_file(FILE *fp, json_map_string_string *env_map) + char *pline = NULL; + size_t length = 0; + char *saveptr = NULL; ++ char empty_str[1] = {'\0'}; + + while (getline(&pline, &length, fp) != -1) { + util_trim_newline(pline); +@@ -205,7 +206,9 @@ static int generate_env_map_from_file(FILE *fp, json_map_string_string *env_map) + } + key = strtok_r(pline, "=", &saveptr); + value = strtok_r(NULL, "=", &saveptr); +- if (key != NULL && value != NULL) { ++ // value of an env varible is allowed to be empty ++ value = value ? value : empty_str; ++ if (key != NULL) { + key = util_trim_space(key); + value = util_trim_space(value); + if ((size_t)(MAX_BUFFER_SIZE - 1) - strlen(key) < strlen(value)) { +@@ -292,15 +295,14 @@ static int check_env_need_append(const oci_runtime_spec *oci_spec, const char *e + { + size_t i = 0; + char *key = NULL; +- char *value = NULL; + char *saveptr = NULL; + + for (i = 0; i < oci_spec->process->env_len; i++) { + char *tmp_env = NULL; + tmp_env = util_strdup_s(oci_spec->process->env[i]); + key = strtok_r(tmp_env, "=", &saveptr); +- value = strtok_r(NULL, "=", &saveptr); +- if (key == NULL || value == NULL) { ++ // value of an env varible is allowed to be empty ++ if (key == NULL) { + ERROR("Bad env format"); + free(tmp_env); + tmp_env = NULL; +diff --git a/src/utils/cutils/utils_string.c b/src/utils/cutils/utils_string.c +index 64afb570..11a65f19 100644 +--- a/src/utils/cutils/utils_string.c ++++ b/src/utils/cutils/utils_string.c +@@ -534,6 +534,9 @@ static char *util_left_trim_space(char *str) + { + char *begin = str; + char *tmp = str; ++ if (strlen(str) == 0) { ++ return str; ++ } + while (isspace(*begin)) { + begin++; + } +-- +2.25.1 + diff --git a/0211-bugfix-for-hostname-env-set-only-once.patch b/0211-bugfix-for-hostname-env-set-only-once.patch new file mode 100644 index 0000000..12db237 --- /dev/null +++ b/0211-bugfix-for-hostname-env-set-only-once.patch @@ -0,0 +1,148 @@ +From 459e811de123e54efb3d98601c955419580277da Mon Sep 17 00:00:00 2001 +From: jikai +Date: Mon, 29 Apr 2024 09:14:53 +0800 +Subject: [PATCH 211/213] bugfix for hostname env: set only once + +Signed-off-by: jikai +--- + src/daemon/modules/spec/specs.c | 11 +++++- + src/daemon/modules/spec/specs_extend.c | 52 +++++++++++++++++--------- + src/daemon/modules/spec/specs_extend.h | 2 + + 3 files changed, 46 insertions(+), 19 deletions(-) + +diff --git a/src/daemon/modules/spec/specs.c b/src/daemon/modules/spec/specs.c +index d2088a8e..bbe73bf5 100644 +--- a/src/daemon/modules/spec/specs.c ++++ b/src/daemon/modules/spec/specs.c +@@ -1793,14 +1793,21 @@ static int merge_process_conf(oci_runtime_spec *oci_spec, const host_config *hos + goto out; + } + +- /* environment variables */ ++ /* 1. merge env from container_spec: --env or --env-file */ + ret = merge_env(oci_spec, (const char **)container_spec->env, container_spec->env_len); + if (ret != 0) { + ERROR("Failed to merge environment variables"); + goto out; + } + +- /* env target file */ ++ /* 2. merge default env hostname, only if hostname not set before */ ++ ret = merge_hostname_env(oci_spec); ++ if (ret != 0) { ++ ERROR("Failed to merge hostname env"); ++ goto out; ++ } ++ ++ /* 3. persist env from --env-target-file, only if the env not set before, system container only */ + ret = merge_env_target_file(oci_spec, host_spec->env_target_file); + if (ret != 0) { + ERROR("Failed to merge env target file"); +diff --git a/src/daemon/modules/spec/specs_extend.c b/src/daemon/modules/spec/specs_extend.c +index a9e8142e..b4b8af5a 100644 +--- a/src/daemon/modules/spec/specs_extend.c ++++ b/src/daemon/modules/spec/specs_extend.c +@@ -421,34 +421,23 @@ out: + int merge_env(oci_runtime_spec *oci_spec, const char **env, size_t env_len) + { + int ret = 0; +- int nret = 0; + size_t new_size = 0; + size_t old_size = 0; + size_t i; + char **temp = NULL; +- // 10 is lenght of "HOSTNAME=" and '\0' +- char host_name_env[MAX_HOST_NAME_LEN + 10] = { 0 }; +- +- nret = snprintf(host_name_env, sizeof(host_name_env), "HOSTNAME=%s", oci_spec->hostname); +- if (nret < 0 || (size_t)nret >= sizeof(host_name_env)) { +- ret = -1; +- ERROR("Sprint failed"); +- goto out; +- } + + ret = make_sure_oci_spec_process(oci_spec); + if (ret < 0) { + goto out; + } + +- if (env_len > LIST_ENV_SIZE_MAX - oci_spec->process->env_len - 1) { ++ if (env_len > LIST_ENV_SIZE_MAX - oci_spec->process->env_len) { + ERROR("The length of envionment variables is too long, the limit is %lld", LIST_ENV_SIZE_MAX); + isulad_set_error_message("The length of envionment variables is too long, the limit is %d", LIST_ENV_SIZE_MAX); + ret = -1; + goto out; + } +- // add 1 for hostname env +- new_size = (oci_spec->process->env_len + env_len + 1) * sizeof(char *); ++ new_size = (oci_spec->process->env_len + env_len) * sizeof(char *); + old_size = oci_spec->process->env_len * sizeof(char *); + ret = util_mem_realloc((void **)&temp, new_size, oci_spec->process->env, old_size); + if (ret != 0) { +@@ -459,10 +448,6 @@ int merge_env(oci_runtime_spec *oci_spec, const char **env, size_t env_len) + + oci_spec->process->env = temp; + +- // append hostname env into default oci spec env list +- oci_spec->process->env[oci_spec->process->env_len] = util_strdup_s(host_name_env); +- oci_spec->process->env_len++; +- + for (i = 0; i < env_len && env != NULL; i++) { + oci_spec->process->env[oci_spec->process->env_len] = util_strdup_s(env[i]); + oci_spec->process->env_len++; +@@ -471,6 +456,39 @@ out: + return ret; + } + ++int merge_hostname_env(oci_runtime_spec *oci_spec) ++{ ++ int nret = 0; ++ bool is_append = true; ++ // 10 is lenght of "HOSTNAME=" and '\0' ++ char host_name_env[MAX_HOST_NAME_LEN + 10] = { 0 }; ++ const char *envs[1] = {host_name_env}; ++ ++ if (make_sure_oci_spec_process(oci_spec) < 0) { ++ return -1; ++ } ++ ++ if (check_env_need_append(oci_spec, "HOSTNAME", &is_append) < 0) { ++ return -1; ++ } ++ ++ if (!is_append) { ++ return 0; ++ } ++ ++ nret = snprintf(host_name_env, sizeof(host_name_env), "HOSTNAME=%s", oci_spec->hostname); ++ if (nret < 0 || (size_t)nret >= sizeof(host_name_env)) { ++ ERROR("Sprint failed"); ++ return -1; ++ } ++ ++ if (merge_env(oci_spec, (const char **)envs, 1) < 0) { ++ return -1; ++ } ++ ++ return 0; ++} ++ + char *oci_container_get_env(const oci_runtime_spec *oci_spec, const char *key) + { + const defs_process *op = NULL; +diff --git a/src/daemon/modules/spec/specs_extend.h b/src/daemon/modules/spec/specs_extend.h +index d70f5bec..15ec6b2f 100644 +--- a/src/daemon/modules/spec/specs_extend.h ++++ b/src/daemon/modules/spec/specs_extend.h +@@ -50,6 +50,8 @@ int make_userns_remap(oci_runtime_spec *container, const char *user_remap); + + int merge_env(oci_runtime_spec *oci_spec, const char **env, size_t env_len); + ++int merge_hostname_env(oci_runtime_spec *oci_spec); ++ + int merge_env_target_file(oci_runtime_spec *oci_spec, const char *env_target_file); + + char *oci_container_get_env(const oci_runtime_spec *oci_spec, const char *key); +-- +2.25.1 + diff --git a/0212-fix-bug-for-invalid-env-write.patch b/0212-fix-bug-for-invalid-env-write.patch new file mode 100644 index 0000000..d14d299 --- /dev/null +++ b/0212-fix-bug-for-invalid-env-write.patch @@ -0,0 +1,51 @@ +From 33bae817df87021795bc3aa7ad4aa3a30a164999 Mon Sep 17 00:00:00 2001 +From: jikai +Date: Sat, 11 May 2024 16:05:27 +0800 +Subject: [PATCH 212/213] fix bug for invalid env write + +Signed-off-by: jikai +--- + src/daemon/modules/spec/specs_extend.c | 13 ++++++++----- + 1 file changed, 8 insertions(+), 5 deletions(-) + +diff --git a/src/daemon/modules/spec/specs_extend.c b/src/daemon/modules/spec/specs_extend.c +index b4b8af5a..e385ff4b 100644 +--- a/src/daemon/modules/spec/specs_extend.c ++++ b/src/daemon/modules/spec/specs_extend.c +@@ -195,17 +195,18 @@ static int generate_env_map_from_file(FILE *fp, json_map_string_string *env_map) + char *value = NULL; + char *pline = NULL; + size_t length = 0; +- char *saveptr = NULL; + char empty_str[1] = {'\0'}; + + while (getline(&pline, &length, fp) != -1) { + util_trim_newline(pline); + pline = util_trim_space(pline); +- if (pline == NULL || pline[0] == '#') { ++ // if pline is invalid as =value, strtok_r will return value as key ++ // check if pline[0] == '=' to avoid this case ++ if (pline == NULL || pline[0] == '#' || pline[0] == '=') { + continue; + } +- key = strtok_r(pline, "=", &saveptr); +- value = strtok_r(NULL, "=", &saveptr); ++ // in case of key=value=value, value should be value=value ++ key = strtok_r(pline, "=", &value); + // value of an env varible is allowed to be empty + value = value ? value : empty_str; + if (key != NULL) { +@@ -302,7 +303,9 @@ static int check_env_need_append(const oci_runtime_spec *oci_spec, const char *e + tmp_env = util_strdup_s(oci_spec->process->env[i]); + key = strtok_r(tmp_env, "=", &saveptr); + // value of an env varible is allowed to be empty +- if (key == NULL) { ++ // if tmp_env is invalid as =value, strtok_r will return value as key ++ // check if tmp_env[0] == '=' to avoid this case ++ if (key == NULL || tmp_env[0] == '=') { + ERROR("Bad env format"); + free(tmp_env); + tmp_env = NULL; +-- +2.25.1 + diff --git a/0213-change-fork-process-exit-mode.patch b/0213-change-fork-process-exit-mode.patch new file mode 100644 index 0000000..75834d0 --- /dev/null +++ b/0213-change-fork-process-exit-mode.patch @@ -0,0 +1,64 @@ +From 723659935f6d1fcee936409dfd09557a72e52af0 Mon Sep 17 00:00:00 2001 +From: zhongtao +Date: Thu, 23 May 2024 01:09:41 +1400 +Subject: [PATCH 213/213] change fork process exit mode + +--- + src/utils/tar/util_archive.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) + +diff --git a/src/utils/tar/util_archive.c b/src/utils/tar/util_archive.c +index 31dea193..89c33f09 100644 +--- a/src/utils/tar/util_archive.c ++++ b/src/utils/tar/util_archive.c +@@ -894,9 +894,9 @@ int archive_unpack(const struct io_read_wrapper *content, const char *dstdir, co + + child_out: + if (ret != 0) { +- exit(EXIT_FAILURE); ++ _exit(EXIT_FAILURE); + } +- exit(EXIT_SUCCESS); ++ _exit(EXIT_SUCCESS); + } + close(pipe_stderr[1]); + pipe_stderr[1] = -1; +@@ -1339,9 +1339,9 @@ int archive_chroot_tar(const char *path, const char *file, const char *root_dir, + child_out: + + if (ret != 0) { +- exit(EXIT_FAILURE); ++ _exit(EXIT_FAILURE); + } else { +- exit(EXIT_SUCCESS); ++ _exit(EXIT_SUCCESS); + } + } + close(pipe_for_read[1]); +@@ -1574,9 +1574,9 @@ int archive_chroot_untar_stream(const struct io_read_wrapper *context, const cha + + child_out: + if (ret != 0) { +- exit(EXIT_FAILURE); ++ _exit(EXIT_FAILURE); + } +- exit(EXIT_SUCCESS); ++ _exit(EXIT_SUCCESS); + } + + close(pipe_stderr[1]); +@@ -1724,9 +1724,9 @@ child_out: + free(tar_base_name); + + if (ret != 0) { +- exit(EXIT_FAILURE); ++ _exit(EXIT_FAILURE); + } else { +- exit(EXIT_SUCCESS); ++ _exit(EXIT_SUCCESS); + } + } + +-- +2.25.1 + diff --git a/iSulad.spec b/iSulad.spec index 5e7472b..d08c5b3 100644 --- a/iSulad.spec +++ b/iSulad.spec @@ -1,5 +1,5 @@ %global _version 2.0.18 -%global _release 17 +%global _release 18 %global is_systemd 1 %global enable_shimv2 1 %global is_embedded 1 @@ -217,6 +217,15 @@ Patch0201: 0201-bugfix-for-the-concurrency-competition-between-the-r.patch Patch0202: 0202-add-concurrent-load-test.patch Patch0203: 0203-get-the-realpath-of-the-host-path-for-archive-when-c.patch Patch0204: 0204-bugfix-for-wrong-goto-branch.patch +Patch0205: 0205-Use-user-defined-shm-for-CRI-request.patch +Patch0206: 0206-Fix-memory-leak-in-set_connected_container_shm_path.patch +Patch0207: 0207-modify-some-grpc-status-codes-of-cri-in-case-of-erro.patch +Patch0208: 0208-bugfix-for-the-pre-created-pipe-was-not-closed-when-.patch +Patch0209: 0209-add-debug-msg-info-in-image_load.sh.patch +Patch0210: 0210-allow-env-variable-has-an-empty-value.patch +Patch0211: 0211-bugfix-for-hostname-env-set-only-once.patch +Patch0212: 0212-fix-bug-for-invalid-env-write.patch +Patch0213: 0213-change-fork-process-exit-mode.patch %ifarch x86_64 aarch64 Provides: libhttpclient.so()(64bit) @@ -461,6 +470,12 @@ fi %endif %changelog +* Tue Jun 11 2024 zhongtao - 2.0.18-18 +- Type: bugfix +- ID: NA +- SUG: NA +- DESC: bugfix for env/cri/fork + * Tue Mar 19 2024 zhongtao - 2.0.18-17 - Type: bugfix - ID: NA