Skip to end of metadata
Go to start of metadata

The tss_create() function creates a thread-specific storage pointer identified by a key. Threads can allocate thread-specific storage and associate the storage with a key that uniquely identifies the storage by calling the tss_set() function. If not properly freed, this memory may be leaked. Ensure that thread-specific storage is freed.

Noncompliant Code Example

In this noncompliant code example, each thread dynamically allocates storage in the get_data() function, which is then associated with the global key by the call to tss_set() in the add_data() function. This memory is subsequently leaked when the threads terminate.

#include <threads.h>
#include <stdlib.h>

/* Global key to the thread-specific storage */
tss_t key;
enum { MAX_THREADS = 3 };

int *get_data(void) {
  int *arr = (int *)malloc(2 * sizeof(int));
  if (arr == NULL) {
    return arr;  /* Report error  */
  }
  arr[0] = 10;
  arr[1] = 42;
  return arr;
}

int add_data(void) {
  int *data = get_data();
  if (data == NULL) {
    return -1;	/* Report error */
  }

  if (thrd_success != tss_set(key, (void *)data)) {
    /* Handle error */
  }
  return 0;
}

void print_data(void) {
  /* Get this thread's global data from key */
  int *data = tss_get(key);

  if (data != NULL) {
    /* Print data */
  } 
}

int function(void *dummy) {
  if (add_data() != 0) {
    return -1;	/* Report error */
  }
  print_data();
  return 0;
}

int main(void) {
  thrd_t thread_id[MAX_THREADS];

  /* Create the key before creating the threads */
  if (thrd_success != tss_create(&key, NULL)) {
    /* Handle error */
  }

  /* Create threads that would store specific storage */
  for (size_t i = 0; i < MAX_THREADS; i++) {
    if (thrd_success != thrd_create(&thread_id[i], function, NULL)) {
      /* Handle error */
    }
  }

  for (size_t i = 0; i < MAX_THREADS; i++) {
    if (thrd_success != thrd_join(thread_id[i], NULL)) {
      /* Handle error */
    }
  }

  tss_delete(key);
  return 0;
}

Compliant Solution

In this compliant solution, each thread explicitly frees the thread-specific storage returned by the tss_get() function before terminating:

#include <threads.h>
#include <stdlib.h>
 
/* Global key to the thread-specific storage */
tss_t key;
 
int function(void *dummy) {
  if (add_data() != 0) {
    return -1;	/* Report error */
  }
  print_data();
  free(tss_get(key));
  return 0;
}

/* ... Other functions are unchanged */

Compliant Solution

This compliant solution invokes a destructor function registered during the call to tss_create() to automatically free any thread-specific storage:

#include <threads.h>
#include <stdlib.h>

/* Global key to the thread-specific storage */
tss_t key;
enum { MAX_THREADS = 3 };

/* ... Other functions are unchanged */

void destructor(void *data) {
  free(data);
}
 
int main(void) {
  thrd_t thread_id[MAX_THREADS];

  /* Create the key before creating the threads */
  if (thrd_success != tss_create(&key, destructor)) {
    /* Handle error */
  }

  /* Create threads that would store specific storage */
  for (size_t i = 0; i < MAX_THREADS; i++) {
    if (thrd_success != thrd_create(&thread_id[i], function, NULL)) {
      /* Handle error */
    }
  }

  for (size_t i = 0; i < MAX_THREADS; i++) {
    if (thrd_success != thrd_join(thread_id[i], NULL)) {
      /* Handle error */
    }
  }

  tss_delete(key);
  return 0;
}


Risk Assessment

Failing to free thread-specific objects results in memory leaks and could result in a denial-of-service attack.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

CON30-C

Medium

Unlikely

Medium

P4

L3

Automated Detection

ToolVersionCheckerDescription
Astrée
19.04

Supported, but no explicit checker
Coverity
2017.07
ALLOC_FREE_MISMATCHPartially implemented, correct implementation is more involved
Parasoft C/C++test
10.4.2

CERT_C-CON30-a

Ensure resources are freed

Related Vulnerabilities

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



7 Comments

  1. From Geoff Clare:

    The pthread_setspecific() call in the destructor() function in the CS is not needed. The system automatically sets the value to NULL on thread exit if there is a destructor function associated with the key; there is no need to do it explicitly.

    The statement "free(NULL) which most compilers might ignore" is incorrect. The standard requires free() to accept a null pointer (and take no action).

    Also the malloc.h includes should be stdlib.h.

  2. One more thing.  For the first compliant solution, should I add the following line:

     

    int function(void *dummy) {
      if (add_data() != 0) {
        return -1;  /* Report error */
      }
      print_data();
      free(tss_get(key));
      (void) tss_set(key, NULL)
      return 0;
    }

    Geoff Clare suggests that pthreads does this automatically from the destructor, although I don't see any such guarantees in the C Standard.

    1. The standard is vague on tss destruction, but I would guess that it is harmless, but not helpful, to do that. Once you call tss_delete(), calls to tss_get() for that key would return 0 in a sane implementation.

      1. OK, maybe I'll just omit it.

  3. At first glance, I thought this rule was about the tss_delete function (e.g. ensure you call tss_delete), but that isn't the case.  Rather, the gist of it seems to be, if you associate dynamically allocated memory with a thread-specific-storage pointer, make sure you free it.  I feel like this scenario is an instantiation of MEM31-C. Free dynamically allocated memory when no longer needed, and may not require a seperate rule.

  4. sorry for only commenting... (I don't have edit privilege now)

    the URL of Defect Report #416 should be http://www.open-std.org/JTC1/SC22/WG14/www/docs/summary.htm#dr_416

     

    1. I've corrected the link, thank you for pointing it out!