From a2cb58d1066dd6042c7dc5655f85987b7bb4c6a5 Mon Sep 17 00:00:00 2001
From: Yalu Zhang <yalu.zhang@iopsys.eu>
Date: Thu, 30 Nov 2023 14:43:12 +0100
Subject: [PATCH] Fix a memory corruption issue related to ubus_free
In libubus, there is global variable defined as below. ubus_free() calls ubus_shutdown() which frees b->buf.
which can cause memory corrupt when there is more than thread calls ubus_free() at the same time. In order
to avoid this, ubus_free() shall be called only once when the program is about to exit. In other cases,
ubus_free_context() shall be called to only free context specific resources but keep global buffer b->buf
untouched.
---
src/channels/chan_voicemngr.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/src/channels/chan_voicemngr.c b/src/channels/chan_voicemngr.c
index 9fa24f3..11c7b1b 100644
--- a/src/channels/chan_voicemngr.c
+++ b/src/channels/chan_voicemngr.c
@@ -251,6 +251,25 @@ typedef struct shared_context {
} share_context_t;
static struct shared_context ctxs[MAX_CONTEXTS];
+/*
+ * In libubus, there is global variable defined as below. ubus_free() calls ubus_shutdown() which frees b->buf.
+ * which can cause memory corrupt when there is more than thread calls ubus_free() at the same time. In order
+ * to avoid this, ubus_free() shall be called only once when the program is about to exit. In other cases,
+ * ubus_free_context() shall be called to only free context specific resources but keep global buffer
+ * b->buf untouched.
+ *
+ * struct blob_buf b __hidden = {};
+ */
+static void ubus_free_context(struct ubus_context *ctx)
+{
+ if (!ctx)
+ return;
+ close(ctx->sock.fd);
+ uloop_timeout_cancel(&ctx->pending_timer);
+ free(ctx->msgbuf.data);
+ free(ctx);
+}
+
static struct ubus_context *get_shared_context(const char *func) {
int tid = ast_get_tid();
@@ -268,7 +287,7 @@ static struct ubus_context *get_shared_context(const char *func) {
for(int i=0; i < MAX_CONTEXTS; i++) {
if (ctxs[i].tid != 0 && ctxs[i].last_used < time(NULL) - 300) {
ast_log(LOG_DEBUG, "tid=%d, expiring context %d", tid, i);
- ubus_free(ctxs[i].ctx);
+ ubus_free_context(ctxs[i].ctx);
ctxs[i].tid = 0;
ctxs[i].last_used = 0;
ctxs[i].ctx = NULL;
@@ -673,7 +692,7 @@ static int chan_voicemngr_send_ubus_event(char *ev_name, int line)
memset(&blob, 0, sizeof(blob));
if(blob_buf_init(&blob, 0)) {
- ubus_free(ubusctx);
+ ubus_free_context(ubusctx);
return -1;
}
@@ -685,7 +704,7 @@ static int chan_voicemngr_send_ubus_event(char *ev_name, int line)
res = -1;
}
- ubus_free(ubusctx);
+ ubus_free_context(ubusctx);
blob_buf_free(&blob);
return res;
@@ -4363,6 +4382,12 @@ static int unload_module(void)
feature_access_code_clear();
ast_sched_context_destroy(sched);
+ /* Free the context and b->buf finally */
+ if (ctx) {
+ ubus_free(ctx);
+ ctx = NULL;
+ }
+
return 0;
}
@@ -4903,7 +4928,7 @@ static int endpt_get_rtp_stats(int line) {
ret = ubus_invoke(local_ctx, endpt_id, "rtp_stats", bb.head, ubus_call_answer_rtp_stats, NULL, 500);
blob_buf_free(&bb);
- ubus_free(local_ctx);
+ ubus_free_context(local_ctx);
if (ret != UBUS_STATUS_OK) {
ast_log(LOG_DEBUG, "ubus_invoke for rtp_stats failed with return value %d\n", ret);
--
GitLab