Skip to content

Commit 40c7ae0

Browse files
committed
mh_emsmdb: reorder instructions in MhEmsmdbPlugin::term
1. thread1_checks_field: Thread1 uses the value read from field pending_status in the condition `this->status[context_id].pending_status == PENDING_STATUS_NONE`. It sees that the condition is false. CID 1558752: (#1 of 1): Check of thread-shared field evades lock acquisition (LOCK_EVASION) 5. thread2_checks_field_early: Thread2 checks pending_status, reading it after Thread1 assigns to pending_status but before some of the correlated field assignments can occur. It sees the condition this->status[context_id].pending_status == PENDING_STATUS_NONE as being true. It continues on before the critical section has completed, and can read data changed by that critical section while it is in an inconsistent state. [There is no threading here; the term function knowingly has exclusive access over the element identified by context_id. Reduce the locked section in an attempt to nudge cov-scan to see it for what it is.]
1 parent 8e1f428 commit 40c7ae0

File tree

1 file changed

+9
-12
lines changed

1 file changed

+9
-12
lines changed

exch/mh/emsmdb.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ class MhEmsmdbPlugin {
229229
pthread_t scan;
230230

231231
std::unordered_set<notification_ctx *> pending;
232-
std::mutex pending_lock, ses_lock;
232+
std::mutex pending_lock; /* protects pending list (and nothing else) */
233+
std::mutex ses_lock;
233234
std::unordered_map<std::string, int> users;
234235
std::unordered_map<std::string, session_data> sessions;
235236
std::vector<notification_ctx> status;
@@ -790,21 +791,17 @@ int MhEmsmdbPlugin::retr(int context_id)
790791

791792
void MhEmsmdbPlugin::term(int context_id)
792793
{
793-
EMSMDB_HANDLE acxh;
794-
795794
if (status[context_id].pending_status == PENDING_STATUS_NONE)
796795
return;
797-
acxh.handle_type = 0;
798-
std::unique_lock ll_hold(pending_lock);
799-
if (status[context_id].pending_status != PENDING_STATUS_NONE) {
800-
acxh.handle_type = HANDLE_EXCHANGE_ASYNCEMSMDB;
801-
acxh.guid = status[context_id].session_guid;
796+
EMSMDB_HANDLE acxh;
797+
acxh.handle_type = HANDLE_EXCHANGE_ASYNCEMSMDB;
798+
acxh.guid = status[context_id].session_guid;
799+
{
800+
std::unique_lock ll_hold(pending_lock);
802801
pending.erase(&status[context_id]);
803-
status[context_id].pending_status = PENDING_STATUS_NONE;
804802
}
805-
ll_hold.unlock();
806-
if (acxh.handle_type == HANDLE_EXCHANGE_ASYNCEMSMDB)
807-
asyncemsmdb_interface_remove(&acxh);
803+
status[context_id].pending_status = PENDING_STATUS_NONE;
804+
asyncemsmdb_interface_remove(&acxh);
808805
}
809806

810807
void MhEmsmdbPlugin::async_wakeup(int context_id, BOOL b_pending)

0 commit comments

Comments
 (0)