From 54c7c98ef65a3fcfd1af35c5c566f9a75e342acf Mon Sep 17 00:00:00 2001 From: lifeng68 Date: Fri, 17 Jul 2020 11:50:31 +0800 Subject: [PATCH] selinux: fix memory leak in selinux Signed-off-by: lifeng68 --- src/daemon/common/selinux_label.c | 9 ++++-- src/daemon/common/selinux_label.h | 2 +- .../oci/storage/rootfs/storage_rootfs_ut.cpp | 19 ++++++++----- .../execution_extend/execution_extend_ut.cpp | 5 ++++ .../execution/spec/selinux_label_ut.cpp | 28 +++++++++++-------- 5 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/daemon/common/selinux_label.c b/src/daemon/common/selinux_label.c index 572ea22..b97e07f 100644 --- a/src/daemon/common/selinux_label.c +++ b/src/daemon/common/selinux_label.c @@ -326,7 +326,7 @@ out: } /* selinux state free */ -static void selinux_state_free(selinux_state *state) +static void do_selinux_state_free(selinux_state *state) { if (state == NULL) { return; @@ -363,7 +363,7 @@ static selinux_state *selinux_state_new(void) return state; error_out: - selinux_state_free(g_selinux_state); + do_selinux_state_free(state); return NULL; } @@ -378,6 +378,11 @@ int selinux_state_init(void) return 0; } +void selinux_state_free() +{ + do_selinux_state_free(g_selinux_state); +} + /* MCS already exists */ static bool is_mcs_already_exists(const char *mcs) { diff --git a/src/daemon/common/selinux_label.h b/src/daemon/common/selinux_label.h index 6bb34ec..625e94c 100644 --- a/src/daemon/common/selinux_label.h +++ b/src/daemon/common/selinux_label.h @@ -31,7 +31,7 @@ int relabel(const char *path, const char *file_label, bool shared); int get_disable_security_opt(char ***labels, size_t *labels_len); int dup_security_opt(const char *src, char ***dst, size_t *len); char *selinux_format_mountlabel(const char *src, const char *mount_label); - +void selinux_state_free(); #ifdef __cplusplus } #endif diff --git a/test/image/oci/storage/rootfs/storage_rootfs_ut.cpp b/test/image/oci/storage/rootfs/storage_rootfs_ut.cpp index 9003ff0..cca5d29 100644 --- a/test/image/oci/storage/rootfs/storage_rootfs_ut.cpp +++ b/test/image/oci/storage/rootfs/storage_rootfs_ut.cpp @@ -104,7 +104,7 @@ protected: } std::vector ids { "0e025f44cdca20966a5e5f11e1d9d8eb726aef2d38ed20f89ea986987c2010a9", - "28a8e1311d71345b08788c16b8c4f45a57641854f0e7c16802eedd0eb334b832" }; + "28a8e1311d71345b08788c16b8c4f45a57641854f0e7c16802eedd0eb334b832" }; char store_real_path[PATH_MAX] = { 0x00 }; }; @@ -127,7 +127,9 @@ TEST_F(StorageRootfsUnitTest, test_rootfs_load) ASSERT_EQ(cntr->names_len, 1); ASSERT_STREQ(cntr->names[0], "0e025f44cdca20966a5e5f11e1d9d8eb726aef2d38ed20f89ea986987c2010a9"); ASSERT_EQ(rootfs_store_set_big_data(ids.at(0).c_str(), "userdata", BIG_DATA_CONTENT.c_str()), 0); - ASSERT_STREQ(rootfs_store_big_data(ids.at(0).c_str(), "userdata"), BIG_DATA_CONTENT.c_str()); + char *userdata_tmp = NULL; + userdata_tmp = rootfs_store_big_data(ids.at(0).c_str(), "userdata"); + ASSERT_STREQ(userdata_tmp, BIG_DATA_CONTENT.c_str()); ASSERT_EQ(rootfs_store_set_metadata(ids.at(0).c_str(), META_DATA_CONTENT.c_str()), 0); cntr_tmp = rootfs_store_get_rootfs(ids.at(0).c_str()); @@ -137,6 +139,7 @@ TEST_F(StorageRootfsUnitTest, test_rootfs_load) free_storage_rootfs(cntr); free_storage_rootfs(cntr_tmp); + free(userdata_tmp); } TEST_F(StorageRootfsUnitTest, test_rootfs_store_create) @@ -149,11 +152,11 @@ TEST_F(StorageRootfsUnitTest, test_rootfs_store_create) std::string layer_without_id { "h88ca140c6716a68d7bba0fe6529334e98de529bd8fb7agf3a21f08e772629a9" }; std::string metadata { "{}" }; char *created_container = rootfs_store_create(id.c_str(), names_with_id, - sizeof(names_with_id) / sizeof(names_with_id[0]), image.c_str(), layer_with_id.c_str(), - metadata.c_str(), nullptr); - char *container_without_id = rootfs_store_create(nullptr, names_without_id, - sizeof(names_without_id) / sizeof(names_without_id[0]), image.c_str(), - layer_without_id.c_str(), metadata.c_str(), nullptr); + sizeof(names_with_id) / sizeof(names_with_id[0]), image.c_str(), + layer_with_id.c_str(), metadata.c_str(), nullptr); + char *container_without_id = + rootfs_store_create(nullptr, names_without_id, sizeof(names_without_id) / sizeof(names_without_id[0]), + image.c_str(), layer_without_id.c_str(), metadata.c_str(), nullptr); ASSERT_STREQ(created_container, id.c_str()); ASSERT_NE(container_without_id, nullptr); @@ -161,6 +164,8 @@ TEST_F(StorageRootfsUnitTest, test_rootfs_store_create) ASSERT_EQ(rootfs_store_get_rootfs(id.c_str()), nullptr); ASSERT_EQ(rootfs_store_delete(container_without_id), 0); ASSERT_FALSE(dirExists((std::string(store_real_path) + "/" + id).c_str())); + free(created_container); + free(container_without_id); } TEST_F(StorageRootfsUnitTest, test_rootfs_store_lookup) diff --git a/test/services/execution/execute/execution_extend/execution_extend_ut.cpp b/test/services/execution/execute/execution_extend/execution_extend_ut.cpp index dcc468d..d12aca0 100644 --- a/test/services/execution/execute/execution_extend/execution_extend_ut.cpp +++ b/test/services/execution/execute/execution_extend/execution_extend_ut.cpp @@ -135,6 +135,7 @@ container_t *invokeContainersStoreGet(const char *id_or_name) container_t *cont = (container_t *)util_common_calloc_s(sizeof(container_t)); cont->common_config = (container_config_v2_common_config *)util_common_calloc_s(sizeof(container_config_v2_common_config)); + cont->refcnt = 1; return cont; } @@ -221,6 +222,8 @@ TEST_F(ExecutionExtendUnitTest, test_container_extend_callback_init_pause) testing::Mock::VerifyAndClearExpectations(&m_containersGc); testing::Mock::VerifyAndClearExpectations(&m_containersOperator); testing::Mock::VerifyAndClearExpectations(&m_containerUnix); + free_container_pause_request(request); + free_container_pause_response(response); } TEST_F(ExecutionExtendUnitTest, test_container_extend_callback_init_resume) @@ -246,4 +249,6 @@ TEST_F(ExecutionExtendUnitTest, test_container_extend_callback_init_resume) testing::Mock::VerifyAndClearExpectations(&m_containersGc); testing::Mock::VerifyAndClearExpectations(&m_containersOperator); testing::Mock::VerifyAndClearExpectations(&m_containerUnix); + free_container_resume_request(request); + free_container_resume_response(response); } diff --git a/test/services/execution/spec/selinux_label_ut.cpp b/test/services/execution/spec/selinux_label_ut.cpp index 5dd828b..d181a59 100644 --- a/test/services/execution/spec/selinux_label_ut.cpp +++ b/test/services/execution/spec/selinux_label_ut.cpp @@ -36,8 +36,7 @@ protected: } void TearDown() override { - std::cout << "selinux_state is the resident memory of the daemon." << - " The process exits and the memory is automatically released." << std::endl; + selinux_state_free(); } }; @@ -48,15 +47,17 @@ TEST_F(SELinuxLabelUnitTest, test_init_label_normal) const char *role_label[] = { "role:fakerole" }; const char *type_label[] = { "type:faketype" }; const char *level_label[] = { "level:s0:c1,c2" }; - const char *full_label[] = { "user:fakeuser", "level:s0:c1,c2", "type:faketype", "role:fakerole" }; + const char *full_label[] = { "user:fakeuser", "level:s0:c1,c2", "type:faketype", "role:fakerole" }; std::vector> normal { std::make_tuple(disable_label, 1, 0, "", ""), std::make_tuple(user_label, 1, 0, "fakeuser:system_r:container_t:s0", "fakeuser:object_r:container_file_t:s0"), std::make_tuple(role_label, 1, 0, "system_u:fakerole:container_t:s0", "system_u:object_r:container_file_t:s0"), std::make_tuple(type_label, 1, 0, "system_u:system_r:faketype:s0", "system_u:object_r:container_file_t:s0"), - std::make_tuple(level_label, 1, 0, "system_u:system_r:container_t:s0:c1,c2", "system_u:object_r:container_file_t:s0:c1,c2"), - std::make_tuple(full_label, 4, 0, "fakeuser:fakerole:faketype:s0:c1,c2", "fakeuser:object_r:container_file_t:s0:c1,c2"), + std::make_tuple(level_label, 1, 0, "system_u:system_r:container_t:s0:c1,c2", + "system_u:object_r:container_file_t:s0:c1,c2"), + std::make_tuple(full_label, 4, 0, "fakeuser:fakerole:faketype:s0:c1,c2", + "fakeuser:object_r:container_file_t:s0:c1,c2"), std::make_tuple(nullptr, 0, 0, "system_u:system_r:container_t:s0", "system_u:object_r:container_file_t:s0"), }; @@ -92,8 +93,8 @@ TEST_F(SELinuxLabelUnitTest, test_init_label_normal) TEST_F(SELinuxLabelUnitTest, test_init_label_abnormal) { - const char *invalid_key_label[] = { "xxx" }; - const char *invalid_value_label[] = { "user:" }; + const char *invalid_key_label[] = { "xxx" }; + const char *invalid_value_label[] = { "user:" }; std::vector> normal { std::make_tuple(invalid_key_label, 1, -1, "", ""), @@ -176,10 +177,14 @@ protected: TEST_F(SELinuxRelabelUnitTest, test_relabel_normal) { std::vector> normal { - std::make_tuple("system_u:object_r:container_file_t:s0:c100,c200", false, 0, "system_u:object_r:container_file_t:s0:c100,c200"), - std::make_tuple("system_u:object_r:container_file_t:s0:c300,c300", false, 0, "system_u:object_r:container_file_t:s0:c300"), - std::make_tuple("system_u:object_r:container_file_t:s0:c100,c200", true, 0, "system_u:object_r:container_file_t:s0"), - std::make_tuple("system_u:object_r:container_file_t:s0:c300,c300", true, 0, "system_u:object_r:container_file_t:s0"), + std::make_tuple("system_u:object_r:container_file_t:s0:c100,c200", false, 0, + "system_u:object_r:container_file_t:s0:c100,c200"), + std::make_tuple("system_u:object_r:container_file_t:s0:c300,c300", false, 0, + "system_u:object_r:container_file_t:s0:c300"), + std::make_tuple("system_u:object_r:container_file_t:s0:c100,c200", true, 0, + "system_u:object_r:container_file_t:s0"), + std::make_tuple("system_u:object_r:container_file_t:s0:c300,c300", true, 0, + "system_u:object_r:container_file_t:s0"), }; if (!is_selinux_enabled()) { @@ -234,4 +239,3 @@ TEST_F(SELinuxRelabelUnitTest, test_get_disable_security_opt) util_free_array(labels); } - -- GitLab