[sr-dev] git:master:cc5f96f9: dmq: safety check for peer_list when calling the callbacks

[Daniel-Constantin Mierla]

Hi Charles,

perhaps Olle can give more details about how he is running it. I was on
an irc-like chat with him reporting and testing, as I wanted to see if
there is anything that can be fixed before building v4.2.2.

On the other hand, not initializong peer_list and dmq_init_callback_done
to NULL and accessing them without check is exposing at list the problem
from destroy() function. Kamailio calls destroy even if the module was
not initialized. Not initializing dmq_init_callback_done at declaration
to NULL results in a random value for it (C doesn't do implicit
initialization), practically shm freeing an invalid pointer in destroy
function.

Cheers,
Daniel


On 08/01/15 16:02, Charles Chance wrote:
> Hi Daniel,
>
> I was wondering, did you find some way in which this was causing the
> issue?
>
> Theoretically, if the module is loaded successfully then peer_list
> cannot be null...
>
> (dmq.c)
>
> static int mod_init(void)
> {
> ...
>         /* load peer list - the list containing the module callbacks
> for dmq */
>         peer_list = init_peer_list();
>         if(peer_list==NULL) {
>                 LM_ERR("cannot initialize peer list\n");
>                 return -1;
>         }
> ...
>
> Also, peer_list should always contain the local notification peer,
> added during startup and prior to run_init_callbacks() ever being called:
>
> ...
>         /**
>          * add the dmq notification peer.
>          * the dmq is a peer itself so that it can receive node
> notifications
>          */
>         if(add_notification_peer()<0) {
>                 LM_ERR("cannot add notification peer\n");
>                 return -1;
>         }
> ...
>
>
> Am I missing something obvious?
>
> Best,
> Charles
>
>
> On 8 January 2015 at 13:21, Daniel-Constantin Mierla
> <miconda at gmail.com <mailto:miconda at gmail.com>> wrote:
>
>     Module: kamailio
>     Branch: master
>     Commit: cc5f96f9c847d285085b0b9809ff0db76ea0a835
>     URL:
>     https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db76ea0a835
>
>     Author: Daniel-Constantin Mierla <miconda at gmail.com
>     <mailto:miconda at gmail.com>>
>     Committer: Daniel-Constantin Mierla <miconda at gmail.com
>     <mailto:miconda at gmail.com>>
>     Date: 2015-01-08T14:20:58+01:00
>
>     dmq: safety check for peer_list when calling the callbacks
>
>     - can result in crashing if it is not set
>     - reported by Olle E. Johansson
>
>     ---
>
>     Modified: modules/dmq/notification_peer.c
>
>     ---
>
>     Diff: 
>     https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db76ea0a835.diff
>     Patch:
>     https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db76ea0a835.patch
>
>     ---
>
>     diff --git a/modules/dmq/notification_peer.c
>     b/modules/dmq/notification_peer.c
>     index 1d804bd..b493717 100644
>     --- a/modules/dmq/notification_peer.c
>     +++ b/modules/dmq/notification_peer.c
>     @@ -173,6 +173,10 @@ int extract_node_list(dmq_node_list_t*
>     update_list, struct sip_msg* msg)
>      int run_init_callbacks() {
>             dmq_peer_t* crt;
>
>     +       if(peer_list==0) {
>     +               LM_WARN("peer list is null\n");
>     +               return 0;
>     +       }
>             crt = peer_list->peers;
>             while(crt) {
>                     if (crt->init_callback) {
>
>
>     _______________________________________________
>     sr-dev mailing list
>     sr-dev at lists.sip-router.org <mailto:sr-dev at lists.sip-router.org>
>     http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>
>
>
> -- 
> *Charles Chance*
> Managing Director
>
> t. 0121 285 4400    m. 07932 063 891
>
> www.sipcentric.com <http://www.sipcentric.com/>
>
> Follow us on twitter @sipcentric <http://twitter.com/sipcentric>
>
> Sipcentric Ltd. Company registered in England & Wales no.
> 7365592. Registered office: Faraday Wharf, Innovation Birmingham
> Campus, Holt Street, Birmingham Science Park, Birmingham B7 4BB.
>
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20150108/a7df311c/attachment-0001.html>