已合并
[Ubse] #fix# ubse修复asan扫描内存泄露 #1449
朱程成创建于 13 天前
[Ubse] #fix# ubse修复asan扫描内存泄露 #1449
已合并
共 5 个文件变更+87-17
| @@ -3,6 +3,7 @@ | |||
| 3 | 3 | ||
| 4 | 4 | ||
| 5 | 5 | ||
| 6 | + | ||
| 6 | 7 | ||
| 7 | 8 | ||
| 8 | 9 | ||
| @@ -166,9 +167,25 @@ void UbseRasObserver::Stop() | |||
| 166 | InvalidateSysSentryConfig(); | 167 | InvalidateSysSentryConfig(); |
| 167 | UnregisterConfigRetryTimer(); | 168 | UnregisterConfigRetryTimer(); |
| 168 | ubse::timer::UbseTimerHandlerUnregister(UBSE_RAS_QUERY_MSG_MONITOR_TIMER_NAME); | 169 | ubse::timer::UbseTimerHandlerUnregister(UBSE_RAS_QUERY_MSG_MONITOR_TIMER_NAME); |
| 170 | + | ||
| 171 | + // 仅断开底层 fd 唤醒阻塞在 xalarmGetEventFunc 的工作线程,不释放 alarm_register, | ||
| 172 | + // 避免工作线程持有的快照成为悬垂指针;结构体由工作线程退出时统一释放。 | ||
| 173 | + { | ||
| 174 | + std::lock_guard<std::mutex> lock(registerMtx_); | ||
| 175 | + if (registerInfo_ != nullptr && registerInfo_->register_fd >= 0) { | ||
| 176 | + // register_fd 为 socket 时 shutdown 可立即唤醒阻塞中的 recv,且 fd 号延迟到 | ||
| 177 | + // 工作线程最终 UnRegisterXalarm 时才释放,避免 close 后 fd 号被其他线程复用; | ||
| 178 | + // 非 socket(如 FIFO)时 shutdown 失败,退化为 close 并标记 -1, | ||
| 179 | + // 防止工作线程最终注销时对已释放的 fd 号二次 close。 | ||
| 180 | + if (shutdown(registerInfo_->register_fd, SHUT_RDWR) != 0) { | ||
| 181 | + close(registerInfo_->register_fd); | ||
| 182 | + registerInfo_->register_fd = -1; // 标记已断开,防止工作线程重用 | ||
| 183 | + } | ||
| 184 | + } | ||
| 185 | + } | ||
| 186 | + | ||
| 169 | if (worker && worker->joinable()) { | 187 | if (worker && worker->joinable()) { |
| 170 | - worker->detach(); | 188 | + worker->join(); |
| 171 | - // 确保释放线程资源 | ||
| 172 | worker.reset(); | 189 | worker.reset(); |
| 173 | } | 190 | } |
| 174 | } | 191 | } |
| @@ -189,9 +206,12 @@ void LogValidFaultMsg(const std::string& invalidStr) | |||
| 189 | 206 | ||
| 190 | void UbseRasObserver::SentryEventListen() | 207 | void UbseRasObserver::SentryEventListen() |
| 191 | { | 208 | { |
| 192 | - struct alarm_register* registerInfo = nullptr; | 209 | + { |
| 193 | - RegisterSentryEvent(®isterInfo); | 210 | + // 首次注册同样纳入 registerMtx_ 保护,避免快速停止场景下与 Stop 并发读写 registerInfo_ |
| 194 | - if (registerInfo == nullptr) { | 211 | + std::lock_guard<std::mutex> lock(registerMtx_); |
| 212 | + RegisterSentryEvent(®isterInfo_); | ||
| 213 | + } | ||
| 214 | + if (registerInfo_ == nullptr) { | ||
| 195 | UBSE_LOG_WARN << "xalarm is not registered, ret: register info is null. "; | 215 | UBSE_LOG_WARN << "xalarm is not registered, ret: register info is null. "; |
| 196 | return; | 216 | return; |
| 197 | } | 217 | } |
| @@ -202,19 +222,34 @@ void UbseRasObserver::SentryEventListen() | |||
| 202 | UBSE_LOG_ERROR << "New alarm msg failed. "; | 222 | UBSE_LOG_ERROR << "New alarm msg failed. "; |
| 203 | continue; | 223 | continue; |
| 204 | } | 224 | } |
| 225 | + // 阻塞性等待,有故障事件返回。取快照后释放锁,避免长时间持锁阻塞 Stop 断开。 | ||
| 226 | + struct alarm_register* snapshot = nullptr; | ||
| 227 | + { | ||
| 228 | + std::lock_guard<std::mutex> lock(registerMtx_); | ||
| 229 | + snapshot = registerInfo_; | ||
| 230 | + } | ||
| 231 | + if (snapshot == nullptr || stopThread) { | ||
| 232 | + // 注册句柄已失效(重注册失败),或通过 while 判断后 Stop 恰好已断开 fd, | ||
| 233 | + // 直接退出循环,避免带着无效 fd 进入长时间阻塞等待 | ||
| 234 | + SafeDelete(msg); | ||
| 235 | + break; | ||
| 236 | + } | ||
| 205 | // 阻塞性等待,有故障事件返回 | 237 | // 阻塞性等待,有故障事件返回 |
| 206 | - auto ret = xalarmGetEventFunc(msg, registerInfo); | 238 | + auto ret = xalarmGetEventFunc(msg, snapshot); |
| 207 | if (ret < 0) { | 239 | if (ret < 0) { |
| 208 | UBSE_LOG_WARN << "Failed to get msg. ErrorCode=" << ret; | 240 | UBSE_LOG_WARN << "Failed to get msg. ErrorCode=" << ret; |
| 209 | const bool disconnected = ret == -ENOTCONN || ret == -EBADF; | 241 | const bool disconnected = ret == -ENOTCONN || ret == -EBADF; |
| 210 | if (disconnected) { | 242 | if (disconnected) { |
| 211 | InvalidateSysSentryConfig(); | 243 | InvalidateSysSentryConfig(); |
| 212 | } | 244 | } |
| 213 | - RegisterSentryEvent(®isterInfo); | 245 | + if (!stopThread) { // 停止中不再重注册,避免与 Stop 竞争 registerInfo_ |
| 246 | + std::lock_guard<std::mutex> lock(registerMtx_); | ||
| 247 | + RegisterSentryEvent(®isterInfo_); | ||
| 248 | + } | ||
| 214 | if (disconnected) { | 249 | if (disconnected) { |
| 215 | UBSE_LOG_INFO << "Re-config sentry"; | 250 | UBSE_LOG_INFO << "Re-config sentry"; |
| 216 | UbseConfigSysSentryWithRetry(); | 251 | UbseConfigSysSentryWithRetry(); |
| 217 | - ubse::ras::UbseRasHandler::GetInstance().ClearAllMsgId(); // 内核重插,清除msgId | 252 | + ubse::ras::UbseRasHandler::GetInstance().ClearAllMsgId(); |
| 218 | } | 253 | } |
| 219 | SafeDelete(msg); | 254 | SafeDelete(msg); |
| 220 | continue; | 255 | continue; |
| @@ -235,7 +270,10 @@ void UbseRasObserver::SentryEventListen() | |||
| 235 | SafeDelete(msg); | 270 | SafeDelete(msg); |
| 236 | TraceContext::Clear(); | 271 | TraceContext::Clear(); |
| 237 | } | 272 | } |
| 238 | - UnRegisterXalarm(®isterInfo); | 273 | + { |
| 274 | + std::lock_guard<std::mutex> lock(registerMtx_); | ||
| 275 | + UnRegisterXalarm(®isterInfo_); | ||
| 276 | + } | ||
| 239 | } | 277 | } |
| 240 | 278 | ||
| 241 | void UbseRasObserver::RegisterSentryEvent(alarm_register** registerInfo) | 279 | void UbseRasObserver::RegisterSentryEvent(alarm_register** registerInfo) |
| @@ -106,6 +106,11 @@ private: | |||
| 106 | // 完整配置成功后存在尚未处理的动态广播域变化,由配置互斥锁保护。 | 106 | // 完整配置成功后存在尚未处理的动态广播域变化,由配置互斥锁保护。 |
| 107 | bool broadcastRefreshPending = false; | 107 | bool broadcastRefreshPending = false; |
| 108 | std::mutex configSysSentryMtx; | 108 | std::mutex configSysSentryMtx; |
| 109 | + | ||
| 110 | + // xalarm 注册句柄。提升为类成员,便于 Stop 中断开底层 fd 以唤醒阻塞的 xalarmGetEventFunc。 | ||
| 111 | + // 由 registerMtx_ 保护:Stop 断开与工作线程注销可能并发;结构体仅由工作线程释放。 | ||
| 112 | + struct alarm_register* registerInfo_{nullptr}; | ||
| 113 | + std::mutex registerMtx_; | ||
| 109 | }; | 114 | }; |
| 110 | 115 | ||
| 111 | uint32_t HandleSysSentryNodeDiscoveryEvent(std::string& eventId, std::string& eventMessage); | 116 | uint32_t HandleSysSentryNodeDiscoveryEvent(std::string& eventId, std::string& eventMessage); |
| @@ -1614,6 +1614,15 @@ void ReplyWhenChannelNotInMap(UbseComMessageCtx& message, const UbseComCallback& | |||
| 1614 | { | 1614 | { |
| 1615 | UBSE_LOG_ERROR << "Reply fail, channel info is abnormal, channel id=" << message.GetChannelId() | 1615 | UBSE_LOG_ERROR << "Reply fail, channel info is abnormal, channel id=" << message.GetChannelId() |
| 1616 | << ", moduleCode=" << message.GetModuleCode() << ", opCode=" << message.GetOpCode(); | 1616 | << ", moduleCode=" << message.GetModuleCode() << ", opCode=" << message.GetOpCode(); |
| 1617 | + std::string traceId = TraceContext::GetTraceId(); | ||
| 1618 | + if (message.GetChannelPtr() == nullptr) { | ||
| 1619 | + UBSE_LOG_ERROR << "Channel is nullptr, nodeId=" << message.GetDstId(); | ||
| 1620 | + if (usrCb.cb != nullptr) { | ||
| 1621 | + usrCb.cb(usrCb.cbCtx, nullptr, 0, UBSE_COM_ERROR_CHANNEL_NULL); | ||
| 1622 | + } | ||
| 1623 | + return; | ||
| 1624 | + } | ||
| 1625 | + message.GetChannelPtr()->SetTraceId(traceId); | ||
| 1617 | UBSHcomRequest reqMsg; | 1626 | UBSHcomRequest reqMsg; |
| 1618 | auto res = (UbseReplyResultToString(UbseReplyResult::ERR_CH_NOT_IN_MAP)); | 1627 | auto res = (UbseReplyResultToString(UbseReplyResult::ERR_CH_NOT_IN_MAP)); |
| 1619 | reqMsg.address = reinterpret_cast<uint8_t*>(res.data()); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) | 1628 | reqMsg.address = reinterpret_cast<uint8_t*>(res.data()); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) |
| @@ -1624,18 +1633,13 @@ void ReplyWhenChannelNotInMap(UbseComMessageCtx& message, const UbseComCallback& | |||
| 1624 | return; | 1633 | return; |
| 1625 | } | 1634 | } |
| 1626 | UBSHcomReplyContext replyContext(message.GetRspCtx(), 0); | 1635 | UBSHcomReplyContext replyContext(message.GetRspCtx(), 0); |
| 1627 | - std::string traceId = TraceContext::GetTraceId(); | ||
| 1628 | - if (message.GetChannelPtr() == nullptr) { | ||
| 1629 | - UBSE_LOG_ERROR << "Channel is nullptr, nodeId=" << message.GetDstId(); | ||
| 1630 | - usrCb.cb(usrCb.cbCtx, nullptr, 0, UBSE_COM_ERROR_CHANNEL_NULL); | ||
| 1631 | - return; | ||
| 1632 | - } | ||
| 1633 | - message.GetChannelPtr()->SetTraceId(traceId); | ||
| 1634 | auto ret = message.GetChannelPtr()->Reply(replyContext, reqMsg, done); | 1636 | auto ret = message.GetChannelPtr()->Reply(replyContext, reqMsg, done); |
| 1635 | if (ret != UBSE_OK) { | 1637 | if (ret != UBSE_OK) { |
| 1636 | UBSE_LOG_ERROR << "Channel reply failed, " << FormatRetCode(ret) << ", moduleCode=" << message.GetModuleCode() | 1638 | UBSE_LOG_ERROR << "Channel reply failed, " << FormatRetCode(ret) << ", moduleCode=" << message.GetModuleCode() |
| 1637 | << ", opCode=" << message.GetOpCode(); | 1639 | << ", opCode=" << message.GetOpCode(); |
| 1638 | - usrCb.cb(usrCb.cbCtx, nullptr, 0, UBSE_COM_ERROR_REPLY_FAIL); | 1640 | + if (usrCb.cb != nullptr) { |
| 1641 | + usrCb.cb(usrCb.cbCtx, nullptr, 0, UBSE_COM_ERROR_REPLY_FAIL); | ||
| 1642 | + } | ||
| 1639 | } else { | 1643 | } else { |
| 1640 | UBSE_LOG_DEBUG << "Channel reply successfully, moduleCode=" << message.GetModuleCode() | 1644 | UBSE_LOG_DEBUG << "Channel reply successfully, moduleCode=" << message.GetModuleCode() |
| 1641 | << ", opCode=" << message.GetOpCode(); | 1645 | << ", opCode=" << message.GetOpCode(); |
| @@ -335,6 +335,18 @@ public: | |||
| 335 | } | 335 | } |
| 336 | } | 336 | } |
| 337 | 337 | ||
| 338 | + /* | ||
| 339 | + * @brief Try to dequeue an item without blocking | ||
| 340 | + * | ||
| 341 | + * @param item [out] item dequeued from front | ||
| 342 | + * | ||
| 343 | + * @return true if successful, false if queue is empty | ||
| 344 | + */ | ||
| 345 | + inline bool TryDequeue(T& item) | ||
| 346 | + { | ||
| 347 | + return mRingBuffer_.PopFront(item); | ||
| 348 | + } | ||
| 349 | + | ||
| 338 | private: | 350 | private: |
| 339 | RingBuffer<T> mRingBuffer_; /* ring buffer to data store */ | 351 | RingBuffer<T> mRingBuffer_; /* ring buffer to data store */ |
| 340 | sem_t mSem_{}; /* semaphore to wait and notify */ | 352 | sem_t mSem_{}; /* semaphore to wait and notify */ |
| @@ -192,6 +192,17 @@ void UbseTaskExecutor::Stop() | |||
| 192 | UBSE_LOG_INFO << "Wait for the thread to exit."; | 192 | UBSE_LOG_INFO << "Wait for the thread to exit."; |
| 193 | mStopped = true; | 193 | mStopped = true; |
| 194 | mStarted = false; | 194 | mStarted = false; |
| 195 | + | ||
| 196 | + // 清理队列中残留的任务(Stop 期间入队但未被工作线程消费的任务),避免泄漏。 | ||
| 197 | + // 此时所有工作线程已 join 退出,无并发 Dequeue;持有 mtx 锁,无新任务入队。 | ||
| 198 | + // 仅释放任务对象本身,不执行 lambda,避免触发已停止模块的依赖。 | ||
| 199 | + UbseRunnable* task = nullptr; | ||
| 200 | + while (mRunnableQueue.TryDequeue(task)) { | ||
| 201 | + if (task != nullptr) { | ||
| 202 | + task->DecreaseRef(); | ||
| 203 | + } | ||
| 204 | + } | ||
| 205 | + | ||
| 195 | mRunnableQueue.UnInitialize(); | 206 | mRunnableQueue.UnInitialize(); |
| 196 | UBSE_LOG_INFO << "Stop TaskExecutor end"; | 207 | UBSE_LOG_INFO << "Stop TaskExecutor end"; |
| 197 | } | 208 | } |
检视意见ID: S1-01
[S1-01] registerInfo_ 快照指针跨锁释放后被 Stop 释放,存在 use-after-free/double-free 风险
registerInfo_快照后释放锁,跨 so 边界将裸指针传入阻塞的xalarmGetEventFunc,而Stop在同一把锁内调用UnRegisterXalarm释放该结构体,快照成为悬垂指针。详细描述: 本提交将
registerInfo_提升为类成员并用registerMtx_保护。SentryEventListen在锁内取快照snapshot = registerInfo_后释放锁,再将裸指针snapshot跨 so 边界传入阻塞调用xalarmGetEventFunc(msg, snapshot)(line 223)。而Stop在同一把锁内调用UnRegisterXalarm(®isterInfo_)(line 174),后者依 xalarm API 契约(xalarm_unregister_event(struct alarm_register**)取二级指针,且仓库内 stubtest/IT/stubs/xalarm_stub.cpp:229明确free(*register_info)并置空)会释放该结构体。由于快照在锁释放后才被使用,Stop可在快照仍被工作线程持有时将其指向的结构体释放:xalarmGetEventFunc入口读取register_info->register_fd(stub line 155)之间存在窗口,Stop恰在此间释放结构体则该次解引用为 heap-use-after-free(asan 可检出)。xalarm_get_event在阻塞返回路径再次读取register_info字段,则在常见的"工作线程阻塞、Stop 注销"场景下即触发 UAF。此外,错误路径的重注册(line 230
RegisterSentryEvent(®isterInfo_))与循环末尾注销(line 255UnRegisterXalarm(®isterInfo_))均未持有registerMtx_,与头文件"由 registerMtx_ 保护"声明不一致,在Stop与工作线程错误恢复并发时存在对registerInfo_的 TOCTOU/double-free 竞争。后果为 ubse daemon 崩溃(UAF/double-free),触发路径为本地 Stop(关停/故障切换/配置重载),属"需前置条件触发严重后果"。若能确认生产 libxalarm 的xalarm_unregister_event不释放结构体(仅断开),则本条风险解除,但 stub 与 API 签名均指向"释放",当前无法排除。// 原始代码(src/adapter_plugins/syssentry/sentry_observer.cpp) // 工作线程:锁内取快照后释放锁,再用裸指针跨 so 阻塞调用 struct alarm_register* snapshot = nullptr; { std::lock_guard<std::mutex> lock(registerMtx_); snapshot = registerInfo_; } if (snapshot == nullptr) { SafeDelete(msg); break; } auto ret = xalarmGetEventFunc(msg, snapshot); // 释放锁后用裸指针跨 so 调用,Stop 可能已释放该结构体 // ... // Stop:锁内释放结构体后立即 join void UbseRasObserver::Stop() { stopThread = true; // ... { std::lock_guard<std::mutex> lock(registerMtx_); UnRegisterXalarm(®isterInfo_); // xalarmUnRegisterFunc 会 free(registerInfo_) 并置空 } if (worker && worker->joinable()) { worker->join(); worker.reset(); } }修改方案: 根因是裸指针快照跨越锁的生命周期、且
xalarm_unregister_event既断开又释放。修复应将"断开以唤醒"与"释放结构体"解耦:Stop只关闭底层 fd 唤醒阻塞调用,不释放结构体;结构体由工作线程退出时统一释放。同时工作线程在停止态不再重注册,且所有registerInfo_访问纳入registerMtx_保护。// 修改后代码(Stop:仅断开 fd 唤醒,不释放结构体) void UbseRasObserver::Stop() { stopThread = true; InvalidateSysSentryConfig(); UnregisterConfigRetryTimer(); ubse::timer::UbseTimerHandlerUnregister(UBSE_RAS_QUERY_MSG_MONITOR_TIMER_NAME); // 仅关闭底层 fd 唤醒阻塞在 xalarmGetEventFunc 的工作线程,不释放 alarm_register, // 避免工作线程持有的快照成为悬垂指针;结构体由工作线程退出时统一释放。 int fdToClose = -1; { std::lock_guard<std::mutex> lock(registerMtx_); if (registerInfo_ != nullptr) { fdToClose = registerInfo_->register_fd; registerInfo_->register_fd = -1; // 标记已断开,防止工作线程重用 } } if (fdToClose >= 0) { close(fdToClose); } if (worker && worker->joinable()) { worker->join(); worker.reset(); } } // 修改后代码(SentryEventListen:停止态不再重注册,且重注册纳入锁保护) auto ret = xalarmGetEventFunc(msg, snapshot); if (ret < 0) { UBSE_LOG_WARN << "Failed to get msg. ErrorCode=" << ret; const bool disconnected = ret == -ENOTCONN || ret == -EBADF; if (disconnected) { InvalidateSysSentryConfig(); } if (!stopThread) { // 停止中不再重注册,避免与 Stop 竞争 registerInfo_ std::lock_guard<std::mutex> lock(registerMtx_); RegisterSentryEvent(®isterInfo_); } if (disconnected) { UBSE_LOG_INFO << "Re-config sentry"; UbseConfigSysSentryWithRetry(); ubse::ras::UbseRasHandler::GetInstance().ClearAllMsgId(); } SafeDelete(msg); continue; } // ... // 循环末尾注销同样纳入锁保护 { std::lock_guard<std::mutex> lock(registerMtx_); UnRegisterXalarm(®isterInfo_); }