<div dir="ltr"><div>Hi,<br><br></div>I'm pretty sure that it's safe to free NULL pointer even in kamailio memory manager. So checking before pkg_free looks redundant. E.g.:<br>+ if (user.s != NULL)<br>
+ pkg_free(user.s);<br></div><div class="gmail_extra"><br><div class="gmail_quote">2014-10-02 21:39 GMT+04:00 Hugh Waite <span dir="ltr"><<a href="mailto:hugh.waite@acision.com" target="_blank">hugh.waite@acision.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Module: sip-router<br>
Branch: master<br>
Commit: 44e29820a759405adb7657334e86ea474196e6fd<br>
URL: <a href="http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=44e29820a759405adb7657334e86ea474196e6fd" target="_blank">http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=44e29820a759405adb7657334e86ea474196e6fd</a><br>
<br>
Author: Hugh Waite <<a href="mailto:hugh.waite@acision.com">hugh.waite@acision.com</a>><br>
Committer: Hugh Waite <<a href="mailto:hugh.waite@acision.com">hugh.waite@acision.com</a>><br>
Date: Thu Oct 2 18:37:00 2014 +0100<br>
<br>
rr: Fix memory leak when using outbound<br>
<br>
- Flow token memory is freed after building the rr header<br>
<br>
---<br>
<br>
modules/rr/record.c | 82 ++++++++++++++++++++++++++++++++++++---------------<br>
1 files changed, 58 insertions(+), 24 deletions(-)<br>
<br>
diff --git a/modules/rr/record.c b/modules/rr/record.c<br>
index 4fda746..a521092 100644<br>
--- a/modules/rr/record.c<br>
+++ b/modules/rr/record.c<br>
@@ -375,11 +375,12 @@ static int copy_flow_token(str *token, struct sip_msg *_m)<br>
int record_route(struct sip_msg* _m, str *params)<br>
{<br>
struct lump* l, *l2;<br>
- str user;<br>
+ str user = {NULL, 0};<br>
struct to_body* from = NULL;<br>
str* tag;<br>
int use_ob = rr_obb.use_outbound ? rr_obb.use_outbound(_m) : 0;<br>
int sips;<br>
+ int ret = 0;<br>
<br>
user.len = 0;<br>
<br>
@@ -406,7 +407,8 @@ int record_route(struct sip_msg* _m, str *params)<br>
if (append_fromtag) {<br>
if (parse_from_header(_m) < 0) {<br>
LM_ERR("From parsing failed\n");<br>
- return -2;<br>
+ ret = -2;<br>
+ goto error;<br>
}<br>
from = (struct to_body*)_m->from->parsed;<br>
tag = &from->tag_value;<br>
@@ -426,17 +428,20 @@ int record_route(struct sip_msg* _m, str *params)<br>
l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);<br>
if (!l || !l2) {<br>
LM_ERR("failed to create an anchor\n");<br>
- return -5;<br>
+ ret = -5;<br>
+ goto error;<br>
}<br>
l = insert_cond_lump_after(l, COND_IF_DIFF_REALMS, 0);<br>
l2 = insert_cond_lump_before(l2, COND_IF_DIFF_REALMS, 0);<br>
if (!l || !l2) {<br>
LM_ERR("failed to insert conditional lump\n");<br>
- return -6;<br>
+ ret = -6;<br>
+ goto error;<br>
}<br>
if (build_rr(l, l2, &user, tag, params, OUTBOUND, sips) < 0) {<br>
LM_ERR("failed to insert outbound Record-Route\n");<br>
- return -7;<br>
+ ret = -7;<br>
+ goto error;<br>
}<br>
}<br>
<br>
@@ -444,17 +449,24 @@ int record_route(struct sip_msg* _m, str *params)<br>
l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);<br>
if (!l || !l2) {<br>
LM_ERR("failed to create an anchor\n");<br>
- return -3;<br>
+ ret = -3;<br>
+ goto error;<br>
}<br>
<br>
if (build_rr(l, l2, &user, tag, params, INBOUND, sips) < 0) {<br>
LM_ERR("failed to insert inbound Record-Route\n");<br>
- return -4;<br>
+ ret = -4;<br>
+ goto error;<br>
}<br>
<br>
/* reset the rr_param buffer */<br>
rr_param_buf.len = 0;<br>
- return 0;<br>
+ ret = 0;<br>
+error:<br>
+ if ((use_ob == 1) || (use_ob == 2))<br>
+ if (user.s != NULL)<br>
+ pkg_free(user.s);<br>
+ return ret;<br>
}<br>
<br>
<br>
@@ -470,7 +482,7 @@ int record_route(struct sip_msg* _m, str *params)<br>
*/<br>
int record_route_preset(struct sip_msg* _m, str* _data)<br>
{<br>
- str user;<br>
+ str user = {NULL, 0};<br>
struct to_body* from;<br>
struct lump* l;<br>
char* hdr, *p;<br>
@@ -479,6 +491,7 @@ int record_route_preset(struct sip_msg* _m, str* _data)<br>
char *rr_prefix;<br>
int rr_prefix_len;<br>
int sips;<br>
+ int ret = 0;<br>
<br>
sips = rr_is_sips(_m);<br>
if(sips==0) {<br>
@@ -513,7 +526,8 @@ int record_route_preset(struct sip_msg* _m, str* _data)<br>
if (append_fromtag) {<br>
if (parse_from_header(_m) < 0) {<br>
LM_ERR("From parsing failed\n");<br>
- return -2;<br>
+ ret = -2;<br>
+ goto error;<br>
}<br>
from = (struct to_body*)_m->from->parsed;<br>
}<br>
@@ -521,7 +535,8 @@ int record_route_preset(struct sip_msg* _m, str* _data)<br>
l = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, HDR_RECORDROUTE_T);<br>
if (!l) {<br>
LM_ERR("failed to create lump anchor\n");<br>
- return -3;<br>
+ ret = -3;<br>
+ goto error;<br>
}<br>
<br>
hdr_len = rr_prefix_len;<br>
@@ -544,7 +559,8 @@ int record_route_preset(struct sip_msg* _m, str* _data)<br>
hdr = pkg_malloc(hdr_len);<br>
if (!hdr) {<br>
LM_ERR("no pkg memory left\n");<br>
- return -4;<br>
+ ret = -4;<br>
+ goto error;<br>
}<br>
<br>
p = hdr;<br>
@@ -581,9 +597,15 @@ int record_route_preset(struct sip_msg* _m, str* _data)<br>
if (!insert_new_lump_after(l, hdr, hdr_len, 0)) {<br>
LM_ERR("failed to insert new lump\n");<br>
pkg_free(hdr);<br>
- return -5;<br>
+ ret = -5;<br>
+ goto error;<br>
}<br>
- return 1;<br>
+ ret = 1;<br>
+error:<br>
+ if ((use_ob == 1) || (use_ob == 2))<br>
+ if (user.s != NULL)<br>
+ pkg_free(user.s);<br>
+ return ret;<br>
}<br>
<br>
/*!<br>
@@ -723,12 +745,13 @@ lump_err:<br>
<br>
int record_route_advertised_address(struct sip_msg* _m, str* _data)<br>
{<br>
- str user;<br>
+ str user = {NULL, 0};<br>
str *tag = NULL;<br>
struct lump* l;<br>
struct lump* l2;<br>
int use_ob = rr_obb.use_outbound ? rr_obb.use_outbound(_m) : 0;<br>
int sips;<br>
+ int ret = 0;<br>
<br>
user.len = 0;<br>
user.s = 0;<br>
@@ -753,7 +776,8 @@ int record_route_advertised_address(struct sip_msg* _m, str* _data)<br>
if (append_fromtag) {<br>
if (parse_from_header(_m) < 0) {<br>
LM_ERR("From parsing failed\n");<br>
- return -2;<br>
+ ret = -2;<br>
+ goto error;<br>
}<br>
tag = &((struct to_body*)_m->from->parsed)->tag_value;<br>
}<br>
@@ -765,18 +789,21 @@ int record_route_advertised_address(struct sip_msg* _m, str* _data)<br>
l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);<br>
if (!l || !l2) {<br>
LM_ERR("failed to create an anchor\n");<br>
- return -3;<br>
+ ret = -3;<br>
+ goto error;<br>
}<br>
l = insert_cond_lump_after(l, COND_IF_DIFF_PROTO, 0);<br>
l2 = insert_cond_lump_before(l2, COND_IF_DIFF_PROTO, 0);<br>
if (!l || !l2) {<br>
LM_ERR("failed to insert conditional lump\n");<br>
- return -4;<br>
+ ret = -4;<br>
+ goto error;<br>
}<br>
if (build_advertised_rr(l, l2, _data, &user, tag, OUTBOUND,<br>
sips) < 0) {<br>
LM_ERR("failed to insert outbound Record-Route\n");<br>
- return -5;<br>
+ ret = -5;<br>
+ goto error;<br>
}<br>
}<br>
<br>
@@ -784,14 +811,21 @@ int record_route_advertised_address(struct sip_msg* _m, str* _data)<br>
l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);<br>
if (!l || !l2) {<br>
LM_ERR("failed to create an anchor\n");<br>
- return -6;<br>
+ ret = -6;<br>
+ goto error;<br>
}<br>
<br>
if (build_advertised_rr(l, l2, _data, &user, tag, INBOUND, sips) < 0) {<br>
LM_ERR("failed to insert outbound Record-Route\n");<br>
- return -7;<br>
+ ret = -7;<br>
+ goto error;<br>
}<br>
- return 1;<br>
+ ret = 1;<br>
+error:<br>
+ if ((use_ob == 1) || (use_ob == 2))<br>
+ if (user.s != NULL)<br>
+ pkg_free(user.s);<br>
+ return ret;<br>
}<br>
<br>
<br>
<br>
<br>
_______________________________________________<br>
sr-dev mailing list<br>
<a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
<a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
</blockquote></div><br><br clear="all"><br>-- <br><div><div>Best regards,<br>Alekzander Spiridonov</div><br></div>
</div>