Invalid write of size 8 after a malloc

Question

I am currently working on a project for my school and I am having issues with my code. The purpose of the programm is to implement a plugin manager that search in a directory all "*_plugin.so" file and add a plugin descriptor into a simple linked list.

The C code :

      //struct of a single node
      typedef
      struct _chainon_ { 
        plugin_descriptor * desc;
        struct _chainon_ * next;
    } Chainon;

      // manager that contains sentry node & number of plugins contained by the list
      struct plugin_manager_t {
        int nbElts;  
        Chainon * sentinel;
    };

  typedef 
  struct {
    const char *    m_name;     // nom du filtre
    const char *    m_description;  // description de l'effet du filtre
    filter_function m_filtre;       // fonction de réalisation du filtre
  } plugin_descriptor;

Now the register_plugin function, it is called while the programm find a new plugin in the directory, it calls an init_ function that call register_plugin :

  void
  init_(plugin_manager * pm)
  {
    register_plugin(pm,
            "null_filter",
            "Exemple de filtre inutile",
            null_filter);
  }

and then it is supposed to add the new plug to the list :

  void
  register_plugin(plugin_manager * pm,
          const char filter_name[],
          const char filter_description[],
          filter_function the_filter)
  {
      Chainon * n = (Chainon *)malloc(sizeof(Chainon)); //new node that i want to add to the linked list
      n->desc = NULL;
      n->next = NULL;
      n->desc->m_name = filter_name;
      n->desc->m_description = filter_description;
      n->desc->m_filtre = the_filter;
      Chainon * current = pm->sentinel;
      for(int i=0;i<pm->nbElts;i++){
        current=current->next;
        i++;
      }
      current->next = n;
  }

And that is what I am getting with valgrind while I execute this programm :

> ==7022== Invalid write of size 8
> ==7022==    at 0x4015A7: register_plugin (pluginmanager.cc:165)
> ==7022==    by 0x66E1BDC: init_ (null_filter_plugin.cc:23)
> ==7022==    by 0x401483: discover_plugins (pluginmanager.cc:113)
> ==7022==    by 0x401187: main (main.cc:17)
> ==7022==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
> ==7022== 
> ==7022== 
> ==7022== Process terminating with default action of signal 11 (SIGSEGV)
> ==7022==  Access not within mapped region at address 0x0
> ==7022==    at 0x4015A7: register_plugin (pluginmanager.cc:165)
> ==7022==    by 0x66E1BDC: init_ (null_filter_plugin.cc:23)
> ==7022==    by 0x401483: discover_plugins (pluginmanager.cc:113)
> ==7022==    by 0x401187: main (main.cc:17)
> ==7022==  If you believe this happened as a result of a stack
> ==7022==  overflow in your program's main thread (unlikely but
> ==7022==  possible), you can try to increase the size of the
> ==7022==  main thread stack using the --main-stacksize= flag.
> ==7022==  The main thread stack size used in this run was 8388608.

I am novice at C programming

But I do not understand why I could not initialize "n->desc->name" since I allocated the memory with malloc and then initialized everything to NULL ?

Any help would be appreciate !

Thank you

Answer 1

Your code has several problems, some of them are minor problems and others are causing the posted valgrind output,

1. Is not really a problem, it's just that you don't need to cast the return value of malloc()

   Chainon *n = malloc(sizeof(Chainon));
   

   is ok, no need for the cast.

2. You need to check that malloc() succeeded, not just assume that it did, under normal situations it will not fail, but in case of failure your program does not handle that, and in case it has some sensitive data that needs to be stored in the hard drive or any other situation where a clean exit is needed, you will cause a lot of problems to the program users, so you should ensure that your program exits cleanly, hence checking the return value of malloc() is a very good thing to do, just check against NULL right after every call to malloc() and handle that according to the situation where the failure occurs.

3. You don't allocate space for your struct members, every pointer must point to valid memory before dereferencing it, so you must ensure that it does point to valid memory, uninitialized pointers can't be checked so in cases where you are going to initialize the pointer after a possible check, initialize it to NULL.

   You do that in one case, but then you dereference the NULL pointer, which is undefined behavior.

Using all the recommendations above, your function has to be re-written like this^*

void
register_plugin(plugin_manager * pm,
                const char *const filter_name,
                const char *const filter_description,
                filter_function the_filter)
{
    Chainon           *chainon;
    plugin_descriptor *descriptor;
    Chainon           *current
    int                i;
    if (pm == NULL)
        return;
    chainon = malloc(sizeof(*chainon));
    if (chainon == NULL)
        return;
    chainon->next = NULL;

    descriptor = malloc(sizeof(*descriptor));
    if (descriptor == NULL)
     {
        free(chainon);
        return;
     }
    chainon->desc = descriptor;

    descriptor->m_name        = filter_name;
    descriptor->m_description = filter_description;
    descripotor->m_filtre     = the_filter;

    current = pm->sentinel;
    if (current == NULL)
        return;
    for(i = 0 ; ((i < pm->nbElts) && (current->next != NULL)) ; ++i)
        current = current->next;
    current->next = chainon;
}

^*_{Some of the things I changed are not really necessary. I just think it's better that way.}