已开启
检视意见修改 #12527
检视意见修改 #12527
已开启
wangrui7创建于 27 天前
共 7 个文件变更+52-36
@@ -917,7 +917,7 @@ void BusCenterClientProxy::OnConversationRecvMsg(const ConversationBusiness *inf
917 }917 }
918 int32_t serverRet = 0;918 int32_t serverRet = 0;
919 if (!reply.ReadInt32(serverRet)) {919 if (!reply.ReadInt32(serverRet)) {
920- LNN_LOGE(LNN_EVENT, "read serverRet failed! serverRet=%{public}d", serverRet);920+ LNN_LOGE(LNN_EVENT, "read serverRet failed!");
921 }921 }
922 return;922 return;
923}923}
@@ -24,7 +24,7 @@
24extern "C" {24extern "C" {
25#endif25#endif
26 26 
27-#define MAX_MSG_ID (0xFFFFFFFF)27+#define MAX_FRAGMENT_CONTEXT_NUM 1000
28 28 
29typedef struct {29typedef struct {
30 uint32_t msgId;30 uint32_t msgId;
@@ -26,7 +26,7 @@
26extern "C" {26extern "C" {
27#endif27#endif
28 28 
29-void FragmentRecvInit(void);29+int32_t FragmentRecvInit(void);
30 30 
31void FragmentRecvDeinit(void);31void FragmentRecvDeinit(void);
32 32 
@@ -28,11 +28,11 @@
28#include "softbus_adapter_thread.h"28#include "softbus_adapter_thread.h"
29#include "softbus_common.h"29#include "softbus_common.h"
30#include "softbus_error_code.h"30#include "softbus_error_code.h"
xuhengxiang
xuhengxiangxuhengxiang3 天前

删除了宏定义 MAX_FRAGMENT_CONTEXT_NUM (5000),但如果该宏在当前文件或其他依赖文件中仍被引用(例如用于限制分片上下文数量的边界检查),将导致编译失败。即使宏的使用处也被同步删除,若原本基于该宏的资源上限检查被移除,则可能导致分片上下文数量无限制增长,引发内存耗尽或资源泄漏。

确认 MAX_FRAGMENT_CONTEXT_NUM 的所有使用点是否已同步移除或替换为等价的限制机制。若该宏用于资源上限保护,必须保留相应的边界检查逻辑,防止无限制分配分片上下文。

likedislike
31+#include "softbus_agent_communication.h"
31#include "g_enhance_lnn_func_pack.h"32#include "g_enhance_lnn_func_pack.h"
32 33 
33-#define MAX_FRAGMENT_CONTEXT_NUM 5000
34#define MAX_FRAGMENT_NUM 102434#define MAX_FRAGMENT_NUM 1024
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang24 天前

删除了 MAX_FRAGMENT_CONTEXT_NUM 宏定义。如果该宏之前用于限制并发分片上下文的最大数量以防止内存耗尽攻击,删除后可能导致无限制创建上下文,引发拒绝服务风险。

确认该宏确实已不再使用,且系统中有其他机制限制分片上下文的创建数量,防止内存耗尽。

likedislike
wangrui7
wangrui7
24 天前 评论:
xuhengxiangxuhengxiang24 天前

删除了 MAX_FRAGMENT_CONTEXT_NUM 宏定义。如果该宏之前用于限制并发分片上下文的最大数量以防止内存耗尽攻击,删除后可能导致无限制创建上下文,引发拒绝服务风险。

确认该宏确实已不再使用,且系统中有其他机制限制分片上下文的创建数量,防止内存耗尽。

likedislike
xuhengxiangxuhengxiang24 天前

删除了 MAX_FRAGMENT_CONTEXT_NUM 宏定义。如果该宏之前用于限制并发分片上下文的最大数量以防止内存耗尽攻击,删除后可能导致无限制创建上下文,引发拒绝服务风险。

确认该宏确实已不再使用,且系统中有其他机制限制分片上下文的创建数量,防止内存耗尽。

likedislike
xuhengxiangxuhengxiang24 天前

移除了MAX_FRAGMENT_CONTEXT_NUM(5000)限制宏定义。如果该宏此前用于限制同时存在的分片上下文数量,移除后可能导致无限制创建上下文,造成内存耗尽。需确认该限制是否已在其他位置实现。

确认分片上下文数量限制是否已迁移到其他位置;若未迁移,应保留对上下文数量的上限检查,防止内存耗尽攻击。

likedislike
35-#define MAX_ASSEMBLED_LEN (10 * 1024 * 1024) // 10MB35+#define MAX_ASSEMBLED_LEN (COMMUNICATION_DATA_MAX_LEN + 1024) // 11K
36#define BASE_RANDOM_ID 1000036#define BASE_RANDOM_ID 10000
37#define BASE_RANDOM_BIT_LEN 1437#define BASE_RANDOM_BIT_LEN 14
38#define BIT_14_MASK 0x3FFF38#define BIT_14_MASK 0x3FFF
@@ -423,18 +423,8 @@ static int32_t ValidateAndParseFragment(const uint8_t *data, uint32_t dataLen,
423 return SOFTBUS_OK;423 return SOFTBUS_OK;
424}424}
425 425 
426-static int32_t FindOrCreateFragmentContext(const DataFragmentInfo *header, FragmentContext **ctx)426+static int32_t CreateFragmentContextFromHeader(const DataFragmentInfo *header, FragmentContext **ctx)
427{427{
428- int32_t ret = FindFragmentContext(header->msgId, ctx);
429- if (ret == SOFTBUS_OK) {
430- if ((*ctx)->total != header->total) {
431- LNN_LOGE(LNN_EVENT, "total mismatch, ctxTotal=%{public}u, headerTotal=%{public}u",
432- (*ctx)->total, header->total);
433- return SOFTBUS_INVALID_PARAM;
434- }
435- return SOFTBUS_OK;
436- }
437- 
438 uint32_t sliceTotal = (header->total + MAX_SLICE_LEN - 1) / MAX_SLICE_LEN;428 uint32_t sliceTotal = (header->total + MAX_SLICE_LEN - 1) / MAX_SLICE_LEN;
439 if (sliceTotal == 0 || sliceTotal > MAX_FRAGMENT_NUM) {429 if (sliceTotal == 0 || sliceTotal > MAX_FRAGMENT_NUM) {
440 LNN_LOGE(LNN_EVENT, "sliceTotal=%{public}u invalid", sliceTotal);430 LNN_LOGE(LNN_EVENT, "sliceTotal=%{public}u invalid", sliceTotal);
@@ -490,18 +480,28 @@ static int32_t ProcessFragmentWithLock(const ProcessFragmentInput *input)
490 LNN_LOGE(LNN_EVENT, "lock fragment mutex failed");480 LNN_LOGE(LNN_EVENT, "lock fragment mutex failed");
491 return SOFTBUS_LOCK_ERR;481 return SOFTBUS_LOCK_ERR;
492 }482 }
493- uint32_t count = 0;
494- CleanupTimeoutFragmentContexts(&count);
495- if (count > MAX_FRAGMENT_CONTEXT_NUM) {
496- LNN_LOGE(LNN_EVENT, "avoid resource exhaution in short time, count=%{public}u", count);
497- (void)SoftBusMutexUnlock(&g_fragmentMutex);
498- return SOFTBUS_INVALID_PARAM;
499- }
500 FragmentContext *ctx = NULL;483 FragmentContext *ctx = NULL;
501- int32_t ret = FindOrCreateFragmentContext(input->header, &ctx);484+ int32_t ret = FindFragmentContext(input->header->msgId, &ctx);
502- if (ret != SOFTBUS_OK || ctx == NULL) {485+ if (ret == SOFTBUS_OK) {
503- (void)SoftBusMutexUnlock(&g_fragmentMutex);486+ if (ctx->total != input->header->total) {
504- return ret;487+ LNN_LOGE(LNN_EVENT, "total mismatch, ctxTotal=%{public}u, headerTotal=%{public}u",
488+ ctx->total, input->header->total);
489+ (void)SoftBusMutexUnlock(&g_fragmentMutex);
490+ return SOFTBUS_INVALID_PARAM;
491+ }
492+ } else {
493+ uint32_t count = 0;
494+ CleanupTimeoutFragmentContexts(&count);
495+ if (count > MAX_FRAGMENT_CONTEXT_NUM) {
496+ LNN_LOGE(LNN_EVENT, "avoid resource exhaution in short time, count=%{public}u", count);
497+ (void)SoftBusMutexUnlock(&g_fragmentMutex);
498+ return SOFTBUS_DDOS_MSG_EXCEED_LIMIT;
499+ }
500+ ret = CreateFragmentContextFromHeader(input->header, &ctx);
501+ if (ret != SOFTBUS_OK || ctx == NULL) {
502+ (void)SoftBusMutexUnlock(&g_fragmentMutex);
503+ return ret;
504+ }
505 }505 }
506 if (input->header->offset + input->header->size > ctx->total) {506 if (input->header->offset + input->header->size > ctx->total) {
507 LNN_LOGE(LNN_EVENT, "offset=%{public}u, size=%{public}u, total=%{public}u",507 LNN_LOGE(LNN_EVENT, "offset=%{public}u, size=%{public}u, total=%{public}u",
@@ -49,29 +49,26 @@ static ListNode g_fragmentList;
49static SoftBusMutex g_fragmentMutex = {0};49static SoftBusMutex g_fragmentMutex = {0};
50static bool g_isInit = false;50static bool g_isInit = false;
51 51 
52-void FragmentRecvInit(void)52+int32_t FragmentRecvInit(void)
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang24 天前

FragmentRecvInit移除了双重检查锁定(Double-Checked Locking)。原代码在加锁后再次检查g_isInit,新代码仅在外部无锁检查g_isInit。当两个线程同时调用FragmentRecvInit且g_isInit为false时,两个线程都会执行ListInit和SoftBusMutexInit,导致列表被重复初始化(数据丢失/损坏)和互斥锁被重复初始化(状态不确定)。

恢复双重检查锁定模式:在SoftBusMutexLock成功后,再次检查g_isInit,如果已为true则解锁并返回SOFTBUS_OK。或者使用单独的初始化锁保护整个初始化过程。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit存在竞态条件:g_isInit的检查、ListInit和SoftBusMutexInit均在锁外执行。两个线程并发调用时,可能同时通过g_isInit==false检查,导致ListInit重置链表(丢失已有分片上下文)和SoftBusMutexInit重复初始化互斥锁(未定义行为或资源泄漏)。旧代码在锁内有二次检查作为部分缓解,此次变更移除了该检查,使问题加剧。

将ListInit和SoftBusMutexInit移入锁内保护,或在初始化前使用原子操作/CAS检查g_isInit,确保只有一个线程执行初始化。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit存在竞态条件:g_isInit的检查在锁外进行,且ListInit和SoftBusMutexInit也在无锁保护下执行。原代码在锁内有二次检查(double-check)被移除,导致两个线程可同时通过首次检查,并发执行ListInit和SoftBusMutexInit,造成链表数据损坏和互斥锁重复初始化(未定义行为)。

在锁内保留g_isInit的二次检查,或将整个初始化逻辑用全局锁保护,确保ListInit和SoftBusMutexInit只执行一次。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 存在线程安全问题:g_isInit 的检查、ListInit 和 SoftBusMutexInit 均在锁外执行。原代码通过锁内二次检查 g_isInit 来缓解并发初始化问题,此次变更移除了该二次检查。若两个线程同时调用 FragmentRecvInit,两者均可通过首次 g_isInit 检查,随后都执行 ListInit(清空已有链表,导致数据丢失)和 SoftBusMutexInit(重新初始化正在使用的互斥锁,导致未定义行为或死锁)。

将 ListInit 和 SoftBusMutexInit 移入锁内保护,或使用原子操作保证 g_isInit 的可见性,确保初始化只执行一次。例如使用静态初始化互斥锁或将整个初始化逻辑置于锁内,并在锁内重新检查 g_isInit。

likedislike
53{53{
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang24 天前

FragmentRecvInit 函数在无锁状态下检查 g_isInit 后,直接执行 ListInit 和 SoftBusMutexInit,移除了原代码中锁内的二次检查。如果多个线程同时调用此函数(例如通过 FragmentRecvProcess 并发调用),可能导致竞态条件,重复初始化链表和互斥锁,引发内存损坏或资源泄漏。

恢复双重检查锁定模式,在获取锁后再次检查 g_isInit,或者使用原子操作确保线程安全。

likedislike
xuhengxiangxuhengxiang24 天前

FragmentRecvInit 函数移除了原代码中的 Double-Checked Locking 机制(锁后二次检查 g_isInit)。在多线程并发调用时,多个线程可能同时通过 g_isInit 的初始检查,导致 ListInit 和 SoftBusMutexInit 被重复执行。重复初始化互斥锁会导致未定义行为或死锁。

恢复锁后的 g_isInit 检查,或者使用原子操作保护初始化标志,确保初始化过程只执行一次。

likedislike
54 if (g_isInit) {54 if (g_isInit) {
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang24 天前

FragmentRecvInit 函数存在并发初始化的竞态条件。移除了原代码在获取互斥锁后的二次检查(Double-Checked Locking),导致如果多个线程同时调用该函数并都通过外层的 if (!g_isInit) 检查,会重复执行 ListInit 和 SoftBusMutexInit,引发内存损坏、死锁或未定义行为。

在成功获取互斥锁后,恢复对 g_isInit 的二次检查,确保初始化逻辑在并发环境下只执行一次。

likedislike
xuhengxiangxuhengxiang24 天前

FragmentRecvInit 函数存在并发初始化的竞态条件。移除了原代码在获取互斥锁后的二次检查(Double-Checked Locking),导致如果多个线程同时调用该函数并都通过外层的 if (!g_isInit) 检查,会重复执行 ListInit 和 SoftBusMutexInit,引发内存损坏、死锁或未定义行为。

在成功获取互斥锁后,恢复对 g_isInit 的二次检查,确保初始化逻辑在并发环境下只执行一次。

likedislike
55- return;55+ return SOFTBUS_OK;
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang24 天前

FragmentRecvInit 函数移除了锁内的第二次检查(double-checked locking),导致并发调用时存在竞态条件。多个线程可能同时通过 g_isInit 的检查,导致 ListInit 和 SoftBusMutexInit 被重复执行,从而引发链表损坏和互斥锁状态异常。

恢复锁内的第二次检查,确保初始化过程只执行一次。或者在模块加载时进行初始化,避免运行时并发初始化。

likedislike
xuhengxiangxuhengxiang24 天前

FragmentRecvInit函数移除了双重检查锁定机制。在多线程并发调用时,如果两个线程同时通过g_isInit检查,会重复执行ListInit和SoftBusMutexInit,导致g_fragmentList中的数据丢失(内存泄漏)以及g_fragmentMutex重复初始化(资源泄漏或未定义行为)。

恢复双重检查锁定机制,在获取锁后再次检查g_isInit状态;或者使用原子操作保证g_isInit的读写安全,并确保初始化操作的幂等性。

likedislike
56 }56 }
57 ListInit(&g_fragmentList);57 ListInit(&g_fragmentList);
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中 ListInit 和 SoftBusMutexInit 在互斥锁保护之外执行,且移除了锁内的二次检查。多线程同时调用 FragmentRecvProcess 时,两个线程可能同时通过 g_isInit 的 false 检查,导致 ListInit 重复执行(丢失已存在的分片数据)和 SoftBusMutexInit 重复初始化已初始化的互斥锁(未定义行为),可能引发数据损坏和崩溃。

将 ListInit 和 SoftBusMutexInit 移入互斥锁保护范围内,或在初始化阶段使用单独的初始化保证(如原子操作或外部同步),确保全局只初始化一次。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中移除了锁内对 g_isInit 的二次检查(double-check)。首个检查在无锁状态下进行,两个线程可同时通过该检查并进入初始化路径,导致 ListInit 被重复调用(销毁已有分片链表、丢失数据),以及 SoftBusMutexInit 对正在使用的互斥锁重复初始化(未定义行为),造成数据损坏和线程安全问题。

在 SoftBusMutexLock 成功后、设置 g_isInit=true 之前,恢复对 g_isInit 的二次检查:if (g_isInit) { SoftBusMutexUnlock(&g_fragmentMutex); return SOFTBUS_OK; }

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中移除了锁内的 g_isInit 二次检查(double-check),导致竞态条件:两个线程可同时通过首次无锁检查(g_isInit==false),随后都执行 ListInit 和 SoftBusMutexInit,造成已初始化的互斥锁被重复初始化(未定义行为)以及链表被重置(数据丢失/内存泄漏)。

在 SoftBusMutexLock 成功后、设置 g_isInit=true 之前,恢复 g_isInit 的二次检查:if (g_isInit) { SoftBusMutexUnlock(&g_fragmentMutex); return SOFTBUS_OK; }

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中移除了锁内的二次检查(double-check)。当两个线程同时通过无锁的 g_isInit 检查(均为 false)后,会依次获取锁并重复执行 ListInit 和 SoftBusMutexInit。这会导致:(1) 正在被其他线程使用的互斥锁被重新初始化,引发未定义行为或崩溃;(2) 已添加的分片上下文列表被 ListInit 清空,造成数据丢失。

在获取锁后恢复对 g_isInit 的二次检查:lock 成功后若 g_isInit 已为 true,则解锁并直接返回 SOFTBUS_OK,避免重复初始化。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中移除了双重检查锁定模式。g_isInit 的首次检查在锁外进行,但锁内不再二次检查 g_isInit。若两个线程同时通过首次检查,将并发执行 ListInit(破坏已存在的链表节点)和 SoftBusMutexInit(对已初始化的互斥锁重复初始化,导致资源泄漏或未定义行为),造成数据损坏和线程安全问题。

在 SoftBusMutexLock 成功后、设置 g_isInit=true 之前,恢复双重检查:if (g_isInit) { SoftBusMutexUnlock(&g_fragmentMutex); return SOFTBUS_OK; }

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit()移除了锁内双重检查(double-check),引入线程安全回归。原代码在获取锁后再次检查g_isInit,新代码删除了该检查。当多个线程同时调用FragmentRecvInit()时(例如多个线程并发调用FragmentRecvProcess()),两个线程都可能通过第一次g_isInit检查,随后都执行ListInit和SoftBusMutexInit,导致链表被重复初始化(若已有节点则数据损坏)和互斥锁被重复初始化(未定义行为),最终两个线程都设置g_isInit=true。

在获取锁后恢复双重检查:在SoftBusMutexLock成功后、设置g_isInit之前,再次检查g_isInit,若已为true则解锁并返回SOFTBUS_OK。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中移除了双重检查锁定(double-check locking)。当两个线程同时调用 FragmentRecvInit 时,两者都可能通过第一道 g_isInit 检查(无锁),随后都执行 ListInit 和 SoftBusMutexInit,导致链表被重新初始化(丢失已有数据)且互斥锁被重复初始化(未定义行为)。原代码在锁内有第二次 g_isInit 检查来防止此竞态,新代码将其删除。

在 SoftBusMutexLock 成功后、设置 g_isInit=true 之前,恢复双重检查:再次检查 g_isInit,若已为 true 则解锁并返回 SOFTBUS_OK。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中对 g_isInit 的检查未加锁,存在 TOCTOU 竞态。两个线程可同时通过 g_isInit==false 检查,随后都调用 ListInit 和 SoftBusMutexInit,导致已存在的分片链表被重置(数据丢失)以及互斥锁被重复初始化(资源泄漏/未定义行为)。原代码中的锁内二次检查被移除,进一步降低了保护。

在首次检查后直接加锁,或在 SoftBusMutexInit 之前加锁(需确保互斥锁已初始化),使用单一加锁路径保护整个初始化过程,避免 ListInit 和 MutexInit 被并发调用。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中 SoftBusMutexInit 成功后若 SoftBusMutexLock 失败,函数直接返回错误,未调用 SoftBusMutexDestroy 清理已初始化的互斥锁。由于 g_isInit 仍为 false,后续调用会再次执行 SoftBusMutexInit,对已初始化的互斥锁重复初始化,在 POSIX 下属于未定义行为,可能导致资源泄漏或崩溃。

在 SoftBusMutexLock 失败的分支中,先调用 SoftBusMutexDestroy(&g_fragmentMutex) 清理互斥锁再返回错误。

likedislike
xuhengxiangxuhengxiang3 天前

FragmentRecvInit 中移除了锁内双重检查(double-checked locking)。g_isInit 的首次检查在无锁状态下进行,若两个线程同时调用 FragmentRecvInit(例如通过 FragmentRecvProcess 并发处理数据),两者均可通过首次检查,导致 ListInit 被重复调用(重置链表,丢失已有的分片上下文数据)以及 SoftBusMutexInit 被重复调用(对可能正在使用的互斥锁重新初始化,行为未定义)。

在获取锁后、设置 g_isInit 之前恢复对 g_isInit 的二次检查,确保仅有一个线程执行初始化逻辑;或将 ListInit 和 SoftBusMutexInit 移入锁保护范围内。

likedislike
58 SoftBusMutexAttr mutexAttr;58 SoftBusMutexAttr mutexAttr;
xuhengxiang
xuhengxiangxuhengxiang24 天前

FragmentRecvInit 函数存在线程安全问题。移除了加锁后的二次检查(Double-Checked Locking),在并发调用时可能导致 SoftBusMutexInit 和 ListInit 被多次执行,引发未定义行为或互斥锁状态损坏。

在获取锁后再次检查 g_isInit 标志,确保初始化逻辑只执行一次。

likedislike
59 (void)SoftBusMutexAttrInit(&mutexAttr);59 (void)SoftBusMutexAttrInit(&mutexAttr);
60 if (SoftBusMutexInit(&g_fragmentMutex, &mutexAttr) != SOFTBUS_OK) {60 if (SoftBusMutexInit(&g_fragmentMutex, &mutexAttr) != SOFTBUS_OK) {
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang24 天前

当SoftBusMutexLock失败时返回SOFTBUS_LOCK_ERR,但g_isInit仍为false。下次调用FragmentRecvInit会再次执行ListInit和SoftBusMutexInit。如果第一次SoftBusMutexInit已成功,重复调用SoftBusMutexInit可能导致互斥锁状态异常(取决于实现);重复调用ListInit会清空已有数据。

区分处理:如果SoftBusMutexInit成功但SoftBusMutexLock失败,不应在下次调用时重复初始化互斥锁。可引入单独的状态标志记录互斥锁是否已初始化,或在Lock失败时设置g_isInit=true(互斥锁已可用,只是加锁失败)。

likedislike
xuhengxiangxuhengxiang3 天前

当SoftBusMutexInit成功但SoftBusMutexLock失败时,函数直接返回SOFTBUS_LOCK_ERR,但g_isInit仍为false。下次调用时会再次执行SoftBusMutexInit,对已初始化的互斥锁重复初始化而不先销毁,导致资源泄漏和未定义行为(pthread_mutex_init对已初始化互斥锁的行为是未定义的)。

在SoftBusMutexLock失败时,应先销毁已初始化的互斥锁(SoftBusMutexDestroy)再返回错误,或设置标志位避免重复初始化。

likedislike
xuhengxiangxuhengxiang3 天前

当 SoftBusMutexInit 成功但 SoftBusMutexLock 失败时,函数直接返回 SOFTBUS_LOCK_ERR,但已初始化的 g_fragmentMutex 未被销毁。由于 g_isInit 仍为 false,后续调用会再次执行 SoftBusMutexInit 对同一互斥锁重复初始化,可能导致资源泄漏和未定义行为。

在 SoftBusMutexLock 失败的分支中调用 SoftBusMutexDestroy 销毁已初始化的互斥锁后再返回。

likedislike
61 LNN_LOGE(LNN_EVENT, "init fragment mutex failed");61 LNN_LOGE(LNN_EVENT, "init fragment mutex failed");
62- return;62+ return SOFTBUS_NO_INIT;
63 }63 }
64 if (SoftBusMutexLock(&g_fragmentMutex) != SOFTBUS_OK) {64 if (SoftBusMutexLock(&g_fragmentMutex) != SOFTBUS_OK) {
65 LNN_LOGE(LNN_EVENT, "lock fragment mutex failed");65 LNN_LOGE(LNN_EVENT, "lock fragment mutex failed");
xuhengxiangxuhengxiang
xuhengxiangxuhengxiang24 天前

在 FragmentRecvInit 中,如果 SoftBusMutexInit 成功但 SoftBusMutexLock 失败,函数直接返回错误,但没有销毁已初始化的 g_fragmentMutex,且 g_isInit 仍为 false。这会导致后续调用时重复执行 SoftBusMutexInit,造成资源泄漏或未定义行为。

在 SoftBusMutexLock 失败时,应调用 SoftBusMutexDestroy 销毁已初始化的互斥锁,或者设置标志表示互斥锁已初始化,避免重复初始化。

likedislike
xuhengxiangxuhengxiang24 天前

FragmentRecvInit函数中,如果SoftBusMutexLock失败,已通过SoftBusMutexInit初始化的g_fragmentMutex未被销毁。由于g_isInit未设置为true,下次调用时会再次执行SoftBusMutexInit,导致原mutex资源泄漏。同样,ListInit也会被重复执行,可能导致已有链表数据丢失。

在SoftBusMutexLock失败时,调用相应的销毁函数释放已初始化的mutex资源;或在重新初始化前检查并销毁旧资源。

likedislike
66- return;66+ return SOFTBUS_LOCK_ERR;
67- }
68- if (g_isInit) {
69- SoftBusMutexUnlock(&g_fragmentMutex);
70- return;
71 }67 }
72 g_isInit = true;68 g_isInit = true;
73 SoftBusMutexUnlock(&g_fragmentMutex);69 SoftBusMutexUnlock(&g_fragmentMutex);
74 LNN_LOGI(LNN_EVENT, "fragment recv init success");70 LNN_LOGI(LNN_EVENT, "fragment recv init success");
71+ return SOFTBUS_OK;
75}72}
76 73 
77void FragmentRecvDeinit(void)74void FragmentRecvDeinit(void)
@@ -182,6 +179,16 @@ static int32_t GetOrCreateFragmentContext(uint32_t msgId, uint32_t moduleType)
182 }179 }
183 FragmentRecvContext *ctx = FindFragmentContext(msgId);180 FragmentRecvContext *ctx = FindFragmentContext(msgId);
184 if (ctx == NULL) {181 if (ctx == NULL) {
182+ int32_t count = 0;
183+ FragmentRecvContext *item = NULL;
184+ LIST_FOR_EACH_ENTRY(item, &g_fragmentList, FragmentRecvContext, node) {
xuhengxiang
xuhengxiangxuhengxiang24 天前

在 GetOrCreateFragmentContext 中,通过遍历整个链表来计算节点数量以判断是否超过上限。这在持有锁的情况下执行,如果链表较长,会导致锁持有时间过长,影响并发性能。

建议维护一个全局的计数变量记录当前 fragment context 的数量,在创建和销毁时进行增减,避免每次都遍历链表。

likedislike
185+ count++;
186+ }
187+ if (count > MAX_FRAGMENT_CONTEXT_NUM) {
188+ LNN_LOGE(LNN_EVENT, "fragment context num exceeds limit, count=%{public}d", count);
xuhengxiang
xuhengxiangxuhengxiang24 天前

GetOrCreateFragmentContext函数中,count变量类型为uint32_t,但日志打印使用%{public}d格式化字符串,类型不匹配,可能导致日志显示错误。

将日志格式化字符串改为%{public}u以匹配uint32_t类型。

likedislike
189+ SoftBusMutexUnlock(&g_fragmentMutex);
190+ return SOFTBUS_DDOS_MSG_EXCEED_LIMIT;
191+ }
185 ctx = CreateFragmentContext(msgId, moduleType);192 ctx = CreateFragmentContext(msgId, moduleType);
186 if (ctx == NULL) {193 if (ctx == NULL) {
187 SoftBusMutexUnlock(&g_fragmentMutex);194 SoftBusMutexUnlock(&g_fragmentMutex);
@@ -281,7 +288,10 @@ int32_t FragmentRecvProcess(const char *udid, const uint8_t *data, uint32_t data
281 return SOFTBUS_INVALID_PARAM;288 return SOFTBUS_INVALID_PARAM;
282 }289 }
283 290 
284- FragmentRecvInit();291+ if (FragmentRecvInit() != SOFTBUS_OK) {
292+ LNN_LOGE(LNN_EVENT, "fragment recv init failed");
293+ return SOFTBUS_NO_INIT;
294+ }
285 295 
286 if (SoftBusMutexLock(&g_fragmentMutex) != SOFTBUS_OK) {296 if (SoftBusMutexLock(&g_fragmentMutex) != SOFTBUS_OK) {
287 LNN_LOGE(LNN_EVENT, "lock fragment mutex failed");297 LNN_LOGE(LNN_EVENT, "lock fragment mutex failed");
@@ -238,6 +238,7 @@ enum SoftBusErrNo {
238 SOFTBUS_SOURCE_IS_NOT_PRIMARY_USER,238 SOFTBUS_SOURCE_IS_NOT_PRIMARY_USER,
239 SOFTBUS_SINK_IS_NOT_PRIMARY_USER,239 SOFTBUS_SINK_IS_NOT_PRIMARY_USER,
240 SOFTBUS_RESOLVE_ABILITY_ERR,240 SOFTBUS_RESOLVE_ABILITY_ERR,
241+ SOFTBUS_DDOS_MSG_EXCEED_LIMIT,
241 242 
242 /* errno begin: -((203 << 21) | (5 << 16) | 0xFFFF) */243 /* errno begin: -((203 << 21) | (5 << 16) | 0xFFFF) */
243 SOFTBUS_TRANS_ERR_BASE = SOFTBUS_ERRNO(TRANS_SUB_MODULE_CODE),244 SOFTBUS_TRANS_ERR_BASE = SOFTBUS_ERRNO(TRANS_SUB_MODULE_CODE),
@@ -20,6 +20,7 @@
20#include "securec.h"20#include "securec.h"
21#include "softbus_access_token_adapter.h"21#include "softbus_access_token_adapter.h"
22#include "softbus_adapter_mem.h"22#include "softbus_adapter_mem.h"
23+#include "softbus_agent_communication.h"
23#include "softbus_error_code.h"24#include "softbus_error_code.h"
24#include "napi_agent_communication_error_code.h"25#include "napi_agent_communication_error_code.h"
25 26
@@ -51,6 +52,10 @@ bool ParseString(napi_env env, std::string &param, napi_value args)
51 COMM_LOGE(COMM_SDK, "Can not get string size.");52 COMM_LOGE(COMM_SDK, "Can not get string size.");
52 return false;53 return false;
53 }54 }
55+ if (size > COMMUNICATION_DATA_MAX_LEN) {
56+ COMM_LOGE(COMM_SDK, "string size exceed limit, size=%{public}zu", size);
57+ return false;
58+ }
54 param.reserve(size + 1);59 param.reserve(size + 1);
55 param.resize(size);60 param.resize(size);
56 size_t copied = 0;61 size_t copied = 0;