Skip to content

Commit

Permalink
rtp_relay: proper cleanup of a copy context
Browse files Browse the repository at this point in the history 
When a copy context was deleted, a dangling pointer would have remained
in the session - this would lead to a crash, since it would access
invalid memory.

Thanks go to Rob Moore from Dubber for reporting this!

(cherry picked from commit 4efc482)
  • Loading branch information
@razvancrainea
razvancrainea committed Feb 22, 2024
1 parent 0a7c2f2 commit 4b21232
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 18 deletions.
1 change: 1 addition & 0 deletions modules/rtp_relay/rtp_relay.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ struct rtp_relay_funcs {
struct rtp_relay_server *server, void *ctx, str *flags);
int (*copy_serialize)(void *ctx, bin_packet_t *packet);
int (*copy_deserialize)(void **ctx, bin_packet_t *packet);
void (*copy_release)(void **ctx);
};

struct rtp_relay_hooks {
Expand Down
42 changes: 27 additions & 15 deletions modules/rtp_relay/rtp_relay_ctx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ static struct {
{ str_init("disabled"), RTP_RELAY_FLAGS_DISABLED },
};

typedef struct rtp_copy_ctx {
str id;
void *ctx;
struct rtp_relay *relay;
struct list_head list;
} rtp_copy_ctx;


static int rtp_relay_offer(struct rtp_relay_session *info,
struct rtp_relay_ctx *ctx, struct rtp_relay_sess *sess,
int leg, str *body);
Expand Down Expand Up @@ -226,6 +234,16 @@ static void rtp_relay_ctx_release_leg(struct rtp_relay_leg *leg)
rtp_relay_ctx_free_leg(leg);
}

static void rtp_copy_ctx_free(struct rtp_copy_ctx *copy_ctx)
{
if (!copy_ctx)
return;
if (copy_ctx->ctx)
copy_ctx->relay->funcs.copy_release(&copy_ctx->ctx);
list_del(&copy_ctx->list);
shm_free(copy_ctx);
};

static void rtp_relay_ctx_free_sess(struct rtp_relay_ctx *ctx, struct rtp_relay_sess *s)
{
if (ctx->established == s)
Expand Down Expand Up @@ -261,6 +279,8 @@ static void rtp_relay_ctx_free(struct rtp_relay_ctx *ctx)

list_for_each_safe(it, safe, &ctx->sessions)
rtp_relay_ctx_free_sess(ctx, list_entry(it, struct rtp_relay_sess, list));
list_for_each_safe(it, safe, &ctx->copy_contexts)
rtp_copy_ctx_free(list_entry(it, struct rtp_copy_ctx, list));

lock_start_write(rtp_relay_contexts_lock);
if (list_is_valid(&ctx->list))
Expand Down Expand Up @@ -752,12 +772,6 @@ static void rtp_relay_b2b_new_local(struct cell* t, int type, struct tmcb_params
} \
} while (0)

typedef struct rtp_copy_ctx {
str id;
void *ctx;
struct list_head list;
} rtp_copy_ctx;

rtp_copy_ctx *rtp_copy_ctx_get(struct rtp_relay_ctx *ctx, str *id)
{
struct list_head *it;
Expand All @@ -771,7 +785,7 @@ rtp_copy_ctx *rtp_copy_ctx_get(struct rtp_relay_ctx *ctx, str *id)
return NULL;
}

rtp_copy_ctx *rtp_copy_ctx_new(struct rtp_relay_ctx *ctx, str *id)
rtp_copy_ctx *rtp_copy_ctx_new(struct rtp_relay_ctx *ctx, struct rtp_relay *relay, str *id)
{

rtp_copy_ctx *copy_ctx = shm_malloc(sizeof(*copy_ctx) + id->len);
Expand All @@ -781,6 +795,7 @@ rtp_copy_ctx *rtp_copy_ctx_new(struct rtp_relay_ctx *ctx, str *id)
copy_ctx->id.s = (char *)(copy_ctx + 1);
copy_ctx->id.len = id->len;
memcpy(copy_ctx->id.s, id->s, id->len);
copy_ctx->relay = relay;
list_add(&copy_ctx->list, &ctx->copy_contexts);
return copy_ctx;
};
Expand Down Expand Up @@ -942,12 +957,10 @@ static void rtp_relay_loaded_callback(struct dlg_cell *dlg, int type,
RTP_RELAY_BIN_POP(int, &index);
while (index-- > 0) {
RTP_RELAY_BIN_POP(str, &tmp);
copy_ctx = rtp_copy_ctx_new(ctx, &tmp);
copy_ctx = rtp_copy_ctx_new(ctx, relay, &tmp);
if (copy_ctx &&
!relay->funcs.copy_deserialize(&copy_ctx->ctx, &packet)) {
list_del(&copy_ctx->list);
shm_free(copy_ctx);
}
!relay->funcs.copy_deserialize(&copy_ctx->ctx, &packet))
rtp_copy_ctx_free(copy_ctx);
}

