[Asterisk-code-review] res_rtp_asterisk: Make sure that payload is freed when not accepted b... (asterisk[16])

[Pirmin Walthert]

Pirmin Walthert has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/14207 )


Change subject: res_rtp_asterisk: Make sure that payload is freed when not accepted by ast_data_buffer_put
......................................................................

res_rtp_asterisk: Make sure that payload is freed when not accepted by
ast_data_buffer_put

When the ast_data_buffer_put rejects to add a packet, for example because
the buffer already contains a packet with the same sequence number, the
payload will never be freed, resulting in a memory leak. Prevent this by
calling ast_free(payload) whenever ast_data_buffer_put retuns -1. For
this to work properly, we also need to change the return value to -1
when the packet has not been inserted.

Change-Id: Ie6c49495d1c921d5f997651c7d0f79646f095cf1
---
M main/data_buffer.c
M res/res_rtp_asterisk.c
2 files changed, 7 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/07/14207/1

diff --git a/main/data_buffer.c b/main/data_buffer.c
index cfc323c..85e7971 100644
--- a/main/data_buffer.c
+++ b/main/data_buffer.c
@@ -254,7 +254,7 @@
 	AST_LIST_TRAVERSE_SAFE_END;
 
 	if (inserted == -1) {
-		return 0;
+		return -1;
 	}
 
 	if (!inserted) {
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 739b17b..2e71c1d 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4991,7 +4991,9 @@
 			if (payload) {
 				payload->size = packet_len;
 				memcpy(payload->buf, rtpheader, packet_len);
-				ast_data_buffer_put(rtp->send_buffer, rtp->seqno, payload);
+				if (ast_data_buffer_put(rtp->send_buffer, rtp->seqno, payload) == -1) {
+					ast_free(payload);
+				}
 			}
 		}
 
@@ -7938,7 +7940,9 @@
 
 		payload->size = res;
 		memcpy(payload->buf, rtpheader, res);
-		ast_data_buffer_put(rtp->recv_buffer, seqno, payload);
+		if (ast_data_buffer_put(rtp->recv_buffer, seqno, payload) == -1) {
+			ast_free(payload);
+		}
 
 		/* If this sequence number is removed that means we had a gap and this packet has filled it in
 		 * some. Since it was part of the gap we will have already added any other missing sequence numbers

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14207
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ie6c49495d1c921d5f997651c7d0f79646f095cf1
Gerrit-Change-Number: 14207
Gerrit-PatchSet: 1
Gerrit-Owner: Pirmin Walthert <infos at nappsoft.ch>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200414/f34cbe4f/attachment.html>