From ca7d6cf3597669363407e5dbb861ed75b834c5dc Mon Sep 17 00:00:00 2001 From: xxlight Date: Tue, 27 Jun 2023 23:34:29 +0800 Subject: [PATCH 1/6] Description: fix problem about cxxdeps Issue: https://gitee.com/openharmony/build/issues/I7GK8Y?from=project-issue Test: build Signed-off-by: xxlight Change-Id: If63e733a9262669857d286b01eeeff11b197a648 Change-Id: I8a449d56ad3fb5342dd4360811ea4b393988f7db --- base/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/BUILD.gn b/base/BUILD.gn index e2e2a26..ae8980d 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -204,7 +204,7 @@ if (host_os == "linux" && !is_asan) { ] deps = [ ":cxx_rust_gen", - "//build/rust/tests:cxx_cppdeps", + "//third_party/rust/crates/cxx:cxx_cppdeps", ] public_deps = [ "//third_party/bounds_checking_function:libsec_shared" ] -- Gitee From 4d074190e0021f8131caecd1298003944f4f544d Mon Sep 17 00:00:00 2001 From: huangyuchen Date: Tue, 27 Jun 2023 17:53:54 +0800 Subject: [PATCH 2/6] Remove redundent interfaces of `utils_rust::ashmem` and add test cases. Issue: I7GHVM Test: UT Signed-off-by: huangyuchen Change-Id: Ic1cf093b2450ff472cadfbda0e20648dae98983f --- base/src/rust/ashmem.rs | 19 ------------------- .../unittest/rust/rust_utils_ashmem_test.rs | 3 ++- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/base/src/rust/ashmem.rs b/base/src/rust/ashmem.rs index 055885c..36ff005 100644 --- a/base/src/rust/ashmem.rs +++ b/base/src/rust/ashmem.rs @@ -36,13 +36,6 @@ pub mod ffi { include!("commonlibrary/c_utils/base/include/ashmem.h"); // global function - /** - * Create an ashmem and return its FD. - * # Safety - * Requires C-style string as parameter to specify the name of Ashmem. - */ - pub unsafe fn AshmemCreate(name: *const c_char, size: usize) -> i32; - /** * Create an C++ Ashmem object managed by std::shared_ptr. * # Safety @@ -217,16 +210,4 @@ pub unsafe fn create_ashmem_instance(name: &str, size: i32) -> Option { } else { Some(Ashmem::new(c_ashmem_ptr)) } -} - -/** - * Create raw ashmem and returns its FD. - * # Safety - * Transmits c-style string of `name`. - */ -#[allow(dead_code)] -pub unsafe fn ashmem_create(name: &str, size: i32) -> i32 { - let c_name = CString::new(name).expect("CString::new Failed!"); - let name_ptr = c_name.as_ptr(); - ffi::AshmemCreate(name_ptr, size.try_into().expect("Invalid size.")) } \ No newline at end of file diff --git a/base/test/unittest/rust/rust_utils_ashmem_test.rs b/base/test/unittest/rust/rust_utils_ashmem_test.rs index b781652..0187a50 100644 --- a/base/test/unittest/rust/rust_utils_ashmem_test.rs +++ b/base/test/unittest/rust/rust_utils_ashmem_test.rs @@ -290,7 +290,6 @@ fn test_ashmem_ffi_invalid_006() ashmem.CloseAshmem(); } - #[test] fn test_ashmem_write_read_001() { @@ -299,6 +298,8 @@ fn test_ashmem_write_read_001() let ashmem = ashmem.unwrap(); assert_eq!(ashmem.get_ashmem_size(), MEMORY_SIZE); + assert_eq!(ashmem.get_protection(), ashmem::PROT_READ | ashmem::PROT_WRITE | ashmem::PROT_EXEC); // default protection mask. + assert_ne!(ashmem.get_ashmem_fd(), -1); assert!(ashmem.map_ashmem(ashmem::PROT_READ | ashmem::PROT_WRITE)); -- Gitee From 73297fbf5a17ca91d1b6cda0121281baa677cdd4 Mon Sep 17 00:00:00 2001 From: huangyuchen Date: Sun, 25 Jun 2023 16:49:20 +0800 Subject: [PATCH 3/6] Fix failing to remove a dangling symlink file in `ForceRemoveDirectory`. 1. Replace `access` to `faccessat` 2. Add related test cases. Issue:I7FUB2 Test:UT Signed-off-by: huangyuchen Change-Id: I8409f783ed9eb88a06517c26f7120989278c70b3 --- base/src/directory_ex.cpp | 9 +-- .../unittest/common/utils_directory_test.cpp | 72 ++++++++++++++++++- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/base/src/directory_ex.cpp b/base/src/directory_ex.cpp index dd8e6f7..9e79b9b 100644 --- a/base/src/directory_ex.cpp +++ b/base/src/directory_ex.cpp @@ -15,7 +15,8 @@ #include "directory_ex.h" #include -#include +#include +#include #include "securec.h" #include "unistd.h" #include "utils_log.h" @@ -217,7 +218,7 @@ bool ForceRemoveDirectory(const string& path) if (ptr->d_type == DT_DIR) { ret = ForceRemoveDirectory(subPath); } else { - if (access(subPath.c_str(), F_OK) == 0) { + if (faccessat(AT_FDCWD, subPath.c_str(), F_OK, AT_SYMLINK_NOFOLLOW) == 0) { if (remove(subPath.c_str()) != 0) { closedir(dir); return false; @@ -228,13 +229,13 @@ bool ForceRemoveDirectory(const string& path) closedir(dir); string currentPath = ExcludeTrailingPathDelimiter(path); - if (access(currentPath.c_str(), F_OK) == 0) { + if (faccessat(AT_FDCWD, currentPath.c_str(), F_OK, AT_SYMLINK_NOFOLLOW) == 0) { if (remove(currentPath.c_str()) != 0) { return false; } } - return ret && (access(path.c_str(), F_OK) != 0); + return ret && (faccessat(AT_FDCWD, path.c_str(), F_OK, AT_SYMLINK_NOFOLLOW) != 0); } bool RemoveFile(const string& fileName) diff --git a/base/test/unittest/common/utils_directory_test.cpp b/base/test/unittest/common/utils_directory_test.cpp index d8d0f95..3ac7148 100644 --- a/base/test/unittest/common/utils_directory_test.cpp +++ b/base/test/unittest/common/utils_directory_test.cpp @@ -12,11 +12,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include + #include "directory_ex.h" +#include +#include #include #include #include + using namespace testing::ext; using namespace std; @@ -212,6 +215,73 @@ HWTEST_F(UtilsDirectoryTest, testRemoveFile001, TestSize.Level0) EXPECT_EQ(ret, true); } +/* + * @tc.name: testRemoveFile002 + * @tc.desc: Remove soft link file. + */ +HWTEST_F(UtilsDirectoryTest, testRemoveFile002, TestSize.Level0) +{ + string dirpath = "/data/test_dir"; + bool ret = ForceCreateDirectory(dirpath); + EXPECT_EQ(ret, true); + + string targetname = "/data/test_target.txt"; + FILE *fp = fopen(targetname.c_str(), "w"); + if (NULL != fp) { + fclose(fp); + } + + // symlink to a directory + string linkpath = "/data/test_symlink_dir"; + int res = symlink(dirpath.c_str(), linkpath.c_str()); + EXPECT_EQ(res, 0); + + ret = ForceRemoveDirectory(linkpath); + EXPECT_EQ(ret, true); + + // Target dir is not removed. + ret = faccessat(AT_FDCWD, dirpath.c_str(), F_OK, AT_SYMLINK_NOFOLLOW); + EXPECT_EQ(ret, 0); + + // symlink to a file + string filename = dirpath + "/test.txt"; + res = symlink(targetname.c_str(), filename.c_str()); + EXPECT_EQ(res, 0); + + ret = ForceRemoveDirectory(dirpath); + EXPECT_EQ(ret, true); + + // Target file is not removed. + ret = faccessat(AT_FDCWD, targetname.c_str(), F_OK, AT_SYMLINK_NOFOLLOW); + EXPECT_EQ(ret, 0); + + ret = RemoveFile(targetname); + EXPECT_EQ(ret, true); +} + +/* + * @tc.name: testRemoveFile003 + * @tc.desc: Remove dangling soft link file. + */ +HWTEST_F(UtilsDirectoryTest, testRemoveFile003, TestSize.Level0) +{ + string dirpath = "/data/test_dir"; + bool ret = ForceCreateDirectory(dirpath); + EXPECT_EQ(ret, true); + + // symlink to a file + string targetname = "/data/nonexisted.txt"; + string filename = dirpath + "/test.txt"; + int res = symlink(targetname.c_str(), filename.c_str()); + EXPECT_EQ(res, 0); + + ret = ForceRemoveDirectory(dirpath); + EXPECT_EQ(ret, true); + + ret = RemoveFile(targetname); + EXPECT_EQ(ret, true); +} + /* * @tc.name: testGetFolderSize001 * @tc.desc: directory unit test -- Gitee From 20ff765db9e0ddf956511de5cb278db16abe4b5d Mon Sep 17 00:00:00 2001 From: peilixia Date: Tue, 27 Jun 2023 16:56:28 +0800 Subject: [PATCH 4/6] Enable Int-Sanitizer for c_utils. Issue:I73WM9 Test:NA unittest Signed-off-by: peilixia --- base/BUILD.gn | 8 ++++++ base/src/datetime_ex.cpp | 3 +++ base/src/file_ex.cpp | 26 +++++++++---------- base/src/string_ex.cpp | 16 ++++++++++-- .../unittest/common/utils_datetime_test.cpp | 2 ++ bundle.json | 5 +++- 6 files changed, 44 insertions(+), 16 deletions(-) diff --git a/base/BUILD.gn b/base/BUILD.gn index ae8980d..338aafb 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -19,6 +19,7 @@ declare_args() { c_utils_track_all = false c_utils_print_track_at_once = false c_utils_debug_log_enabled = false + c_utils_feature_intsan = true } config("utils_config") { @@ -148,6 +149,13 @@ ohos_shared_library("utils") { } else { sources = sources_utils configs = [ ":utils_coverage_config" ] + + if (c_utils_feature_intsan) { + sanitize = { + integer_overflow = true + } + } + if (c_utils_debug_refbase) { configs += [ ":debug_refbase" ] if (c_utils_track_all) { diff --git a/base/src/datetime_ex.cpp b/base/src/datetime_ex.cpp index 12a8145..3ec5508 100644 --- a/base/src/datetime_ex.cpp +++ b/base/src/datetime_ex.cpp @@ -46,6 +46,9 @@ int64_t GetSecondsBetween(struct tm inputTm1, struct tm inputTm2) { int64_t second1 = GetSecondsSince1970ToPointTime(inputTm1); int64_t second2 = GetSecondsSince1970ToPointTime(inputTm2); + if (second1 == -1 || second2 == -1) { + return -1; + } return second1 >= second2 ? (second1 - second2) : (second2 - second1); } diff --git a/base/src/file_ex.cpp b/base/src/file_ex.cpp index b7e3a6e..897cc41 100644 --- a/base/src/file_ex.cpp +++ b/base/src/file_ex.cpp @@ -116,9 +116,9 @@ bool LoadStringFromFile(const string& filePath, string& content) } file.seekg(0, ios::end); - const long fileLength = file.tellg(); + const long long fileLength = file.tellg(); if (fileLength > MAX_FILE_LENGTH) { - UTILS_LOGD("invalid file length(%{public}ld)!", fileLength); + UTILS_LOGD("invalid file length(%{public}lld)!", fileLength); return false; } @@ -137,7 +137,7 @@ string GetFileNameByFd(const int fd) string fdPath = "/proc/self/fd/" + std::to_string(fd); char fileName[PATH_MAX + 1] = {0}; - int ret = readlink(fdPath.c_str(), fileName, PATH_MAX); + ssize_t ret = readlink(fdPath.c_str(), fileName, PATH_MAX); if (ret < 0 || ret > PATH_MAX) { UTILS_LOGD("Get fileName failed, ret is: %{public}d!", ret); return string(); @@ -168,9 +168,9 @@ bool LoadStringFromFd(int fd, string& content) return false; } - const long fileLength = lseek(fd, 0, SEEK_END); + const off_t fileLength = lseek(fd, 0, SEEK_END); if (fileLength > MAX_FILE_LENGTH) { - UTILS_LOGE("invalid file length(%{public}ld)!", fileLength); + UTILS_LOGE("invalid file length(%{public}jd)!", static_cast(fileLength)); return false; } @@ -184,16 +184,16 @@ bool LoadStringFromFd(int fd, string& content) } content.resize(fileLength); - int loc = lseek(fd, 0, SEEK_SET); + off_t loc = lseek(fd, 0, SEEK_SET); if (loc == -1) { UTILS_LOGE("lseek file to begin failed!"); return false; } - const long len = read(fd, content.data(), fileLength); + const ssize_t len = read(fd, content.data(), fileLength); if (len != fileLength) { - UTILS_LOGE("the length read from file is not equal to fileLength!len:%{public}ld,fileLen:%{public}ld", - len, fileLength); + UTILS_LOGE("the length read from file is not equal to fileLength!len:%{public}zd,fileLen:%{public}jd", + len, static_cast(fileLength)); return false; } @@ -240,14 +240,14 @@ bool SaveStringToFd(int fd, const std::string& content) return true; } - const long len = write(fd, content.c_str(), content.length()); + const ssize_t len = write(fd, content.c_str(), content.length()); if (len < 0) { UTILS_LOGE("write file failed!errno:%{public}d, err:%{public}s", errno, strerror(errno)); return false; } if (static_cast(len) != content.length()) { - UTILS_LOGE("the length write to file is not equal to fileLength!len:%{public}ld, fileLen:%{public}zu", + UTILS_LOGE("the length write to file is not equal to fileLength!len:%{public}zd, fileLen:%{public}zu", len, content.length()); return false; } @@ -301,9 +301,9 @@ bool LoadBufferFromFile(const string& filePath, vector& content) } file.seekg(0, std::ios::end); - const long fileLength = file.tellg(); + const long long fileLength = file.tellg(); if (fileLength > MAX_FILE_LENGTH) { - UTILS_LOGD("invalid file length(%{public}ld)!", fileLength); + UTILS_LOGD("invalid file length(%{public}lld)!", fileLength); return false; } diff --git a/base/src/string_ex.cpp b/base/src/string_ex.cpp index b279544..0082ff7 100644 --- a/base/src/string_ex.cpp +++ b/base/src/string_ex.cpp @@ -56,9 +56,21 @@ string ReplaceStr(const string& str, const string& src, const string& dst) string TrimStr(const string& str, const char cTrim /*= ' '*/) { + if (str.size() == 1 && str[0] == cTrim) { + return string{}; + } + string strTmp = str; - strTmp.erase(0, strTmp.find_first_not_of(cTrim)); - strTmp.erase(strTmp.find_last_not_of(cTrim) + sizeof(char)); + std::string::size_type firstBound = strTmp.find_first_not_of(cTrim); + if (firstBound != std::string::npos) { + strTmp.erase(0, firstBound); + } + + std::string::size_type lastBound = strTmp.find_last_not_of(cTrim); + if (lastBound != std::string::npos && lastBound != strTmp.size() - 1) { + strTmp.erase(lastBound + sizeof(char)); + } + return strTmp; } diff --git a/base/test/unittest/common/utils_datetime_test.cpp b/base/test/unittest/common/utils_datetime_test.cpp index 4cc88a5..c10c219 100644 --- a/base/test/unittest/common/utils_datetime_test.cpp +++ b/base/test/unittest/common/utils_datetime_test.cpp @@ -91,6 +91,8 @@ HWTEST_F(UtilsDateTimeTest, testTime001, TestSize.Level0) info.tm_isdst = 0; second2 = GetSecondsSince1970ToPointTime(info); EXPECT_EQ(-1, second2); + int64_t ret2 = GetSecondsBetween(curTime, info); + EXPECT_TRUE(ret2 = -1); } /* diff --git a/bundle.json b/bundle.json index 19d708d..4366b17 100644 --- a/bundle.json +++ b/bundle.json @@ -16,7 +16,10 @@ "name": "c_utils", "subsystem": "commonlibrary", "adapted_system_type": [ "standard" ], - "features":[ "c_utils_feature_coverage = false" ], + "features":[ + "c_utils_feature_coverage = false", + "c_utils_feature_intsan = true" + ], "deps": { "components": [ "hilog" -- Gitee From 196834894cbbdeafac5281f09b8688baa97d7821 Mon Sep 17 00:00:00 2001 From: huangyuchen Date: Fri, 30 Jun 2023 20:02:30 +0800 Subject: [PATCH 5/6] Fix UAF issue on timer. Issue:I7HYIO Test:UtilsTimerTest Signed-off-by: huangyuchen Change-Id: Id598175e92759d509abf4ccdab68b6c5374206a6 --- base/src/event_demultiplexer.cpp | 20 ++++++++++++++++---- base/src/event_demultiplexer.h | 3 ++- base/src/event_handler.h | 4 +++- base/src/timer.cpp | 8 -------- base/src/timer_event_handler.h | 2 +- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/base/src/event_demultiplexer.cpp b/base/src/event_demultiplexer.cpp index 87df8a7..e444684 100644 --- a/base/src/event_demultiplexer.cpp +++ b/base/src/event_demultiplexer.cpp @@ -70,7 +70,7 @@ uint32_t EventDemultiplexer::UpdateEventHandler(EventHandler* handler) std::lock_guard lock(mutex_); auto itor = eventHandlers_.find(handler->GetHandle()); if (itor == eventHandlers_.end()) { - eventHandlers_.insert(std::make_pair(handler->GetHandle(), handler)); + eventHandlers_.insert(std::make_pair(handler->GetHandle(), handler->shared_from_this())); return Update(EPOLL_CTL_ADD, handler); } @@ -79,7 +79,7 @@ uint32_t EventDemultiplexer::UpdateEventHandler(EventHandler* handler) return Update(EPOLL_CTL_DEL, handler); } - if (handler != itor->second) { + if (handler != itor->second.get()) { return TIMER_ERR_DEAL_FAILED; } return Update(EPOLL_CTL_MOD, handler); @@ -102,7 +102,19 @@ uint32_t EventDemultiplexer::Update(int operation, EventHandler* handler) void EventDemultiplexer::Polling(int timeout /* ms */) { + std::vector> holdHandlers; std::vector epollEvents(maxEvents_); + { + std::lock_guard lock(mutex_); + if (eventHandlers_.size() == 0) { + return; + } + + for (auto itor = eventHandlers_.begin(); itor != eventHandlers_.end();++itor) { + holdHandlers.emplace_back(itor->second); + } + } + int nfds = epoll_wait(epollFd_, &epollEvents[0], static_cast(epollEvents.size()), timeout); if (nfds == 0) { return; @@ -113,8 +125,8 @@ void EventDemultiplexer::Polling(int timeout /* ms */) } for (int idx = 0; idx < nfds; ++idx) { - uint32_t events = epollEvents[idx].events; void* ptr = epollEvents[idx].data.ptr; + uint32_t events = epollEvents[idx].events; auto handler = reinterpret_cast(ptr); if (handler != nullptr) { handler->HandleEvents(Epoll2Reactor(events)); @@ -149,4 +161,4 @@ uint32_t EventDemultiplexer::Reactor2Epoll(uint32_t reactorEvent) } } -} +} \ No newline at end of file diff --git a/base/src/event_demultiplexer.h b/base/src/event_demultiplexer.h index 5c25e56..13fe966 100644 --- a/base/src/event_demultiplexer.h +++ b/base/src/event_demultiplexer.h @@ -17,6 +17,7 @@ #define UTILS_EVENT_DEMULTIPLEXER_H #include +#include #include #include @@ -47,7 +48,7 @@ private: int epollFd_; int maxEvents_; std::recursive_mutex mutex_; - std::map eventHandlers_; // guard by mutex_ + std::map> eventHandlers_; // guard by mutex_ }; } diff --git a/base/src/event_handler.h b/base/src/event_handler.h index af75cbe..511bb6f 100644 --- a/base/src/event_handler.h +++ b/base/src/event_handler.h @@ -19,13 +19,15 @@ #include #include #include +#include +#include namespace OHOS { namespace Utils { class EventReactor; -class EventHandler { +class EventHandler : public std::enable_shared_from_this { public: using Callback = std::function; diff --git a/base/src/timer.cpp b/base/src/timer.cpp index 4fc9a1d..bb4db90 100644 --- a/base/src/timer.cpp +++ b/base/src/timer.cpp @@ -49,14 +49,6 @@ void Timer::Shutdown(bool useJoin) } reactor_->SwitchOff(); - if (timeoutMs_ == -1) { - std::lock_guard lock(mutex_); - if (intervalToTimers_.empty()) { - UTILS_LOGI("no event for epoll wait, use detach to shutdown"); - thread_.detach(); - return; - } - } if (!useJoin) { thread_.detach(); return; diff --git a/base/src/timer_event_handler.h b/base/src/timer_event_handler.h index 3201480..1fd0bd6 100644 --- a/base/src/timer_event_handler.h +++ b/base/src/timer_event_handler.h @@ -55,7 +55,7 @@ private: uint32_t interval_; EventReactor* reactor_; - std::unique_ptr handler_; + std::shared_ptr handler_; TimerCallback callback_; }; -- Gitee From a2717e988105f332dcd8053cc731c3c214b4a576 Mon Sep 17 00:00:00 2001 From: huangyuchen Date: Sat, 1 Jul 2023 16:09:35 +0800 Subject: [PATCH 6/6] Remove invalid test cases since int-san is enabled. Issue:I7HM3W Test:UtilsParcelTest Signed-off-by: huangyuchen Change-Id: Iaf35fd144709976bb0cc0bf6a35ae45e640df8cf --- base/test/unittest/common/utils_parcel_test.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/base/test/unittest/common/utils_parcel_test.cpp b/base/test/unittest/common/utils_parcel_test.cpp index ff8bbbf..46653d8 100644 --- a/base/test/unittest/common/utils_parcel_test.cpp +++ b/base/test/unittest/common/utils_parcel_test.cpp @@ -1307,12 +1307,6 @@ HWTEST_F(UtilsParcelTest, test_parcel_Data_Structure_002, TestSize.Level0) result = parcel.WriteBufferAddTerminator(static_cast(str.data()), 0, sizeof(char)); EXPECT_EQ(false, result); - result = parcel.WriteBuffer(static_cast(strOverflow.data()), SIZE_MAX); - EXPECT_EQ(false, result); - result = parcel.WriteBufferAddTerminator(static_cast(strOverflow.data()), - SIZE_MAX, sizeof(char)); - EXPECT_EQ(false, result); - result = parcel.WriteBuffer(static_cast(strWriteFail.data()), strWriteFail.length()); EXPECT_EQ(false, result); result = parcel.WriteBufferAddTerminator(static_cast(strWriteFail.data()), -- Gitee