RTP_RELAY_BIN_POP(str, &tmp);
Expand Down Expand Up @@ -2714,7 +2727,7 @@ int rtp_relay_copy_offer(rtp_ctx _ctx, str *id, str *flags,
}
rtp_copy = rtp_copy_ctx_get(ctx, id);
if (!rtp_copy) {
rtp_copy = rtp_copy_ctx_new(ctx, id);
rtp_copy = rtp_copy_ctx_new(ctx, sess->relay, id);
if (!rtp_copy) {
LM_ERR("oom for rtp copy context!\n");
return -1;
Expand Down Expand Up @@ -2820,8 +2833,7 @@ int rtp_relay_copy_delete(rtp_ctx _ctx, str *id, str *flags)
ret = sess->relay->funcs.copy_delete(
&info, &sess->server,
copy_ctx->ctx, flags);
list_del(&copy_ctx->list);
shm_free(copy_ctx);
rtp_copy_ctx_free(copy_ctx);
return ret;
}

Expand Down
12 changes: 10 additions & 2 deletions modules/rtpengine/rtpengine.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ static int rtpengine_api_copy_delete(struct rtp_relay_session *sess,
struct rtp_relay_server *server, void *_ctx, str *flags);
static int rtpengine_api_copy_serialize(void *_ctx, bin_packet_t *packet);
static int rtpengine_api_copy_deserialize(void **_ctx, bin_packet_t *packet);
static void rtpengine_api_copy_release(void **_ctx);

static int parse_flags(struct ng_flags_parse *, struct sip_msg *, enum rtpe_operation *, const char *);

Expand Down Expand Up @@ -1487,6 +1488,7 @@ static int mod_preinit(void)
.copy_delete = rtpengine_api_copy_delete,
.copy_serialize = rtpengine_api_copy_serialize,
.copy_deserialize = rtpengine_api_copy_deserialize,
.copy_release = rtpengine_api_copy_release,
};
if (!pv_parse_spec(&rtpengine_relay_pvar_str, &media_pvar))
return -1;
Expand Down Expand Up @@ -4439,8 +4441,6 @@ static int rtpengine_api_copy_delete(struct rtp_relay_session *sess,
bencode_item_t *ret;
ret = rtpengine_api_copy_op(sess, OP_UNSUBSCRIBE,
server, subs, flags, 0, NULL);
if (subs)
shm_free(subs);
if (!ret)
return -1;
bencode_buffer_free(bencode_item_buffer(ret));
Expand Down Expand Up @@ -4469,6 +4469,14 @@ static int rtpengine_api_copy_deserialize(void **_ctx, bin_packet_t *packet)
return 1;
}

static void rtpengine_api_copy_release(void **ctx)
{
if (*ctx) {
shm_free(*ctx);
*ctx = NULL;
}
}

static inline void raise_rtpengine_status_event(struct rtpe_node *node)
{
static str status_connected = str_init("active");
Expand Down
9 changes: 8 additions & 1 deletion modules/rtpproxy/rtpproxy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ static int rtpproxy_api_copy_delete(struct rtp_relay_session *sess,
struct rtp_relay_server *server, void *_ctx, str *flags);
static int rtpproxy_api_copy_serialize(void *_ctx, bin_packet_t *packet);
static int rtpproxy_api_copy_deserialize(void **_ctx, bin_packet_t *packet);
static void rtpproxy_api_copy_release(void **_ctx);

int connect_rtpproxies(struct rtpp_set *filter);
int update_rtpp_proxies(struct rtpp_set *filter);
Expand Down Expand Up @@ -1106,6 +1107,7 @@ static int mod_preinit(void)
.copy_delete = rtpproxy_api_copy_delete,
.copy_serialize = rtpproxy_api_copy_serialize,
.copy_deserialize = rtpproxy_api_copy_deserialize,
.copy_release = rtpproxy_api_copy_release,
};
if (!pv_parse_spec(&rtpproxy_relay_pvar_str, &media_pvar))
return -1;
Expand Down Expand Up @@ -5902,7 +5904,6 @@ static int rtpproxy_api_copy_delete(struct rtp_relay_session *sess,
if (nh_lock)
lock_stop_read(nh_lock);
rtpproxy_free_call_args(&args);
rtpproxy_copy_ctx_free(_ctx);
return ret <= 0?-1:1;
}

Expand Down Expand Up @@ -5997,3 +5998,9 @@ static int rtpproxy_api_copy_deserialize(void **_ctx, bin_packet_t *packet)
*_ctx = ctx;
return -1;
}

static void rtpproxy_api_copy_release(void **_ctx)
{
rtpproxy_copy_ctx_free(*_ctx);
*_ctx = NULL;
}

0 comments on commit 4b21232

Please sign in to comment.