From ee25bdfb5edd5dbf4449a74713d5999ee28e100e Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Wed, 17 Nov 2010 21:26:30 +0000 Subject: [PATCH] Merged revision 2015 from https://origsvn.digium.com/svn/libpri/branches/1.4 .......... r2015 | rmudgett | 2010-10-14 12:09:40 -0500 (Thu, 14 Oct 2010) | 16 lines 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) issue_18032_v1.4.11.4.patch uploaded by rmudgett (license 664) Tested by: rmudgett .......... git-svn-id: https://origsvn.digium.com/svn/libpri/tags/1.4.11.5@2136 2fbb986a-6c06-0410-b554-c9c1f0a7f128 --- pri.c | 102 ++++++++++++++++++++++++++++++++++--------------- pri_facility.c | 2 +- pri_internal.h | 14 +++++++ q931.c | 70 +++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 32 deletions(-) diff --git a/pri.c b/pri.c index 65ace9d..b186db6 100644 --- a/pri.c +++ b/pri.c @@ -365,8 +365,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) @@ -575,74 +582,83 @@ 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); } @@ -738,7 +754,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; } @@ -804,7 +820,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; } @@ -908,23 +924,31 @@ int pri_redirecting_update(struct pri *ctrl, q931_call *call, const struct pri_p /* 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 int pri_channel_bridge(q931_call *call1, q931_call *call2) { - 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; + } /* Make sure we have compatible switchtypes */ if (call1->pri->switchtype != call2->pri->switchtype) @@ -976,8 +1000,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; @@ -1055,8 +1080,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); @@ -1075,8 +1102,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); @@ -1093,8 +1122,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); } @@ -1104,8 +1134,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); @@ -1344,11 +1377,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); } @@ -1497,7 +1536,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); @@ -1505,7 +1544,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); @@ -1513,7 +1552,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); @@ -1521,7 +1560,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); @@ -1529,7 +1568,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); @@ -1537,7 +1576,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); @@ -1545,8 +1584,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); } @@ -1565,7 +1605,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_facility.c b/pri_facility.c index 59c22f8..94dbd48 100644 --- a/pri_facility.c +++ b/pri_facility.c @@ -3180,7 +3180,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; } diff --git a/pri_internal.h b/pri_internal.h index 24ab005..1b98f6e 100644 --- a/pri_internal.h +++ b/pri_internal.h @@ -543,6 +543,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 c4a1f03..3cbb808 100644 --- a/q931.c +++ b/q931.c @@ -320,6 +320,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 *