[Nagios-devel] [PATCH] Fix event broker callback functions self

[Guest]

--=-trYTbNe8mVMPLBt1ojzb
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

Hello everyone,

I stumbled into this while debugging the relatively new Merlin event
broker module. It resulted in a segfault and took a while to hunt down.

If a Nagios event broker callback function de-registers itself (via
neb_deregister_callback) while it is being called (from
neb_make_callbacks) there is a pointer problem.

neb_make_callbacks walks the callback linked list like this:

for(temp_callback=neb_callback_list[callback_type];temp_callback!=NULL;temp_callback=temp_callback->next){
callbackfunc=temp_callback->callback_func;
cbresult=callbackfunc(callback_type,data);
}

If callbackfunc() calls neb_deregister_callback then the memory address
pointed to by temp_callback is free()d and no longer valid, making the
temp_callback=temp_callback->next not valid (though if the memory hasn't
been overwritten yet it may work).

I have attached a simple patch which I believe addresses the issues and
makes it safe to let the callback functions de-register themselves.

Thanks,
Sean


--=-trYTbNe8mVMPLBt1ojzb
Content-Disposition: attachment;
filename="nagios-fix-callback-self-deregistering.patch"
Content-Type: text/x-patch; name="nagios-fix-callback-self-deregistering.patch";
charset="UTF-8"
Content-Transfer-Encoding: 7bit

diff -ru nagios-3.2.0-orig/base/nebmods.c nagios-3.2.0/base/nebmods.c
--- nagios-3.2.0-orig/base/nebmods.c 2008-11-30 12:22:58.000000000 -0500
+++ nagios-3.2.0/base/nebmods.c 2009-10-30 17:09:19.765012219 -0400
@@ -561,7 +561,7 @@

/* make callbacks to modules */
int neb_make_callbacks(int callback_type, void *data){
- nebcallback *temp_callback=NULL;
+ nebcallback *temp_callback=NULL, *next_callback=NULL;
int (*callbackfunc)(int,void *);
register int cbresult=0;
int total_callbacks=0;
@@ -577,7 +577,10 @@
log_debug_info(DEBUGL_EVENTBROKER,1,"Making callbacks (type %d)...\n",callback_type);

/* make the callbacks... */
- for(temp_callback=neb_callback_list[callback_type];temp_callback!=NULL;temp_callback=temp_callback->next){
+ for(temp_callback=neb_callback_list[callback_type];temp_callback!=NULL;temp_callback=next_callback){
+ /* Save temp_callback->next because if the callback function de-registers itself temp_callback's */
+ /* pointer isn't guaranteed to be usable anymore (neb_deregister_callback will free() it) */
+ next_callback=temp_callback->next;
callbackfunc=temp_callback->callback_func;
cbresult=callbackfunc(callback_type,data);


--=-trYTbNe8mVMPLBt1ojzb--






This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]