All locking and unlocking of mutexes should be performed in the same module and at the same level of abstraction. Failure to follow this recommendation can lead to some lock or unlock operations not being executed by the multithreaded program as designed, eventually resulting in deadlock, race conditions, or other security vulnerabilities, depending on the mutex type.

A common consequence of improper locking is for a mutex to be unlocked twice, via two calls to mtx_unlock(). This can cause the unlock operation to return errors. In the case of recursive mutexes, an error is returned only if the lock count is 0 (making the mutex available to other threads) and a call to mtx_unlock() is made.

Noncompliant Code Example

In this noncompliant code example for a simplified multithreaded banking system, imagine an account with a required minimum balance. The code would need to verify that all debit transactions are allowable. Suppose a call is made to debit() asking to withdraw funds that would bring account_balance below MIN_BALANCE, which would result in two calls to mtx_unlock(). In this example, because the mutex is defined statically, the mutex type is implementation-defined.

#include <threads.h>

enum { MIN_BALANCE = 50 };

int account_balance;
mtx_t mp;

/* Initialize mp */

int verify_balance(int amount) {
  if (account_balance - amount < MIN_BALANCE) {
    /* Handle error condition */
    if (mtx_unlock(&mp) ==  thrd_error) {
      /* Handle error */
    }
    return -1;
  }
  return 0;
}

void debit(int amount) {
  if (mtx_lock(&mp) == thrd_error) {
    /* Handle error */
  }
  if (verify_balance(amount) == -1) {
    if (mtx_unlock(&mp) == thrd_error) {
      /* Handle error */
    }
    return;
  }
  account_balance -= amount;
  if (mtx_unlock(&mp) == thrd_error) {
    /* Handle error */
  }
}

Compliant Solution

This compliant solution unlocks the mutex only in the same module and at the same level of abstraction at which it is locked. This technique ensures that the code will not attempt to unlock the mutex twice.

#include <threads.h>

enum { MIN_BALANCE = 50 };

static int account_balance;
static mtx_t mp;

/* Initialize mp */

static int verify_balance(int amount) {
  if (account_balance - amount < MIN_BALANCE) {
    return -1;   /* Indicate error to caller */
  }

  return 0;  /* Indicate success to caller */
}

int debit(int amount) {
  if (mtx_lock(&mp) == thrd_error) {
    return -1;   /* Indicate error to caller */
  }

  if (verify_balance(amount)) {
    mtx_unlock(&mp);
    return -1;   /* Indicate error to caller */
  }

  account_balance -= amount;

  if (mtx_unlock(&mp) == thrd_error) {
    return -1;   /* Indicate error to caller */
  }

  return 0;   /* Indicate success */
}

Risk Assessment

Improper use of mutexes can result in denial-of-service attacks or the unexpected termination of a multithreaded program.

Automated Detection

Tool	Version	Checker	Description
Astrée			Supported, but no explicit checker
CodeSonar		CONCURRENCY.LOCK.NOLOCK CONCURRENCY.LOCK.NOUNLOCK	Missing Lock Acquisition Missing Lock Release
Coverity	6.5	LOCK	Fully implemented
Parasoft C/C++test		CERT_C-CON01-a	Do not abandon unreleased locks
PC-lint Plus		454, 455, 456	Partially supported: acquire and release synchronization primitives within the same scope
Polyspace Bug Finder		CERT C: Rec. CON01-C	Checks for: Missing lock and unlock functions Double lock or double unlock Rec. fully covered.

Bibliography