From 6e848765ba93fded6d9f5e064fb571fbe07fb533 Mon Sep 17 00:00:00 2001
From: Iryna Antsyferova <iryna.antsyferova@iopsys.eu>
Date: Tue, 6 Aug 2024 09:17:32 +0000
Subject: [PATCH] Fix crash during internal calls, REF 14916
The crash happens randomly at internal calls. It seems like sometimes asterisk-chan-voicemngr
hangs due to some race conditions (ubus calls not responding anymore) and sometimes it crashes
in random places of the code.
What is done:
1. Refactor function for retrieving RTP statistics - add handling of ubus context the initial
fix for ubus calls race condition was done in 025ea9914b4f367fa9c463054475e408f1f1c0ab line:
`ret = ubus_invoke(get_shared_context(__FUNCTION__), endpt_id, "rtp_stats", bb.head,
ubus_call_answer_rtp_stats, sub, 500);` The 'sub' parameter is passed as a context for the ubus
call and needs to be handled in the ubus_call_answer_rtp_stats callback function. This part was
missed in the initial commit.
2. Fixed parsing of blob data for ubus method callbacks. The "sip_client_status" and
"call_status" ubus methods were wrongly copy-pasted from the event ubus method, which could lead
to potential memory corruption during the parsing of the ubus response.
3. Added error handling for null subchannel. The subchannel is now validated in all parts of the
code, but it was previously missed in some areas.
---
src/channels/chan_voicemngr.c | 112 ++++++++++++++++++++++------------
1 file changed, 72 insertions(+), 40 deletions(-)
diff --git a/src/channels/chan_voicemngr.c b/src/channels/chan_voicemngr.c
index 9336c72..195b73e 100644
--- a/src/channels/chan_voicemngr.c
+++ b/src/channels/chan_voicemngr.c
@@ -135,7 +135,7 @@ static struct ast_channel *chan_voicemngr_new(struct chan_voicemngr_subchannel *
struct ast_format_cap *format);
static int handle_dialtone_timeout(const void *data);
static int handle_congestion_timeout(const void *data);
-static int endpt_get_rtp_stats(int line);
+static int endpt_get_rtp_stats(struct chan_voicemngr_subchannel *sub);
static int is_call_waiting_enabled(const char *sip_account);
static int has_call_in_sip_client(const char *sip_account);
static struct ast_format *map_rtpname_to_format(char* name);
@@ -847,10 +847,15 @@ static void chan_voicemngr_modify_codec(struct chan_voicemngr_subchannel *sub) {
static int chan_voicemngr_indicate(struct ast_channel *ast, int condition, const void *data, size_t datalen)
{
struct chan_voicemngr_subchannel *sub = ast_channel_tech_pvt(ast);
- struct chan_voicemngr_subchannel *sub_peer = chan_voicemngr_subchannel_get_peer(sub);
+ struct chan_voicemngr_subchannel *sub_peer;
struct ast_bridge_channel *play_bridge_channel;
int res = 0;
+ if (!sub || !sub->parent) {
+ ast_log(LOG_ERROR, "Failed to get peer subchannel\n");
+ return -1;
+ }
+ sub_peer = chan_voicemngr_subchannel_get_peer(sub);
ast_log(LOG_NOTICE, "Channel %s gets an indication, condition = %d, sub->channel_state: %s, sub->codec: %d,ast_channel_codec_get(ast): %s\n",
ast ? ast_channel_name(ast) : "",
condition, sub ? state2str(sub->channel_state) : "", sub ? sub->codec : -1, ast ? ast_channel_codec_get(ast) : "");
@@ -1121,7 +1126,7 @@ static int chan_voicemngr_getRtpStats(struct ast_channel *ast)
if (sub->parent) {
pvt_lock(sub->parent, "chan_voicemngr_getRtpStats");
- if (endpt_get_rtp_stats(sub->parent->line_id)) {
+ if (endpt_get_rtp_stats(sub)) {
ast_log(LOG_WARNING, "Unable to get RTP statistics\n");
}
pvt_unlock(sub->parent);
@@ -1247,6 +1252,10 @@ static int chan_voicemngr_senddigit_begin(struct ast_channel *ast, char digit)
//ast_debug(5, "DTMF send begin: %c\n", digit);
sub = ast_channel_tech_pvt(ast);
+ if (!sub || !sub->parent) {
+ ast_log(LOG_ERROR, "Failed to get peer subchannel\n");
+ return -1;
+ }
pvt_lock(sub->parent, "DTMF senddigit_begin");
snprintf(signal, sizeof(signal), "dtmf%c", digit);
@@ -1265,6 +1274,10 @@ static int chan_voicemngr_senddigit_end(struct ast_channel *ast, char digit, uns
//ast_debug(5, "DTMF send end: %c, %d ms\n", digit, duration);
sub = ast_channel_tech_pvt(ast);
+ if (!sub || !sub->parent) {
+ ast_log(LOG_ERROR, "Failed to get peer subchannel\n");
+ return -1;
+ }
pvt_lock(sub->parent, "DTMF senddigit_end");
snprintf(signal, sizeof(signal), "dtmf%c", digit);
@@ -1347,6 +1360,11 @@ static int chan_voicemngr_call(struct ast_channel *chan, const char *dest, int t
sub = ast_channel_tech_pvt(chan);
int sip_client_id = -1;
+ if (!sub || !sub->parent) {
+ ast_log(LOG_ERROR, "Failed to get peer subchannel\n");
+ return -1;
+ }
+
ast_debug(1, "line %d\n", sub->parent->line_id);
sscanf(sub->parent->context, "sip%d", &sip_client_id);
ast_localtime(&UtcTime, &tm, NULL);
@@ -1467,7 +1485,7 @@ static int chan_voicemngr_hangup(struct ast_channel *ast)
struct ast_channel *peer_owner;
int conf_timer_removed = 0;
- if (!sub) {
+ if (!sub || !sub->parent) {
ast_log(LOG_WARNING, "Asked to hangup channel not connected\n");
return 0;
}
@@ -1642,8 +1660,14 @@ static int chan_voicemngr_hangup(struct ast_channel *ast)
static int chan_voicemngr_answer(struct ast_channel *ast)
{
struct chan_voicemngr_subchannel *sub = ast_channel_tech_pvt(ast);
- struct chan_voicemngr_pvt *pvt = sub->parent;
+ struct chan_voicemngr_pvt *pvt;
+ if (!sub || !sub->parent) {
+ ast_log(LOG_ERROR, "Failed to get peer subchannel\n");
+ return -1;
+ }
+
+ pvt = sub->parent;
ast_debug(1, "chan_voicemngr_answer(%s)\n", ast_channel_name(ast));
pvt_lock(pvt, "TELCHAN answer");
@@ -1755,6 +1779,11 @@ static int chan_voicemngr_write(struct ast_channel *ast, struct ast_frame *frame
audio_packet_t *ap;
int sip_client_id = -1;
+ if (!sub || !sub->parent) {
+ ast_log(LOG_ERROR, "Failed to get peer subchannel\n");
+ return -1;
+ }
+
if (ast_channel_state(ast) != AST_STATE_UP && ast_channel_state(ast) != AST_STATE_RING) {
/* Silently ignore packets until channel is up */
return 0;
@@ -4858,29 +4887,32 @@ static int endpt_get_count(void) {
static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struct blob_attr *msg) {
struct blob_attr *tb[__MAX_RTP_STATS];
uint16_t lineId = 0;
- struct chan_voicemngr_pvt *p = NULL;
- struct chan_voicemngr_subchannel *sub = NULL;
+ struct chan_voicemngr_subchannel *sub = (struct chan_voicemngr_subchannel *)req->priv;
int sip_client_id = -1;
uint16_t overruns = 0;
uint16_t underruns = 0;
+ if (!sub) {
+ ast_log(LOG_ERROR, "No active subchannel\n");
+ return;
+ }
+
ast_log(LOG_DEBUG, "thread %d: got answer from voicemngr on rtp_stats ubus call.\n", ast_get_tid());
- blobmsg_parse(endpt_rtp_stats_policy, __MAX_RTP_STATS, tb, blob_data(msg), blob_len(msg));
+ if (blobmsg_parse(endpt_rtp_stats_policy, __MAX_RTP_STATS, tb, blob_data(msg), blob_len(msg))) {
+ ast_log(LOG_ERROR, "Failed to parse rtp_stats answer\n");
+ return;
+ }
+
if (tb[RTP_STATS_LINE_ID])
lineId = blobmsg_get_u16(tb[RTP_STATS_LINE_ID]);
- p = chan_voicemngr_get_pvt_from_lineid(iflist, lineId);
- if (!p) {
- ast_log(LOG_ERROR, "No pvt with the line_id %d found!\n", lineId);
+ if (lineId != sub->parent->line_id) {
+ ast_log(LOG_ERROR, "Got rtp stats responce for other line. Should not happen\n");
return;
}
- sub = chan_voicemngr_get_active_subchannel(p);
- if (!sub) {
- ast_log(LOG_ERROR, "No active subchannel to write rtp stats!\n");
- return;
- }
+ sip_client_id = chan_voicemngr_get_sip_client_id(sub);
if (tb[RTP_STATS_LOCAL_BURST_DENSITY])
sub->rtp_stats.localBurstDensity = blobmsg_get_u16(tb[RTP_STATS_LOCAL_BURST_DENSITY]);
@@ -4918,7 +4950,6 @@ static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struc
sub->rtp_stats.discarded = blobmsg_get_u32(tb[RTP_STATS_DISCARDED]);
if (tb[RTP_STATS_LOST]) {
sub->rtp_stats.lost = blobmsg_get_u32(tb[RTP_STATS_LOST]);
- sip_client_id = chan_voicemngr_get_sip_client_id(sub);
if (sip_client_id >= 0 && sip_client_id < MAX_SIP_CLIENTS) {
line_stats[sip_client_id].pktslost += sub->rtp_stats.lost;
}
@@ -4941,14 +4972,12 @@ static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struc
sub->rtp_stats.averageRoundTripDelay = blobmsg_get_u32(tb[RTP_STATS_AVERAGE_ROUND_TRIP_DELAY]);
if (tb[RTP_STATS_OVERRUNS]) {
overruns = blobmsg_get_u16(tb[RTP_STATS_OVERRUNS]);
- sip_client_id = chan_voicemngr_get_sip_client_id(sub);
if (sip_client_id >= 0 && sip_client_id < MAX_SIP_CLIENTS) {
line_stats[sip_client_id].total_overruns += overruns;
}
}
if (tb[RTP_STATS_UNDERRUNS]) {
underruns = blobmsg_get_u16(tb[RTP_STATS_UNDERRUNS]);
- sip_client_id = chan_voicemngr_get_sip_client_id(sub);
if (sip_client_id >= 0 && sip_client_id < MAX_SIP_CLIENTS) {
line_stats[sip_client_id].total_underruns += underruns;
}
@@ -4978,24 +5007,15 @@ static void ubus_call_answer_rtp_stats(struct ubus_request *req, int type, struc
overruns, underruns);
}
-static int endpt_get_rtp_stats(int line) {
+static int endpt_get_rtp_stats(struct chan_voicemngr_subchannel *sub) {
struct blob_buf bb;
int ret;
- struct chan_voicemngr_pvt *p = NULL;
- struct chan_voicemngr_subchannel *sub = NULL;
/*
* Reset rtp_stats first because ubus_call_answer_rtp_stats() will not be called if "ubus call endpt rtp_stats" fails,
* e.g. an unanswered incoming call on which the connection is not created. In this case, all RTP statistics counters
* shall be zeros.
*/
- p = chan_voicemngr_get_pvt_from_lineid(iflist, line);
- if (!p) {
- ast_log(LOG_ERROR, "No pvt with the line %d found!\n", line);
- return -1;
- }
-
- sub = chan_voicemngr_get_active_subchannel(p);
if (!sub) {
ast_log(LOG_ERROR, "No active subchannel to get rtp stats!\n");
return -1;
@@ -5011,10 +5031,10 @@ static int endpt_get_rtp_stats(int line) {
return -1;
}
- blobmsg_add_u32(&bb, "line", line);
+ blobmsg_add_u32(&bb, "line", sub->parent->line_id);
blobmsg_add_u8(&bb, "reset", 1); // always reset RTP stats after get them
- ast_log(LOG_DEBUG, "thread %d: ubus call endpt rtp_stats \"{'line':%d,'reset':true}\"", ast_get_tid(), line);
- ret = ubus_invoke(get_shared_context(__FUNCTION__), endpt_id, "rtp_stats", bb.head, ubus_call_answer_rtp_stats, sub, 500);
+ ast_log(LOG_DEBUG, "thread %d: ubus call endpt rtp_stats \"{'line':%d,'reset':true}\"", ast_get_tid(), sub->parent->line_id);
+ ret = ubus_invoke(get_shared_context(__FUNCTION__), endpt_id, "rtp_stats", bb.head, ubus_call_answer_rtp_stats, sub, 50);
blob_buf_free(&bb);
if (ret != UBUS_STATUS_OK) {
@@ -5036,8 +5056,12 @@ static int asterisk_event(struct ubus_context *ctx, struct ubus_object *obj,
struct endpt_event *ev;
int line;
ast_log(LOG_DEBUG, "Event received!\n");
- blobmsg_parse(asterisk_event_policy, __EVENT_MAX,
- tb, blob_data(msg), blob_len(msg));
+ if (blobmsg_parse(asterisk_event_policy, __EVENT_MAX,
+ tb, blob_data(msg), blob_len(msg))) {
+ ast_log(LOG_ERROR, "Failed to parse asterisk event\n");
+ return UBUS_STATUS_INVALID_ARGUMENT;
+ }
+
if (!tb[EVENT_LINE_ID] || !tb[EVENT_TYPE]) {
ast_log(LOG_DEBUG, "Wrong param: tb[EVENT_LINE_ID]: %p, tb[EVENT_TYPE]: %p\n", (void*)tb[EVENT_LINE_ID], (void*)tb[EVENT_TYPE]);
@@ -5071,7 +5095,7 @@ static int asterisk_call_status(struct ubus_context *ctx, struct ubus_object *ob
struct ubus_request_data *req, const char *method,
struct blob_attr *msg)
{
- struct blob_attr *tb[__EVENT_MAX];
+ struct blob_attr *tb[__CALL_STATUS_MAX];
enum ubus_msg_status res = UBUS_STATUS_UNKNOWN_ERROR;
struct blob_buf blob;
int extension = -1;
@@ -5079,8 +5103,12 @@ static int asterisk_call_status(struct ubus_context *ctx, struct ubus_object *ob
char sipAccount[6];
void *table_stats, *table_rtp;
- blobmsg_parse(asterisk_call_status_policy, __CALL_STATUS_MAX,
- tb, blob_data(msg), blob_len(msg));
+ if (blobmsg_parse(asterisk_call_status_policy, __CALL_STATUS_MAX,
+ tb, blob_data(msg), blob_len(msg))) {
+ ast_log(LOG_ERROR, "Failed to parse call status\n");
+ return UBUS_STATUS_INVALID_ARGUMENT;
+ }
+
memset(&blob, 0, sizeof(blob));
if(blob_buf_init(&blob, 0))
@@ -5162,13 +5190,17 @@ static int asterisk_sip_client_status(struct ubus_context *ctx, struct ubus_obje
struct ubus_request_data *req, const char *method,
struct blob_attr *msg)
{
- struct blob_attr *tb[__EVENT_MAX];
+ struct blob_attr *tb[__SIP_CLIENT_MAX];
struct blob_buf blob;
const char *sip_alias;
char *status;
- blobmsg_parse(asterisk_sip_client_status_policy, __SIP_CLIENT_MAX,
- tb, blob_data(msg), blob_len(msg));
+ if (blobmsg_parse(asterisk_sip_client_status_policy, __SIP_CLIENT_MAX,
+ tb, blob_data(msg), blob_len(msg))) {
+ ast_log(LOG_ERROR, "Failed to parse sip client status\n");
+ return UBUS_STATUS_INVALID_ARGUMENT;
+ }
+
memset(&blob, 0, sizeof(blob));
if(blob_buf_init(&blob, 0))
--
GitLab