SEI CERT C Coding Standard

Space shortcuts

Page tree

Versions Compared

Old Version 7

changes.mady.by.user ABHISHEK VELDURTHY

Saved on

compared with

New Version Current

changes.mady.by.user David Svoboda

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: REM Cost Reform

While using pthread_key_The tss_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

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 terminateThe 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
langc

#include <pthread<threads.h>
#include <stdio<stdlib.h>
#include<malloc.h>

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


/* global Global key to the thread -specific datastorage */
pthreadtss_key_t key;
enum {max MAX_threadsTHREADS = 3 };

int main*get_data( 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)*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) {
            /*Handle Errorreturn -1;	/* Report 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,  if (thrd_success != tss_set(key, (void *)data);
}

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

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

  if (data != NULL) {
    /* 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.

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 function(void *dummy) {
  if (add_data() != 0) {
    return -1;	/* Report error */
  }
  print_data();
  return 0;
}

int main( void) )
{
    int i,ret;
    pthread thrd_t thread_id[maxMAX_threadsTHREADS];

     /* createCreate the key before creating the threads */
    pthread_key  if (thrd_success != tss_create( &key, NULL );

   )) {
    /* Handle error */
  }

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

     for (size_t i = 0; i < maxMAX_threadsTHREADS; i++) {
         ret = pthread    if (thrd_success != thrd_join(thread_id[i], NULL));
        if(ret !=0){
            /* Handle Errorerror */
     }
   }
   
 }
    pthreadtss_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  return 0;
}

Compliant Solution

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

Code Block
bgColor#ccccff
langc
#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();
{
    /* get this thread's global data from key */
    int *data = pthread_getspecific(key);
    /* Print data */
}free(tss_get(key));
  return 0;
}

/* ... Other functions are unchanged */

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.This compliant solution invokes a destructor function registered during the call to tss_create() to automatically free any thread-specific storage:

Code Block
bgColor#ccccff
langc

#include <pthread<threads.h>
#include <stdio<stdlib.h>
#include<malloc.h>

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

/* global Global key to the thread -specific datastorage */
pthreadtss_key_t key;
enum {max MAX_threadsTHREADS = 3 };

/* ... Other functions are unchanged */

void destructor(void * data) {
    pthread_setspecific(key, NULL); // thread specific should be cleared (could be sensitive)
    free(data);
}
 
int main( void) )
{
    int i;
    pthreadthrd_t thread_id[maxMAX_threadsTHREADS];

     /* createCreate the thread specific data key before creating the threads */
    pthread_key  if (thrd_success != tss_create( &key, destructor );

   )) {
    /* Handle error */
  }

  /* createCreate threadthreads that willwould usestore thespecific keystorage */
     for (size_t i = 0; i < maxMAX_threadsTHREADS; i++) {
        pthread    if (thrd_success != thrd_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 Errorerror */
           }
    }
    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  for (size_t i = 0; i < MAX_THREADS; i++) {
    if (thrd_success != thrd_join(thread_id[i], NULL)) {
      /* Handle error */
    }
  }

  tss_delete(key);
    /* Print Data */
}


return 0;
}


Risk Assessment

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

Rule

Severity

Likelihood

Remediation Cost

Detectable

Repairable

Priority

Level


POS45-C

medium

probable

medium

 

 

CON30-C

Medium

Unlikely

No

No

P2

L3

Automated Detection

ToolVersionCheckerDescription
Astrée
Include Page
Astrée_V
Astrée_V

Supported, but no explicit checker
CodeSonar
Include Page
CodeSonar_V
CodeSonar_V

ALLOC.LEAK

Leak

Coverity
Include Page
Coverity_V
Coverity_V
ALLOC_FREE_MISMATCHPartially implemented, correct implementation is more involved
Cppcheck Premium

Include Page
Cppcheck Premium_V
Cppcheck Premium_V

premium-cert-con30-c
Helix QAC

Include Page
Helix QAC_V
Helix QAC_V

C1780, C1781, C1782, C1783, C1784


Parasoft C/C++test
Include Page
Parasoft_V
Parasoft_V

CERT_C-CON30-a

Ensure resources are freed

Polyspace Bug Finder

Include Page
Polyspace Bug Finder_V
Polyspace Bug Finder_V

CERT C: Rule CON30-CChecks for thread-specific memory leak (rule fully covered)

Related Vulnerabilities

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


...

Image Added Image Added Image Added