Skip to end of metadata
Go to start of metadata

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.

#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
19.04

Supported, but no explicit checker
 PRQA QA-C

9.5

4961, 4962
Parasoft C/C++test
10.4.2

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

R2019a

CERT C: Rule CON31-CChecks 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.11CWE-667, Improper Locking2017-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"



9 Comments

  1. What happens if the worker thread dies while holding the mutex? This would cause the clean up thread to block forever... is there a fix for this? Should we mention it?

  2. This page seems to be in an inconsistent state.  It says "This solution requires the cleanup function to acquire the lock before destroying it" but then goes on to give a quote from POSIX that effectively forbids doing this ("Attempting to destroy a locked mutex results in undefined behavior") and the code given as the solution does not lock the mutex in the cleanup function.  Instead the code assumes that pthread_mutex_lock() will return an error if the mutex has been destroyed, but this is no good either, because according to POSIX the behaviour is undefined ("A destroyed mutex object can be reinitialized using pthread_mutex_init(); the results of otherwise referencing the object after it has been destroyed are undefined.")

    The proper solution is to design the application in such a way that the cleanup cannot be done while worker threads might still have the mutex locked.  For example, don't do the cleanup until all the worker threads have been joined. 

    1. This is correct.. the quote from POSIX kind of hoses this entire rule, as there is really no way to destroy another thread's mutex that would be problematic since pthread_mutex_destroy() acquires all necessary locks...

      Looking at this now, the compliant and non-compliant solutions are actually the same

      We should probably delete this rule since it is in contradiction with what POSIX says

      another quote from the man page:

      pthread_mutex_destroy destroys a mutex object, freeing the resources
      it might hold. The mutex must be unlocked on entrance. In the Linux-
      Threads implementation, no resources are associated with mutex
      objects, thus pthread_mutex_destroy actually does nothing except
      checking that the mutex is unlocked.

  3. This seems to be confused about processes and threads.  Linux doesn't help by making threads close to processes.

  4. I believe the issues are addressed now.

    1. There were still some things that implied an application could expect pthread_mutex_lock() to fail if the mutex has been destroyed.  The error is optional in POSIX, and on systems that don't detect it you get undefined behaviour.  There was also an incorrect statement about pthread_mutex_destroy() acquiring locks.  I have applied fixes.

      The quote from POSIX before the compliant code now seems to be orphaned, but I wasn't sure what should be done with it.

      1. I agree.  The quote didn't seem to serve a purpose any more, so I removed it.

  5. I removed the unenforceable flag since this is enforceable via dynamic analysis.

    1. We appear to be referring to a static analysis checker in Fortify that can detect violations of this rule.  That suggests that this rule could also be checked through static analysis, or that this is a mistake.