[sr-dev] git:master: rr: Fix memory leak when using outbound
Alekzander Spiridonov
alekz at li.ru
Thu Oct 2 19:54:12 CEST 2014
Hi,
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.:
+ if (user.s != NULL)
+ pkg_free(user.s);
2014-10-02 21:39 GMT+04:00 Hugh Waite <hugh.waite at acision.com>:
> Module: sip-router
> Branch: master
> Commit: 44e29820a759405adb7657334e86ea474196e6fd
> URL:
> http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=44e29820a759405adb7657334e86ea474196e6fd
>
> Author: Hugh Waite <hugh.waite at acision.com>
> Committer: Hugh Waite <hugh.waite at acision.com>
> Date: Thu Oct 2 18:37:00 2014 +0100
>
> rr: Fix memory leak when using outbound
>
> - Flow token memory is freed after building the rr header
>
> ---
>
> modules/rr/record.c | 82
> ++++++++++++++++++++++++++++++++++++---------------
> 1 files changed, 58 insertions(+), 24 deletions(-)
>
> diff --git a/modules/rr/record.c b/modules/rr/record.c
> index 4fda746..a521092 100644
> --- a/modules/rr/record.c
> +++ b/modules/rr/record.c
> @@ -375,11 +375,12 @@ static int copy_flow_token(str *token, struct
> sip_msg *_m)
> int record_route(struct sip_msg* _m, str *params)
> {
> struct lump* l, *l2;
> - str user;
> + str user = {NULL, 0};
> struct to_body* from = NULL;
> str* tag;
> int use_ob = rr_obb.use_outbound ? rr_obb.use_outbound(_m) : 0;
> int sips;
> + int ret = 0;
>
> user.len = 0;
>
> @@ -406,7 +407,8 @@ int record_route(struct sip_msg* _m, str *params)
> if (append_fromtag) {
> if (parse_from_header(_m) < 0) {
> LM_ERR("From parsing failed\n");
> - return -2;
> + ret = -2;
> + goto error;
> }
> from = (struct to_body*)_m->from->parsed;
> tag = &from->tag_value;
> @@ -426,17 +428,20 @@ int record_route(struct sip_msg* _m, str *params)
> l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);
> if (!l || !l2) {
> LM_ERR("failed to create an anchor\n");
> - return -5;
> + ret = -5;
> + goto error;
> }
> l = insert_cond_lump_after(l, COND_IF_DIFF_REALMS, 0);
> l2 = insert_cond_lump_before(l2, COND_IF_DIFF_REALMS, 0);
> if (!l || !l2) {
> LM_ERR("failed to insert conditional lump\n");
> - return -6;
> + ret = -6;
> + goto error;
> }
> if (build_rr(l, l2, &user, tag, params, OUTBOUND, sips) <
> 0) {
> LM_ERR("failed to insert outbound Record-Route\n");
> - return -7;
> + ret = -7;
> + goto error;
> }
> }
>
> @@ -444,17 +449,24 @@ int record_route(struct sip_msg* _m, str *params)
> l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);
> if (!l || !l2) {
> LM_ERR("failed to create an anchor\n");
> - return -3;
> + ret = -3;
> + goto error;
> }
>
> if (build_rr(l, l2, &user, tag, params, INBOUND, sips) < 0) {
> LM_ERR("failed to insert inbound Record-Route\n");
> - return -4;
> + ret = -4;
> + goto error;
> }
>
> /* reset the rr_param buffer */
> rr_param_buf.len = 0;
> - return 0;
> + ret = 0;
> +error:
> + if ((use_ob == 1) || (use_ob == 2))
> + if (user.s != NULL)
> + pkg_free(user.s);
> + return ret;
> }
>
>
> @@ -470,7 +482,7 @@ int record_route(struct sip_msg* _m, str *params)
> */
> int record_route_preset(struct sip_msg* _m, str* _data)
> {
> - str user;
> + str user = {NULL, 0};
> struct to_body* from;
> struct lump* l;
> char* hdr, *p;
> @@ -479,6 +491,7 @@ int record_route_preset(struct sip_msg* _m, str* _data)
> char *rr_prefix;
> int rr_prefix_len;
> int sips;
> + int ret = 0;
>
> sips = rr_is_sips(_m);
> if(sips==0) {
> @@ -513,7 +526,8 @@ int record_route_preset(struct sip_msg* _m, str* _data)
> if (append_fromtag) {
> if (parse_from_header(_m) < 0) {
> LM_ERR("From parsing failed\n");
> - return -2;
> + ret = -2;
> + goto error;
> }
> from = (struct to_body*)_m->from->parsed;
> }
> @@ -521,7 +535,8 @@ int record_route_preset(struct sip_msg* _m, str* _data)
> l = anchor_lump(_m, _m->headers->name.s - _m->buf, 0,
> HDR_RECORDROUTE_T);
> if (!l) {
> LM_ERR("failed to create lump anchor\n");
> - return -3;
> + ret = -3;
> + goto error;
> }
>
> hdr_len = rr_prefix_len;
> @@ -544,7 +559,8 @@ int record_route_preset(struct sip_msg* _m, str* _data)
> hdr = pkg_malloc(hdr_len);
> if (!hdr) {
> LM_ERR("no pkg memory left\n");
> - return -4;
> + ret = -4;
> + goto error;
> }
>
> p = hdr;
> @@ -581,9 +597,15 @@ int record_route_preset(struct sip_msg* _m, str*
> _data)
> if (!insert_new_lump_after(l, hdr, hdr_len, 0)) {
> LM_ERR("failed to insert new lump\n");
> pkg_free(hdr);
> - return -5;
> + ret = -5;
> + goto error;
> }
> - return 1;
> + ret = 1;
> +error:
> + if ((use_ob == 1) || (use_ob == 2))
> + if (user.s != NULL)
> + pkg_free(user.s);
> + return ret;
> }
>
> /*!
> @@ -723,12 +745,13 @@ lump_err:
>
> int record_route_advertised_address(struct sip_msg* _m, str* _data)
> {
> - str user;
> + str user = {NULL, 0};
> str *tag = NULL;
> struct lump* l;
> struct lump* l2;
> int use_ob = rr_obb.use_outbound ? rr_obb.use_outbound(_m) : 0;
> int sips;
> + int ret = 0;
>
> user.len = 0;
> user.s = 0;
> @@ -753,7 +776,8 @@ int record_route_advertised_address(struct sip_msg*
> _m, str* _data)
> if (append_fromtag) {
> if (parse_from_header(_m) < 0) {
> LM_ERR("From parsing failed\n");
> - return -2;
> + ret = -2;
> + goto error;
> }
> tag = &((struct to_body*)_m->from->parsed)->tag_value;
> }
> @@ -765,18 +789,21 @@ int record_route_advertised_address(struct sip_msg*
> _m, str* _data)
> l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);
> if (!l || !l2) {
> LM_ERR("failed to create an anchor\n");
> - return -3;
> + ret = -3;
> + goto error;
> }
> l = insert_cond_lump_after(l, COND_IF_DIFF_PROTO, 0);
> l2 = insert_cond_lump_before(l2, COND_IF_DIFF_PROTO, 0);
> if (!l || !l2) {
> LM_ERR("failed to insert conditional lump\n");
> - return -4;
> + ret = -4;
> + goto error;
> }
> if (build_advertised_rr(l, l2, _data, &user, tag, OUTBOUND,
> sips) < 0) {
> LM_ERR("failed to insert outbound Record-Route\n");
> - return -5;
> + ret = -5;
> + goto error;
> }
> }
>
> @@ -784,14 +811,21 @@ int record_route_advertised_address(struct sip_msg*
> _m, str* _data)
> l2 = anchor_lump(_m, _m->headers->name.s - _m->buf, 0, 0);
> if (!l || !l2) {
> LM_ERR("failed to create an anchor\n");
> - return -6;
> + ret = -6;
> + goto error;
> }
>
> if (build_advertised_rr(l, l2, _data, &user, tag, INBOUND, sips) <
> 0) {
> LM_ERR("failed to insert outbound Record-Route\n");
> - return -7;
> + ret = -7;
> + goto error;
> }
> - return 1;
> + ret = 1;
> +error:
> + if ((use_ob == 1) || (use_ob == 2))
> + if (user.s != NULL)
> + pkg_free(user.s);
> + return ret;
> }
>
>
>
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
--
Best regards,
Alekzander Spiridonov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20141002/e31e403f/attachment.html>
More information about the sr-dev
mailing list