Skip to content

Commit 7ad3c85

Browse files
committed
audiobridge: fix double free of buffered packet in participant thread
jitter_buffer_get() only assigns jbp.data when it actually returns a packet, so a failed read leaves there the pointer to the buffered packet that was already freed at the end of a previous iteration. The PLC branch then passed that dangling bpkt to janus_audiobridge_buffer_packet_destroy() whenever it saw participant->decoder == NULL, i.e. exactly when the participant was being cleaned up concurrently: the packet got freed a second time and the process crashed inside the allocator. There is nothing to free in that branch, so drop the call; clear bpkt after every destroy and reset jbp.data before each read so that a stale pointer cannot be used again.
1 parent 4602fcc commit 7ad3c85

1 file changed

Lines changed: 11 additions & 1 deletion

File tree

src/plugins/janus_audiobridge.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9213,6 +9213,9 @@ static void *janus_audiobridge_participant_thread(void *data) {
92139213
before += 20000;
92149214
if(participant->jitter) {
92159215
janus_mutex_lock(&participant->qmutex);
9216+
/* jitter_buffer_get() only sets jbp.data when it actually returns a packet,
9217+
* so clear it first to make sure a failed read leaves no stale pointer */
9218+
jbp.data = NULL;
92169219
ret = jitter_buffer_get(participant->jitter, &jbp, participant->codec == JANUS_AUDIOCODEC_OPUS ? 960 : 160, NULL);
92179220
jitter_ticks++;
92189221
/* Adjust the buffer size every 50 ticks (~1 second) */
@@ -9230,7 +9233,10 @@ static void *janus_audiobridge_participant_thread(void *data) {
92309233
if(participant->decoder == NULL) {
92319234
/* This means we're cleaning up, so don't try to decode */
92329235
janus_mutex_unlock(&participant->decoding_mutex);
9233-
janus_audiobridge_buffer_packet_destroy(bpkt);
9236+
/* Notice that we have no buffered packet to get rid of here: we
9237+
* only got to this branch because the jitter buffer had nothing
9238+
* for us, so bpkt is either NULL or a dangling pointer to the
9239+
* packet we already freed in a previous iteration */
92349240
break;
92359241
}
92369242
int32_t output_samples = 0;
@@ -9286,6 +9292,7 @@ static void *janus_audiobridge_participant_thread(void *data) {
92869292
JANUS_LOG(LOG_ERR, "[%s] Ops! got an error accessing the RTP payload\n",
92879293
participant->codec == JANUS_AUDIOCODEC_OPUS ? "Opus" : "G.711");
92889294
janus_audiobridge_buffer_packet_destroy(bpkt);
9295+
bpkt = NULL;
92899296
continue;
92909297
}
92919298
rtp = (janus_rtp_header *)buffer;
@@ -9308,6 +9315,7 @@ static void *janus_audiobridge_participant_thread(void *data) {
93089315
/* This means we're cleaning up, so don't try to decode */
93099316
janus_mutex_unlock(&participant->decoding_mutex);
93109317
janus_audiobridge_buffer_packet_destroy(bpkt);
9318+
bpkt = NULL;
93119319
break;
93129320
}
93139321
pkt->length = opus_decode(participant->decoder, payload, plen, (opus_int16 *)pkt->data, BUFFER_SAMPLES, 0);
@@ -9317,6 +9325,7 @@ static void *janus_audiobridge_participant_thread(void *data) {
93179325
if(plen != 160) {
93189326
JANUS_LOG(LOG_WARN, "[G.711] Wrong packet size (expected 160, got %d), skipping audio packet\n", plen);
93199327
janus_audiobridge_buffer_packet_destroy(bpkt);
9328+
bpkt = NULL;
93209329
g_free(pkt->data);
93219330
g_free(pkt);
93229331
continue;
@@ -9341,6 +9350,7 @@ static void *janus_audiobridge_participant_thread(void *data) {
93419350
#endif
93429351
/* Get rid of the buffered packet */
93439352
janus_audiobridge_buffer_packet_destroy(bpkt);
9353+
bpkt = NULL;
93449354
/* Update the details */
93459355
participant->last_seq = pkt->seq_number;
93469356
participant->last_timestamp = pkt->timestamp;

0 commit comments

Comments
 (0)