Skip to content

Commit d57e772

Browse files
committed
http: turn pdu_processor's g_last_async_id into an atomic<uint32_t>
cov-scan misidentified what g_async_lock protects. Try to nudge it. CID 1596017: (#1 of 1): Data race condition (MISSING_LOCK) CID 1596019: (#1 of 1): Data race condition (MISSING_LOCK) 2. missing_lock: Accessing g_bigendian without holding lock g_async_lock. Elsewhere, g_bigendian is written to with g_async_lock held 1 out of 1 times (1 of these accesses strongly imply that it is necessary). CID 1596018: (#1 of 1): Data race condition (MISSING_LOCK) 1. missing_lock: Accessing g_connection_num without holding lock g_async_lock. Elsewhere, g_connection_num is written to with g_async_lock held 1 out of 1 times.
1 parent f9f0577 commit d57e772

File tree

1 file changed

+18
-9
lines changed

1 file changed

+18
-9
lines changed

exch/http/pdu_processor.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-FileCopyrightText: 2021–2025 grommunio GmbH
33
// This file is part of Gromox.
44
#include <algorithm>
5+
#include <atomic>
56
#include <climits>
67
#include <cstdint>
78
#include <cstdio>
@@ -112,12 +113,13 @@ static int g_connection_ratio;
112113
static char g_dns_domain[128];
113114
static char g_netbios_name[128];
114115
static size_t g_max_request_mem;
115-
static uint32_t g_last_async_id;
116+
static std::atomic<uint32_t> g_last_async_id;
116117
static thread_local DCERPC_CALL *g_call_key;
117118
static thread_local NDR_STACK_ROOT *g_stack_key;
118119
static thread_local PROC_PLUGIN *g_cur_plugin;
119120
static std::list<PROC_PLUGIN> g_plugin_list;
120-
static std::mutex g_list_lock, g_async_lock;
121+
static std::mutex g_list_lock; /* protects g_processor_list */
122+
static std::mutex g_async_lock; /* protects g_async_hash */
121123
static std::list<DCERPC_ENDPOINT> g_endpoint_list;
122124
static bool support_negotiate = false; /* possibly nonfunctional */
123125
static std::unordered_map<int, ASYNC_NODE *> g_async_hash;
@@ -193,7 +195,6 @@ void pdu_processor_init(int connection_num, const char *netbios_name,
193195
} e;
194196

195197
e.i = 0xFF000000;
196-
std::unique_lock as_hold(g_async_lock);
197198
g_bigendian = e.c[0] != 0 ? TRUE : false;
198199
g_last_async_id = 0;
199200
g_connection_ratio = connection_ratio;
@@ -1477,9 +1478,20 @@ static BOOL pdu_processor_reply_request(DCERPC_CALL *pcall,
14771478
return TRUE;
14781479
}
14791480

1481+
static uint32_t get_next_async_id()
1482+
{
1483+
do {
1484+
uint32_t curr = g_last_async_id;
1485+
uint32_t next = curr + 1;
1486+
if (next >= INT32_MAX)
1487+
next = 0;
1488+
if (g_last_async_id.compare_exchange_weak(curr, next))
1489+
return next;
1490+
} while (true);
1491+
}
1492+
14801493
static uint32_t pdu_processor_apply_async_id()
14811494
{
1482-
int async_id;
14831495
DCERPC_CALL *pcall;
14841496
HTTP_CONTEXT *pcontext;
14851497

@@ -1514,12 +1526,9 @@ static uint32_t pdu_processor_apply_async_id()
15141526
pasync_node->vconn_port = pcontext->port;
15151527
strcpy(pasync_node->vconn_cookie, pchannel_in->connection_cookie);
15161528

1517-
std::unique_lock as_hold(g_async_lock);
1518-
g_last_async_id ++;
1519-
async_id = g_last_async_id;
1520-
if (g_last_async_id >= INT32_MAX)
1521-
g_last_async_id = 0;
15221529
auto ctx_num = g_connection_num * g_connection_ratio;
1530+
auto async_id = get_next_async_id();
1531+
std::unique_lock as_hold(g_async_lock);
15231532
if (g_async_hash.size() >= 2 * ctx_num) {
15241533
as_hold.unlock();
15251534
delete pasync_node;

0 commit comments

Comments
 (0)