Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: general cleanup

While using pthread_key_create() to prepare a key to be used by various threads to maintain thread specific data, ensure that the thread specific data stored for a key is cleaned up when the thread exits otherwise there could be potential memory leaks or misuse of data.

...

The dynamically allocated block of memory for every thread results in a memory leak and is not destroyed in the noncompliant code example. This also leads to a potential vulnerability to access the one thread's specific data even after it exits apart from a memory leak.

Code Block
bgColor#ffcccc
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

/* function prototypes */
void destructor(void * data);
void* function();
void add_data();
void print_data();


/* global key to the thread specific data */
pthread_key_t key;
enum {max_threads = 3};

int main( void )*get_data() {
  int i,ret;
  pthread_t thread_id[max_threads];

  /* create the key before creating the threads */
  pthread_key_create( &key, NULL );

  /* create threads that would store specific data*/
  for(i =0; i < max_threads; i++){
    pthread_create( &thread_id[i], NULL, function, NULL );
  }

  for(i=0;i < max_threads; i++){
    ret = pthread_join(thread_id[i], NULL);
    if(ret !=0)*arr = malloc(2*sizeof(int));
  if (arr == NULL) {
      /* Handle Error  */
  }
  }
  }
  pthread_key_delete(key);
}

void *function(void){
  add_dataarr[0] = rand();
  arr[1] = rand();
  print_data();
  pthread_exit(NULL)  return arr;
}

void add_data(void) {
   int *data = get_data();
   int result;
  if ((result = pthread_setspecific( key,(void *)data);
}

int *get_data(){
  int *arr = malloc(2*sizeof(int));
  *arr = rand();
  *(arr+1) = rand();
  return arr;)) != 0) {
    /* Handle Error */
  }
}

void print_data() {
   /* get this thread's global data from key */
   int *data = pthread_getspecific(key);
  /* Printprint data */
}

Noncompliant Code Example

This code example is an improvement over the above sample where we try to avoid a memory leak. However if the address of the data is known it could still lead to a potential vulnerability. If for any reason the data received from an arbitrary function get_data() is NULL, a call to free(pthread_getspecific(key) would be equivalent to free(NULL) for which the compiler takes no action by standard. This solution however does not avoid a call to free when there is no thread specific data, which is not unsafe but can be avoided (shown in Compliant Solution).

Code Block
bgColor#ffcccc
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

/* function prototypes */
void destructor(void * data);
void* function();
void
void *function(void* dummy) {
  add_data();
void  print_data();


/* global key to the thread specific data */
pthread_key_t key;
enum {max_threads = 3};pthread_exit(NULL);
  return NULL;
}

int main( void) ){
   int i,retresult;
   pthread_t thread_id[max_threads];

   /* create the key before creating the threads */
   if ((result = pthread_key_create( &key, NULL );

 ) != 0) {
    /* Handle Error */
  }

  /* create threads that would store specific data */
   for (i = 0; i < max_threads; i++) {
       if ((result = pthread_create( &thread_id[i], NULL, function, NULL )) );
 != 0) {
      /* Handle Error */
    }
  }

   for (i = 0; i < max_threads; i++) {
    ret    if ((result = pthread_join(thread_id[i], NULL));
    if(ret != 0) {
      /* Handle Error */
    }
   }
 

  if ((result = pthread_key_delete(key)) != 0) {
    /* Handle Error */
  }
  return 0;
}

Noncompliant Code Example

This code example is an improvement over the above sample where we try to avoid a memory leak. However if the address of the data is known it could still lead to a potential vulnerability. If for any reason the data received from an arbitrary function get_data() is NULL, a call to free( pthread_getspecific(key)) would be equivalent to free(NULL) for which the compiler takes no action according to the standard. This solution however does not avoid a call to free() when there is no thread specific data, which is not unsafe but can be avoided (as shown in the subsequent compliant solution).

Code Block
bgColor#ffcccc
void *function(void* dummy) {
   add_data();
   print_data();
  free( pthread_getspecific(key));
   pthread_exit(NULL);
  return NULL;
}

void add_data(void){
  int *data = get_data();
  pthread_setspecific( key,(void *)data);
}

int *get_data(void){
  int *arr = malloc(2*sizeof(int));
  *arr = rand();
  *(arr+1) = rand();
  return arr;
}

void print_data(void){
  /* get this thread's global data from key */
  int *data = pthread_getspecific(key);
  /* Print data *//* ... Other functions are unchanged */

int main(void) {
  /* ... */
  if ((result = pthread_key_delete(key)) != 0) {
    /* Handle Error */
  }
  return 0;
}

Compliant Solution

The This compliant solution avoids the memory leak and destroys value associated to a key for the thread specific data. A destructor function is a clean approach because it would automatically set sets the specific value associated to the key to NULL on when the thread exitexits. In case a any particular thread does not maintain specific data (a call to pthread_getspecific() returns NULL), it also ensures that the destructor function is not executed unnecessarily on when the thread exitexits.

Code Block
bgColor#ccccff
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

/* function prototypes */
void void* destructor(void * data); {
void* function();
void add_data();
void print_data( free(data);

/* global key to the thread specific data */
pthread_key_t key;
enum {max_threads = 3};

void destructor(void * data){
  free(data)return NULL;
}

int main( void) ){
   int i,result;
   pthread_t thread_id[max_threads];

   /* create the thread specific data key before creating the threads */
  pthread_key_create( &key, destructor );

  /* create thread that will use the key */
  for(i =0; i < max_threads; i++){
    pthread if ((result = pthread_key_create( &thread_id[i]key, NULL, function, NULL );
  }
 
  for(i=0;i < max_threads; i++){
    int ret = pthread_join(thread_id[i], NULL);
    if(ret !=0){
     destructor)) != 0) {
    /* Handle Error */
    }
  }
  pthread_key_delete(key);
}

void *function(void){
  add_data();
  print_data();
  pthread_exit(NULL);
}

void add_data(void){
  int *data = get_data();
  /* set current thread's data */
  pthread_setspecific( key,(void *) data);
}

int *get_data(void){
  int *arr = malloc(2*sizeof(int)); /* Not handling the integer overflow */
  *arr = rand();
  *(arr+1) = rand();
  return arr;
}

void print_data(void){
  /* retrieve current thread's data from key */
  int *data = pthread_getspecific(key);
  /* Print Data */
}

... */

  if ((result = pthread_key_delete(key)) != 0) {
    /* Handle Error */
  }
  return 0;
}

Risk Assessment

Failing to destroy the objects could lead to memory leaks and misuse of data

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

POS45-C

medium

unlikely

medium

P2

L3

References