Skip to content

Commit

Permalink
pua: Fix several locking & memory access bugs
Browse files Browse the repository at this point in the history
Several issues with publ_cback_func():
    * 1 x extra lock release upon breaking "send publish" loop
    * N x extra lock releases inside the loop, per each failed publish
    * invalid memory access on "presentity" ptr after send_publish_int()

(cherry picked from commit 9957ff2)
  • Loading branch information
liviuchircu committed Jul 4, 2017
1 parent 4ba179d commit e32975f
Showing 1 changed file with 20 additions and 8 deletions.
28 changes: 20 additions & 8 deletions modules/pua/send_publish.c
Expand Up @@ -46,7 +46,14 @@
#include "pua_callback.h"
#include "event_list.h"

int send_publish_int(ua_pres_t* presentity, publ_info_t* publ,
/**
* !! IMPORTANT !!
*
* - MUST be called with lock grabbed on "hash_index"
* - This lock is _guaranteed_ to be released upon return
* - DO NOT rely on "presentity" anymore after calling this function
*/
int send_publish_int(ua_pres_t *presentity, publ_info_t* publ,
pua_event_t* ev, int hash_index);

str* publ_build_hdr(int expires, pua_event_t* ev, str* content_type, str* etag,
Expand Down Expand Up @@ -347,27 +354,32 @@ void publ_cback_func(struct cell *t, int type, struct tmcb_params *ps)
presentity->cb_param = NULL;
}
presentity->waiting_reply = 0;
while(presentity->pending_publ)

/* attempt to send out a single queued PUBLISH */
while (presentity->pending_publ)
{
publ_t* pending_publ = presentity->pending_publ;
publ_info_t* publ = construct_pending_publ(presentity);

/* if unable to construct the info, simply drop this PUBLISH */
if(publ == NULL)
{
LM_ERR("Failed to create publish record\n");
lock_release(&HashT->p_records[hash_index].lock);
presentity->pending_publ = pending_publ->next;
shm_free(pending_publ);
continue;
}
LM_DBG("Found pending publish\n");
presentity->pending_publ = 0;

presentity->waiting_reply = 1;
presentity->pending_publ = pending_publ->next;

send_publish_int(presentity, publ, get_event(presentity->event),
presentity->hash_index);
pkg_free(publ);
presentity->pending_publ = pending_publ->next;

shm_free(pending_publ);
break;
pkg_free(publ);

return;
}

lock_release(&HashT->p_records[hash_index].lock);
Expand Down

0 comments on commit e32975f

Please sign in to comment.