
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 while the thread finishes otherwise there could be potential memory leaks or misuse of data.
Noncompliant Code Example
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.
#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 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 might ignore but is not a good practice.
#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 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 a particular thread does not maintain specific data the destructor would not be executed for that thread which eliminates the case of free(NULL) in case of second noncompliant code sample.
#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 |
|
|