git:master:cc5f96f9: dmq: safety check for peer_list when calling the callbacks

List overview All Threads
Download

newer

older

git:master:4d52cc05: dmq: more...

git:master:b27c38b7: dmq:...

Daniel-Constantin Mierla

8 Jan 2015 8 Jan '15

3:21 p.m.

Module: kamailio Branch: master Commit: cc5f96f9c847d285085b0b9809ff0db76ea0a835 URL: https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db… Author: Daniel-Constantin Mierla <miconda(a)gmail.com> Committer: Daniel-Constantin Mierla <miconda(a)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/cc5f96f9c847d285085b0b9809ff0db… Patch: https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db… --- 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) {

Show replies by date

Charles Chance

8 Jan 8 Jan

5:02 p.m.

New subject: git:master:cc5f96f9: dmq: safety check for peer_list when calling the callbacks

...

-- *Charles Chance* Managing Director t. 0121 285 4400 m. 07932 063 891 -- 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.

Daniel-Constantin Mierla

5:15 p.m.

New subject: git:master:cc5f96f9: dmq: safety check for peer_list when calling the callbacks

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(a)gmail.com <mailto:miconda@gmail.com>> wrote: Module: kamailio Branch: master Commit: cc5f96f9c847d285085b0b9809ff0db76ea0a835 URL: https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db… Author: Daniel-Constantin Mierla <miconda(a)gmail.com <mailto:miconda@gmail.com>> Committer: Daniel-Constantin Mierla <miconda(a)gmail.com <mailto:miconda@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/cc5f96f9c847d285085b0b9809ff0db… Patch: https://github.com/kamailio/kamailio/commit/cc5f96f9c847d285085b0b9809ff0db… --- 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(a)lists.sip-router.org <mailto:sr-dev@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(a)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

Charles Chance

5:34 p.m.

New subject: git:master:cc5f96f9: dmq: safety check for peer_list when calling the callbacks

Hi Daniel, Thanks for your reply. On 8 January 2015 at 15:15, Daniel-Constantin Mierla <miconda(a)gmail.com> wrote:

...

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(a)gmail.com> wrote:

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@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(a)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.

3611

days inactive

3611

days old

sr-dev@lists.kamailio.org

Manage subscription

3 comments

2 participants

tags (0)

participants (2)

Charles Chance
Daniel-Constantin Mierla