Skip to content

DoNotAllowAMutexToGoOutOfScopeWhileLocked.md

Latest commit

 

History

History
172 lines (117 loc) · 7.18 KB

DoNotAllowAMutexToGoOutOfScopeWhileLocked.md

File metadata and controls

172 lines (117 loc) · 7.18 KB

CON31-C: Do not destroy a mutex while it is locked

This query implements the CERT-C rule CON31-C:

Do not destroy a mutex while it is locked

Description

Mutexes are used to protect shared data structures being concurrently accessed. If a mutex is destroyed while a thread is blocked waiting for that mutex, critical sections and shared data are no longer protected.

The C Standard, 7.26.4.1, paragraph 2 [ISO/IEC 9899:2011], states

The mtx_destroy function releases any resources used by the mutex pointed to by mtx. No threads can be blocked waiting for the mutex pointed to by mtx.

This statement implies that destroying a mutex while a thread is waiting on it is undefined behavior.

Noncompliant Code Example

This noncompliant code example creates several threads that each invoke the do_work() function, passing a unique number as an ID. The do_work() function initializes the lock mutex if the argument is 0 and destroys the mutex if the argument is max_threads - 1. In all other cases, the do_work() function provides normal processing. Each thread, except the final cleanup thread, increments the atomic completed variable when it is finished.

Unfortunately, this code contains several race conditions, allowing the mutex to be destroyed before it is unlocked. Additionally, there is no guarantee that lock will be initialized before it is passed to mtx_lock(). Each of these behaviors is undefined.

#include <stdatomic.h>
#include <stddef.h>
#include <threads.h>
 
mtx_t lock;
/* Atomic so multiple threads can modify safely */
atomic_int completed = ATOMIC_VAR_INIT(0);
enum { max_threads = 5 }; 

int do_work(void *arg) {
  int *i = (int *)arg;

  if (*i == 0) { /* Creation thread */
    if (thrd_success != mtx_init(&lock, mtx_plain)) {
      /* Handle error */
    }
    atomic_store(&completed, 1);
  } else if (*i < max_threads - 1) { /* Worker thread */
    if (thrd_success != mtx_lock(&lock)) {
      /* Handle error */
    }
    /* Access data protected by the lock */
    atomic_fetch_add(&completed, 1);
    if (thrd_success != mtx_unlock(&lock)) {
      /* Handle error */
    }
  } else { /* Destruction thread */
    mtx_destroy(&lock);
  }
  return 0;
}
 
int main(void) {
  thrd_t threads[max_threads];
  
  for (size_t i = 0; i < max_threads; i++) {
    if (thrd_success != thrd_create(&threads[i], do_work, &i)) {
      /* Handle error */
    }
  }
  for (size_t i = 0; i < max_threads; i++) {
    if (thrd_success != thrd_join(threads[i], 0)) {
      /* Handle error */
    }
  }
  return 0;
}

Compliant Solution

This compliant solution eliminates the race conditions by initializing the mutex in main() before creating the threads and by destroying the mutex in main() after joining the threads:

#include <stdatomic.h>
#include <stddef.h>
#include <threads.h>
 
mtx_t lock;
/* Atomic so multiple threads can increment safely */
atomic_int completed = ATOMIC_VAR_INIT(0);
enum { max_threads = 5 }; 

int do_work(void *dummy) {
  if (thrd_success != mtx_lock(&lock)) {
    /* Handle error */
  }
  /* Access data protected by the lock */
  atomic_fetch_add(&completed, 1);
  if (thrd_success != mtx_unlock(&lock)) {
    /* Handle error */
  }

  return 0;
}

int main(void) {
  thrd_t threads[max_threads];
  
  if (thrd_success != mtx_init(&lock, mtx_plain)) {
    /* Handle error */
  }
  for (size_t i = 0; i < max_threads; i++) {
    if (thrd_success != thrd_create(&threads[i], do_work, NULL)) {
      /* Handle error */
    }
  }
  for (size_t i = 0; i < max_threads; i++) {
    if (thrd_success != thrd_join(threads[i], 0)) {
      /* Handle error */
    }
  }

  mtx_destroy(&lock);
  return 0;
}

Risk Assessment

Destroying a mutex while it is locked may result in invalid control flow and data corruption.

Rule Severity Likelihood Remediation Cost Priority Level
CON31-C Medium Probable High P4 L3

Automated Detection

Tool Version Checker Description
Astrée 22.04 Supported, but no explicit checker
CodeSonar 7.0p0 CONCURRENCY.LOCALARG Local Variable Passed to Thread
Helix QAC 2022.2 C4961, C4962
PRQA QA-C 9.7 4961, 4962
Parasoft C/C++test 2022.1 CERT_C-CON31-a CERT_C-CON31-b CERT_C-CON31-c Do not destroy another thread's mutex Do not use resources that have been freed Do not free resources using invalid pointers
Polyspace Bug Finder R2022a CERT C: Rule CON31-C Checks for destruction of locked mutex (rule fully covered)

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Related Guidelines

Key here (explains table format and definitions)

Taxonomy Taxonomy item Relationship
CWE 2.11 CWE-667 , Improper Locking 2017-07-10: CERT: Rule subset of CWE

CERT-CWE Mapping Notes

Key here for mapping notes

CWE-667 and CON31-C/POS48-C

Intersection( CON31-C, POS48-C) = Ø

CWE-667 = Union, CON31-C, POS48-C, list) where list =

  • Locking & Unlocking issues besides unlocking another thread’s C mutex or pthread mutex.

Bibliography

\[ ISO/IEC 9899:2011 \] 7.26.4.1, "The mtx_destroy Function"

Implementation notes

This implementation does not allow for thread synchronization to be performed in subroutines. All synchronization must be performed within the context of the other thread management functions.

References