While using tss_create() to prepare a key for use 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, potential memory leaks or misuse of data can occur.
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 enabling access to the one thread's specific data even after it exits from a memory leak.
/* global key to the thread-specific data */
tss_t key;
enum {max_threads = 3};
int *get_data() {
int *arr = malloc(2*sizeof(int));
if (arr == NULL) {
/* Handle Error */
}
arr[0] = rand();
arr[1] = rand();
return arr;
}
void add_data(void) {
int *data = get_data();
int result;
if ((result = tss_set( key, (void *)data)) != thrd_success) {
/* Handle Error */
}
}
void print_data() {
/* get this thread's global data from key */
int *data = tss_get(key);
/* print data */
}
void *function(void* dummy) {
add_data();
print_data();
thrd_exit(0);
return NULL;
}
int main(void) {
int i,result;
thrd_t thread_id[max_threads];
/* create the key before creating the threads */
if ((result = tss_create( &key, NULL )) != thrd_success) {
/* Handle Error */
}
/* create threads that would store specific data */
for (i = 0; i < max_threads; i++) {
if ((result = thrd_create( &thread_id[i], function, NULL )) != thrd_success) {
/* Handle Error */
}
}
for (i = 0; i < max_threads; i++) {
if ((result = thrd_join(thread_id[i], NULL)) != thrd_success) {
/* Handle Error */
}
}
if ((result = tss_delete(key)) != thrd_success) {
/* Handle Error */
}
return 0;
}
Noncompliant Code Example
This code example is an improvement over the previous example. However, even if the address of the data is known, it still presents a potential vulnerability. If, for any reason, the data received from an arbitrary function, get_data(), is NULL, a call to free(tss_get(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).
void *function(void* dummy) {
add_data();
print_data();
free( tss_get(key));
thrd_exit(0);
return NULL;
}
/* ... Other functions are unchanged */
int main(void) {
/* ... */
if ((result = tss_delete(key)) != thrd_success) {
/* Handle Error */
}
return 0;
}
Compliant Solution
This compliant solution avoids the memory leak and destroys the value associated to a key for the thread-specific data. A destructor function is a clean approach because it automatically sets the specific value associated to the key to NULL when the thread exits. In case any particular thread does not maintain specific data (a call to tss_get() returns NULL), it also ensures that the destructor function is not executed unnecessarily when the thread exits.
void* destructor(void* data) {
free(data);
return NULL;
}
int main(void) {
int i,result;
thrd_t thread_id[max_threads];
/* create the key before creating the threads */
if ((result = tss_create( &key, destructor)) != thrd_success) {
/* Handle Error */
}
/* ... */
if ((result = tss_delete(key)) != thrd_success) {
/* Handle Error */
}
return 0;
}
Risk Assessment
Failing to destroy thread-specific objects could lead to memory leaks and misuse of data.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
CON30-C | medium | unlikely | medium | P4 | L3 |