Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

Code Block
bgColor#ffcccc
#include <pthread.h>
#include <stdio.h>
#include<malloc.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 )
{
    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){
            /*Handle Error */
        }
    }
    pthread_key_delete(key);
}

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

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

int *get_data()
{
    int *arr = malloc(2*sizeof(int));
    *arr = rand();
    *(arr+1) = rand();
    return arr;
}
void print_data()
{
    /* get this thread's global data from
    * priority_key */
    int *data = pthread_getspecific(key);
    /* Print 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) which most compilers would might ignore but is definitely not a good practice.

Code Block
bgColor#ffcccc
#include <pthread.h>
#include <stdio.h>
#include<malloc.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 )
{
    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){
            /*Handle Error */
        }
    }
    pthread_key_delete(key);
}

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

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

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

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


Compliant Solution

The compliant solution attempts to avoid the memory leak and sets the thread specific data to NULL. An explicit destructor should be used to free memory resources and clean thread specific data. In case the a particular thread does not maintain specific data is NULL the destructor would not be executed for that thread which eliminates the case of free(NULL) in case of second noncompliant code sample.

Code Block
bgColor#ccccff
#include <pthread.h>
#include <stdio.h>
#include<malloc.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};

void destructor(void * data){
    pthread_setspecific(key, NULL); // thread specific should be cleared (could be sensitive)
    free(data);
}

int main( void )
{
    int i;
    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_create( &thread_id[i], NULL, function, NULL );
    }
 
    for(i=0;i < max_threads; i++){
        int ret = pthread_join(thread_id[i], NULL);
        if(ret !=0){
            /* Handle Error */
        }
    }
    pthread_key_delete(key);
}

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

void add_data()
{
    int *data = get_data();
    /* set current thread's data */
    pthread_setspecific( key,(void *) data);
}
int *get_data(){
    int *arr = malloc(2*sizeof(int)); /* Not handling the integer overflow */
    *arr = rand();
    *(arr+1) = rand();
    return arr;
}
void print_data()
{
    /* retrieve current thread's data from key */
    int *data = pthread_getspecific(key);
    /* Print Data */
}


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

 

probable

medium