From 8af0dea0c785a40b18559bb4eb1d26aa46faa1a5 Mon Sep 17 00:00:00 2001 From: George Joseph Date: Wed, 4 Dec 2019 14:01:22 -0700 Subject: [PATCH] res_rtp_asterisk: Add frame list cleanups to ast_rtp_read In Asterisk 16+, there are a few places in ast_rtp_read where we've allocated a frame list but return a null frame instead of the list. In these cases, any frames left in the list won't be freed. In the vast majority of the cases, the list is empty when we return so there's nothing to free but there have been leaks reported in the wild that can be traced back to frames left in the list before returning. The escape paths now all have logic to free frames left in the list. ASTERISK-28609 Reported by: Ted G Change-Id: Ia1d7075857ebd26b47183c44b1aebb0d8f985f7a --- res/res_rtp_asterisk.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c index c870fce13c..916a616d34 100644 --- a/res/res_rtp_asterisk.c +++ b/res/res_rtp_asterisk.c @@ -7561,7 +7561,9 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc if (!rtp->recv_buffer) { /* If there is no receive buffer then we can pass back the frame directly */ - return ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno); + frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno); + AST_LIST_INSERT_TAIL(&frames, frame, frame_list); + return AST_LIST_FIRST(&frames); } else if (rtp->expectedrxseqno == -1 || seqno == rtp->expectedrxseqno) { rtp->expectedrxseqno = seqno + 1; @@ -7569,7 +7571,9 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc * return it directly without duplicating it. */ if (!ast_data_buffer_count(rtp->recv_buffer)) { - return ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno); + frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno); + AST_LIST_INSERT_TAIL(&frames, frame, frame_list); + return AST_LIST_FIRST(&frames); } if (!AST_VECTOR_REMOVE_CMP_ORDERED(&rtp->missing_seqno, seqno, find_by_value, @@ -7582,7 +7586,9 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc * chance it will be overwritten. */ if (!ast_data_buffer_get(rtp->recv_buffer, seqno + 1)) { - return ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno); + frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno); + AST_LIST_INSERT_TAIL(&frames, frame, frame_list); + return AST_LIST_FIRST(&frames); } /* Otherwise we need to dupe the frame so that the potential processing of frames placed after @@ -7696,7 +7702,12 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc AST_VECTOR_RESET(&rtp->missing_seqno, AST_VECTOR_ELEM_CLEANUP_NOOP); return AST_LIST_FIRST(&frames); - } else if (seqno < rtp->expectedrxseqno) { + } + + /* We're finished with the frames list */ + ast_frame_free(AST_LIST_FIRST(&frames), 0); + + if (seqno < rtp->expectedrxseqno) { /* If this is a packet from the past then we have received a duplicate packet, so just drop it */ ast_debug(2, "Received an old packet with sequence number '%d' on RTP instance '%p', dropping it\n", seqno, instance); @@ -7807,8 +7818,6 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc ast_rtcp_calculate_sr_rr_statistics(instance, rtcp_report, remote_address, ice, sr); } } - - return &ast_null_frame; } return &ast_null_frame;