Segmentation fault when nebmodule returns with something else than 0

[Andreas Ericsson]

Tobias Mucke wrote:
> Hi list,
> 
> found the problem, at least I think to have found the problem.
> Actually function neb_set_module_info does not operate on the
> nebmodule struct but on the mod_handle it got from its caller. The
> latter is from type lt_dlhandle. To set an info of type
> NEBMODULE_MODINFO_* does not make sense.
> Two problems arose by this and I fall into both. Writing informations
> to the module by neb_set_module_info works for some of the
> NEBMODULE_MODINFO_*. Some other lead to a segfault. The second problem
> is, if you have overwritten the NEBMODULE_MODINFO_TITLE you overwrite
> the filename of the lt_dlhandle and the module can't be unloaded
> anymore.
> 
> I have written a patch. Since I am not a very experienced C programmer
> please review it carefully before applying to official Nagios code.
> 
> 
> 
> --- nagios-3.0b7/base/nebmods.c 2007-10-22 15:39:30.000000000 +0200
> +++ nagios-3.0b7-mod/base/nebmods.c     2007-11-27 07:35:52.000000000 +0100
> @@ -338,24 +338,29 @@
>  /****************************************************************************/
> 
>  /* sets module information */
> -int neb_set_module_info(void *handle, int type, char *data){
> -       nebmodule *mod=NULL;
> +int neb_set_module_info(void *mod_handle, int type, char *data){
> +       nebmodule *temp_module = NULL;
> 
> -       if(handle==NULL)
> +       if(mod_handle==NULL)
>                 return NEBERROR_NOMODULE;
> 
>         /* check type */
>         if(type<0 || type>=NEBMODULE_MODINFO_NUMITEMS)
>                 return NEBERROR_MODINFOBOUNDS;
> 
> -       /* get the module */
> -       mod=(nebmodule *)handle;
> +       /* find corresponding nebmodule */
> +       for(temp_module=neb_module_list;temp_module;temp_module=temp_module->next){
> +               if((void *)temp_module->module_handle == (void *)mod_handle)

The typecasts here aren't necessary. Anal compilers might want the lhs one,
but none of them are helped by the rhs one.

> +                       break;
> +       }
> +       if(temp_module==NULL)
> +               return NEBERROR_BADMODULEHANDLE;
> 
>         /* free any previously allocated memory */
> -       my_free(mod->info[type]);
> +       my_free(temp_module->info[type]);
> 
>         /* allocate memory for the new data */
> -       if((mod->info[type]=(char *)strdup(data))==NULL)
> +       if((temp_module->info[type]=(char *)strdup(data))==NULL)
>                 return NEBERROR_NOMEM;
> 
>         return OK;
> 

Other than that, it looks obviously correct, although I wonder if it
wouldn't be better to actually pass the real nebmodule struct to the
init function. I guess that's what the entire idea was initially, or
Ethan wouldn't have missed that one in set_module_info().

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/