From 2045db6a6983efee2abb65863d50e7c67361df1d Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Thu, 14 Oct 2010 17:09:40 +0000 Subject: [PATCH] Segfault in pri_schedule_del() - ctrl value is invalid. Validate the given call pointer in libpri API calls. If the call pointer is not an active call record then a complaint message is issued and the API call aborts. The call pointer is likely stale. This patch is defensive. More information is needed to figure out why Asterisk still has a call pointer during its hangup sequence. (closes issue #17522) (closes issue #18032) Reported by: schmoozecom Patches: issue_18032_v1.4.patch uploaded by rmudgett (license 664) Tested by: rmudgett git-svn-id: https://origsvn.digium.com/svn/libpri/branches/1.4@2015 2fbb986a-6c06-0410-b554-c9c1f0a7f128 --- pri.c | 104 ++++++++++++++++++++++++++++++++++--------------- pri_aoc.c | 10 ++--- pri_cc.c | 4 +- pri_facility.c | 6 +-- pri_internal.h | 14 +++++++ q931.c | 70 +++++++++++++++++++++++++++++++++ 6 files changed, 166 insertions(+), 42 deletions(-) diff --git a/pri.c b/pri.c index 97ca6dc..6cb0e0d 100644 --- a/pri.c +++ b/pri.c @@ -416,8 +416,15 @@ struct pri *__pri_new_tei(int fd, int node, int switchtype, struct pri *master, void pri_call_set_useruser(q931_call *c, const char *userchars) { - if (userchars) - libpri_copy_string(c->useruserinfo, userchars, sizeof(c->useruserinfo)); + /* + * There is a slight risk here if c is actually stale. However, + * if it is stale then it is better to catch it here than to + * write with it. + */ + if (!userchars || !pri_is_call_valid(NULL, c)) { + return; + } + libpri_copy_string(c->useruserinfo, userchars, sizeof(c->useruserinfo)); } void pri_sr_set_useruser(struct pri_sr *sr, const char *userchars) @@ -642,80 +649,89 @@ void pri_facility_enable(struct pri *pri) int pri_acknowledge(struct pri *pri, q931_call *call, int channel, int info) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_alerting(pri, call, channel, info); } int pri_proceeding(struct pri *pri, q931_call *call, int channel, int info) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_call_proceeding(pri, call, channel, info); } int pri_progress_with_cause(struct pri *pri, q931_call *call, int channel, int info, int cause) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_call_progress_with_cause(pri, call, channel, info, cause); } int pri_progress(struct pri *pri, q931_call *call, int channel, int info) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_call_progress(pri, call, channel, info); } int pri_information(struct pri *pri, q931_call *call, char digit) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_information(pri, call, digit); } int pri_keypad_facility(struct pri *pri, q931_call *call, const char *digits) { - if (!pri || !call || !digits || !digits[0]) + if (!pri || !pri_is_call_valid(pri, call) || !digits || !digits[0]) { return -1; + } return q931_keypad_facility(pri, call, digits); } int pri_notify(struct pri *pri, q931_call *call, int channel, int info) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_notify(pri, call, channel, info); } void pri_destroycall(struct pri *pri, q931_call *call) { - if (pri && call) + if (pri && pri_is_call_valid(pri, call)) { q931_destroycall(pri, call); - return; + } } int pri_need_more_info(struct pri *pri, q931_call *call, int channel, int nonisdn) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_setup_ack(pri, call, channel, nonisdn); } int pri_answer(struct pri *pri, q931_call *call, int channel, int nonisdn) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_connect(pri, call, channel, nonisdn); } int pri_connect_ack(struct pri *ctrl, q931_call *call, int channel) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } return q931_connect_acknowledge(ctrl, call, channel); @@ -821,7 +837,7 @@ int pri_connected_line_update(struct pri *ctrl, q931_call *call, const struct pr unsigned idx; struct q931_call *subcall; - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -887,7 +903,7 @@ int pri_redirecting_update(struct pri *ctrl, q931_call *call, const struct pri_p unsigned idx; struct q931_call *subcall; - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -1037,15 +1053,17 @@ void pri_status_req_rsp(struct pri *ctrl, int invoke_id, int status) /* deprecated routines, use pri_hangup */ int pri_release(struct pri *pri, q931_call *call, int cause) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_release(pri, call, cause); } int pri_disconnect(struct pri *pri, q931_call *call, int cause) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } return q931_disconnect(pri, call, cause); } #endif @@ -1054,8 +1072,14 @@ int pri_channel_bridge(q931_call *call1, q931_call *call2) { struct q931_call *winner; - if (!call1 || !call2) + /* + * There is a slight risk here if call1 or call2 is actually + * stale. However, if they are stale then it is better to catch + * it here than to write with these pointers. + */ + if (!pri_is_call_valid(NULL, call1) || !pri_is_call_valid(NULL, call2)) { return -1; + } winner = q931_find_winning_call(call1); if (!winner) { @@ -1122,8 +1146,9 @@ void pri_hangup_fix_enable(struct pri *ctrl, int enable) int pri_hangup(struct pri *pri, q931_call *call, int cause) { - if (!pri || !call) + if (!pri || !pri_is_call_valid(pri, call)) { return -1; + } if (cause == -1) /* normal clear cause */ cause = PRI_CAUSE_NORMAL_CLEARING; @@ -1201,8 +1226,10 @@ int pri_mwi_activate(struct pri *pri, q931_call *c, char *caller, int callerplan int calledplan) { struct pri_sr req; - if (!pri || !c) + + if (!pri || !pri_is_call_valid(pri, c)) { return -1; + } pri_sr_init(&req); pri_sr_set_connection_call_independent(&req); @@ -1221,8 +1248,10 @@ int pri_mwi_deactivate(struct pri *pri, q931_call *c, char *caller, int callerpl int calledplan) { struct pri_sr req; - if (!pri || !c) + + if (!pri || !pri_is_call_valid(pri, c)) { return -1; + } pri_sr_init(&req); pri_sr_set_connection_call_independent(&req); @@ -1239,8 +1268,9 @@ int pri_mwi_deactivate(struct pri *pri, q931_call *c, char *caller, int callerpl int pri_setup(struct pri *pri, q931_call *c, struct pri_sr *req) { - if (!pri || !c) + if (!pri || !pri_is_call_valid(pri, c)) { return -1; + } return q931_setup(pri, c, req); } @@ -1250,8 +1280,11 @@ int pri_call(struct pri *pri, q931_call *c, int transmode, int channel, int excl int calledplan, int ulayer1) { struct pri_sr req; - if (!pri || !c) + + if (!pri || !pri_is_call_valid(pri, c)) { return -1; + } + pri_sr_init(&req); pri_sr_set_caller(&req, caller, callername, callerplan, callerpres); pri_sr_set_called(&req, called, calledplan, 0); @@ -1494,11 +1527,17 @@ char *pri_dump_info_str(struct pri *ctrl) int pri_get_crv(struct pri *pri, q931_call *call, int *callmode) { + if (!pri || !pri_is_call_valid(pri, call)) { + return -1; + } return q931_call_getcrv(pri, call, callmode); } int pri_set_crv(struct pri *pri, q931_call *call, int crv, int callmode) { + if (!pri || !pri_is_call_valid(pri, call)) { + return -1; + } return q931_call_setcrv(pri, call, crv, callmode); } @@ -1655,7 +1694,7 @@ void pri_hold_enable(struct pri *ctrl, int enable) int pri_hold(struct pri *ctrl, q931_call *call) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } return q931_send_hold(ctrl, call); @@ -1663,7 +1702,7 @@ int pri_hold(struct pri *ctrl, q931_call *call) int pri_hold_ack(struct pri *ctrl, q931_call *call) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } return q931_send_hold_ack(ctrl, call); @@ -1671,7 +1710,7 @@ int pri_hold_ack(struct pri *ctrl, q931_call *call) int pri_hold_rej(struct pri *ctrl, q931_call *call, int cause) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } return q931_send_hold_rej(ctrl, call, cause); @@ -1679,7 +1718,7 @@ int pri_hold_rej(struct pri *ctrl, q931_call *call, int cause) int pri_retrieve(struct pri *ctrl, q931_call *call, int channel) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } return q931_send_retrieve(ctrl, call, channel); @@ -1687,7 +1726,7 @@ int pri_retrieve(struct pri *ctrl, q931_call *call, int channel) int pri_retrieve_ack(struct pri *ctrl, q931_call *call, int channel) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } return q931_send_retrieve_ack(ctrl, call, channel); @@ -1695,7 +1734,7 @@ int pri_retrieve_ack(struct pri *ctrl, q931_call *call, int channel) int pri_retrieve_rej(struct pri *ctrl, q931_call *call, int cause) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } return q931_send_retrieve_rej(ctrl, call, cause); @@ -1703,8 +1742,9 @@ int pri_retrieve_rej(struct pri *ctrl, q931_call *call, int cause) int pri_callrerouting_facility(struct pri *pri, q931_call *call, const char *dest, const char* original, const char* reason) { - if (!pri || !call || !dest) + if (!pri || !pri_is_call_valid(pri, call) || !dest) { return -1; + } return qsig_cf_callrerouting(pri, call, dest, original, reason); } @@ -1723,7 +1763,7 @@ int pri_reroute_call(struct pri *ctrl, q931_call *call, const struct pri_party_i struct q931_party_id local_caller; struct q931_party_redirecting reroute; - if (!ctrl || !call || !deflection) { + if (!ctrl || !pri_is_call_valid(ctrl, call) || !deflection) { return -1; } diff --git a/pri_aoc.c b/pri_aoc.c index 46993dd..23d7be6 100644 --- a/pri_aoc.c +++ b/pri_aoc.c @@ -1610,7 +1610,7 @@ static int aoc_e_encode(struct pri *ctrl, q931_call *call, const struct pri_subc int pri_aoc_de_request_response_send(struct pri *ctrl, q931_call *call, int response, int invoke_id) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -1629,7 +1629,7 @@ int pri_aoc_de_request_response_send(struct pri *ctrl, q931_call *call, int resp int pri_aoc_s_request_response_send(struct pri *ctrl, q931_call *call, int invoke_id, const struct pri_subcmd_aoc_s *aoc_s) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -1692,7 +1692,7 @@ int aoc_charging_request_send(struct pri *ctrl, q931_call *call, enum PRI_AOC_RE int pri_aoc_s_send(struct pri *ctrl, q931_call *call, const struct pri_subcmd_aoc_s *aoc_s) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -1711,7 +1711,7 @@ int pri_aoc_s_send(struct pri *ctrl, q931_call *call, const struct pri_subcmd_ao int pri_aoc_d_send(struct pri *ctrl, q931_call *call, const struct pri_subcmd_aoc_d *aoc_d) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -1729,7 +1729,7 @@ int pri_aoc_d_send(struct pri *ctrl, q931_call *call, const struct pri_subcmd_ao int pri_aoc_e_send(struct pri *ctrl, q931_call *call, const struct pri_subcmd_aoc_e *aoc_e) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } diff --git a/pri_cc.c b/pri_cc.c index 93089b0..407e24b 100644 --- a/pri_cc.c +++ b/pri_cc.c @@ -6803,7 +6803,7 @@ long pri_cc_available(struct pri *ctrl, q931_call *call) struct pri_cc_record *cc_record; long cc_id; - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } if (call->cc.record) { @@ -7811,7 +7811,7 @@ int pri_cc_call(struct pri *ctrl, long cc_id, q931_call *call, struct pri_sr *re { struct pri_cc_record *cc_record; - if (!ctrl || !call || !req) { + if (!ctrl || !pri_is_call_valid(ctrl, call) || !req) { return -1; } cc_record = pri_cc_find_by_id(ctrl, cc_id); diff --git a/pri_facility.c b/pri_facility.c index 158e39e..ed01b45 100644 --- a/pri_facility.c +++ b/pri_facility.c @@ -3632,7 +3632,7 @@ int pri_rerouting_rsp(struct pri *ctrl, q931_call *call, int invoke_id, enum PRI { enum rose_error_code rose_err; - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -3673,7 +3673,7 @@ int pri_rerouting_rsp(struct pri *ctrl, q931_call *call, int invoke_id, enum PRI int pri_transfer_rsp(struct pri *ctrl, q931_call *call, int invoke_id, int is_successful) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } @@ -3804,7 +3804,7 @@ static int rose_mcid_req_encode(struct pri *ctrl, q931_call *call) int pri_mcid_req_send(struct pri *ctrl, q931_call *call) { - if (!ctrl || !call) { + if (!ctrl || !pri_is_call_valid(ctrl, call)) { return -1; } if (call->cc.originated) { diff --git a/pri_internal.h b/pri_internal.h index 3b8c379..84f5a27 100644 --- a/pri_internal.h +++ b/pri_internal.h @@ -882,6 +882,20 @@ struct d_ctrl_dummy { struct q931_call dummy_call; }; +/*! + * \brief Check if the given call ptr is valid. + * + * \param ctrl D channel controller. + * \param call Q.931 call leg. + * + * \retval TRUE if call ptr is valid. + * \retval FALSE if call ptr is invalid. + */ +#define pri_is_call_valid(ctrl, call) \ + q931_is_call_valid(ctrl, call, __PRETTY_FUNCTION__, __LINE__) + +int q931_is_call_valid(struct pri *ctrl, struct q931_call *call, const char *func_name, unsigned long func_line); + extern int pri_schedule_event(struct pri *pri, int ms, void (*function)(void *data), void *data); extern pri_event *pri_schedule_run(struct pri *pri); diff --git a/q931.c b/q931.c index 0d40f9b..c26453a 100644 --- a/q931.c +++ b/q931.c @@ -322,6 +322,76 @@ static int q931_encode_channel(const q931_call *call) | held_call; } +/*! + * \brief Check if the given call ptr is valid. + * + * \param ctrl D channel controller. + * \param call Q.931 call leg. + * \param func_name Calling function name for debug tracing. (__PRETTY_FUNCTION__) + * \param func_line Calling function line number for debug tracing. (__LINE__) + * + * \retval TRUE if call ptr is valid. + * \retval FALSE if call ptr is invalid. + */ +int q931_is_call_valid(struct pri *ctrl, struct q931_call *call, const char *func_name, unsigned long func_line) +{ + struct q931_call *cur; + struct pri *gripe; + struct pri *link; + int idx; + + if (!call) { + return 0; + } + + if (!ctrl) { + /* Must use suspect ctrl from call ptr. */ + if (!call->pri) { + pri_message(NULL, + "!! %s() line:%lu Called with invalid call ptr (%p) (No ctrl)\n", + func_name, func_line, call); + return 0; + } + /* Find the master - He has the call pool */ + ctrl = PRI_MASTER(call->pri); + gripe = NULL; + } else { + /* Find the master - He has the call pool */ + ctrl = PRI_MASTER(ctrl); + gripe = ctrl; + } + + /* Check real call records. */ + for (cur = *ctrl->callpool; cur; cur = cur->next) { + if (call == cur) { + /* Found it. */ + return 1; + } + if (cur->outboundbroadcast) { + /* Check subcalls for call ptr. */ + for (idx = 0; idx < ARRAY_LEN(cur->subcalls); ++idx) { + if (call == cur->subcalls[idx]) { + /* Found it. */ + return 1; + } + } + } + } + + /* Check dummy call records. */ + for (link = ctrl; link; link = link->subchannel) { + if (link->dummy_call == call) { + /* Found it. */ + return 1; + } + } + + /* Well it looks like this is a stale call ptr. */ + pri_message(gripe, "!! %s() line:%lu Called with invalid call ptr (%p)\n", + func_name, func_line, call); + return 0; +} + /*! * \brief Initialize the given struct q931_party_name *