SEI CERT C Coding Standard

Space shortcuts

Page tree

Pthread mutual exclusion (mutex) locks are used to avoid simultaneous usage of common resources. Several types of mutex locks are defined by pthreads: NORMAL, ERRORCHECK, RECURSIVE, and DEFAULT.

POSIX describes PTHREAD_MUTEX_NORMAL locks as having the following undefined behavior [Open Group 2004]:

This type of mutex does not provide deadlock detection. A thread attempting to relock this mutex without first unlocking it shall deadlock. An error is not returned to the caller. Attempting to unlock a mutex locked by a different thread results in undefined behavior. Attempting to unlock an unlocked mutex results in undefined behavior.

The DEFAULT mutex pthread is also generally mapped to PTHREAD_MUTEX_NORMAL but is known to vary from platform to platform [SOL 2010]. Consequently, NORMAL locks should not be used, and ERRORCHECK or RECURSIVE locks should be defined explicitly when mutex locks are used.

Noncompliant Code Example

This noncompliant code example shows a simple mutex being created using PTHREAD_MUTEX_NORMAL. Note that the caller does not expect a return code when NORMAL mutex locks are used.

pthread_mutexattr_t attr;
pthread_mutex_t mutex;
size_t const shared_var = 0;

int main(void) {
  int result;

  if ((result = pthread_mutexattr_init(&attr)) != 0) {
    /* Handle Error */
  }
  if ((result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_NORMAL)) != 0) {
    /* Handle Error */
  }
  if ((result = pthread_mutex_init(&mutex, &attr)) != 0) {
    /* Handle Error */
  }
  if ((result = pthread_mutex_lock(&mutex)) != 0) {
    /* Handle Error */
  }

  /* Critical Region*/

  if ((result = pthread_mutex_unlock(&mutex)) != 0) {
    /* Handle Error */
  }

  return 0;
}

Compliant Solution

This compliant solution shows an ERRORCHECK mutex lock being created so that return codes will be available during locking and unlocking:

pthread_mutexattr_t attr;
pthread_mutex_t mutex;
size_t const shared_var = 0;

int main(void) {
  int result;

  if ((result = pthread_mutexattr_init(&attr)) != 0) {
    /* Handle Error */
  }
  if ((result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK)) != 0) {
    /* Handle Error */
  }
  if ((result = pthread_mutex_init(&mutex, &attr)) != 0) {
    /* Handle Error */
  }
  if ((result = pthread_mutex_lock(&mutex)) != 0) {
    /* Handle Error */
  }

  /* Critical Region*/

  if ((result = pthread_mutex_unlock(&mutex)) != 0) {
    /* Handle Error */
  }

  return 0;
}

Risk Assessment

Using NORMAL mutex locks can lead to deadlocks or abnormal program termination.

Recommendation

Severity

Likelihood

Detectable

Repairable

Priority

Level

POS04-C

Low

Unlikely

Yes

Yes

P3

L3

Automated Detection

Tool

Version

Checker

Description

PC-lint Plus

1.4

586

Fully supported

Bibliography

[Open Group 2004]
[SOL 2010]


10 Comments

  1. David Svoboda

    Much better rule. Comments:

    Our style (instead of /* Check Eror Code */) has always been:

      if (cc != ) {
    /* Handle Error */
  }
    • Your current code is well illustrative, but please change this bit for consistency with our rules.
    • The Risk Assessment should have some text describing what happens when the rule is violated. I suspect the severity isn't really high, if the worst that can happen is deadlock.

    1. David Svoboda

      This rule is now complete.

  2. Geoff Clare

    Recommending that secure programs use error checking (or recursive) mutexes seems reasonable (even though it means they will suffer a performance penalty). However, I disagree with the use of the word "likely" in the risk assessment. The situations in which using normal mutexes results in undefined behaviour are all caused by programming errors. In a well written program (which secure programs ought to be), such situations are therefore not "likely".

    1. David Svoboda

      Changed liklihood to 'unlikely'.

  3. Frank Martinez

    Would the NCE even compile, given the "{{}}" sequence?

    1. David Svoboda

      No. I've fixed the code, it at least compiles now (smile)

  4. David Svoboda

    I wonder if this rule has an analog using C11 threads.

    C11 mutexes support recursive vs. non-recursive locks, and recursive locks seem to offer the same tradeoff in C11 that they do in POSIX...for a small performance cost, they let one thread re-acquire a mutex.

    OTOH C11 has no analogue for 'error-checking mutexes'.

    So is this rule more useful for requiring recursive mutexes, or is it more for requiring mutexes with error-checking?

  5. Adam Schettenhelm

    I was using the compliant solution as an example for initializing a mutex, but `pthread_mutex_init` was failing and returning "Operation not supported". When I called pthread_mutexattr_init() on the pthread_mutexattr_t before calling pthread_mutexattr_settype(), I was able to initialize the mutex successfully.

    1. David Svoboda

      I amended both code examples to use pthread_mutexattr_init(), because working with uninitialized attributes violates EXP33-C. Do not read uninitialized memory.

  6. Brandon Casey

    Recursive mutexes are not safe to use with condition variables. As Posix explains in the APPLICATION USAGE section of the pthread_mutexattr_settype docs here:

    It is advised that an application should not use a PTHREAD_MUTEX_RECURSIVE mutex with condition variables because the implicit unlock performed for a pthread_cond_timedwait() or pthread_cond_wait() may not actually release the mutex (if it had been locked multiple times). If this happens, no other thread can satisfy the condition of the predicate.

    Furthermore, in my opinion, the perceived necessity to use recursive mutexes often suggests a lack of understanding on the part of the author of their locking strategy, which can lead to more problems than a deadlock that is possible (and is indicative of a bug) by using a non-recursive mutex. i.e. problems related to using recursive mutexes can be more subtle and difficult to detect than a bug that results in a deadlock condition.

    Aside from the performance implications, I don't see any problem with the recommendation to use error checking mutexes though.