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

[Charles Chance]

Hi Daniel,

Thanks for your reply.

On 8 January 2015 at 15:15, Daniel-Constantin Mierla <miconda at gmail.com>
wrote:

>  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.
>
>
Will try to investigate further.


> 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.
>
>
Agreed - should be done anyway, regardless of whether it is related or not
to the reported issue.

Best,
Charles



>
> 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>
> wrote:
>
>> Module: kamailio
>> Branch: master
>> Commit: cc5f96f9c847d285085b0b9809ff0db76ea0a835
>> URL:
>> https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db76ea0a835
>>
>> Author: Daniel-Constantin Mierla <miconda at gmail.com>
>> Committer: Daniel-Constantin Mierla <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
>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>
>
>
>
> 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 listsr-dev at lists.sip-router.orghttp://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>
> --
> Daniel-Constantin Mierlahttp://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
>
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>

-- 
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20150108/5e4b07c0/attachment.html>