This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
fixeria gerrit-no-reply at lists.osmocom.orgfixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26460 ) Change subject: libmsc: fix memory leak (struct msgb) in msc_i_ran_enc() ...................................................................... libmsc: fix memory leak (struct msgb) in msc_i_ran_enc() Function msc_i_ran_enc() calls msc_role_ran_encode(), but unlike the other callers of this function it does not free() the encoded message. A simple solution would be to call msgb_free(), like it's done in the other places. But a more elegant solution is to modify function msc_role_ran_encode(), so that it attaches the msgb to OTC_SELECT. This way there is no need to call msgb_free() here and there. This change fixes a memleak observed while running ttcn3-msc-test. Change-Id: I741e082badc32ba9a97c1495c894e1d22e122e3a Related: OS#5340 --- M src/libmsc/msc_a.c M src/libmsc/msc_a_remote.c M src/libmsc/msc_t.c M src/libmsc/msub.c 4 files changed, 7 insertions(+), 14 deletions(-) Approvals: Jenkins Builder: Verified neels: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/src/libmsc/msc_a.c b/src/libmsc/msc_a.c index 74721d2..c9b0572 100644 --- a/src/libmsc/msc_a.c +++ b/src/libmsc/msc_a.c @@ -1659,12 +1659,9 @@ .an_proto = msc_a->c.ran->an_proto, .msg = msc_role_ran_encode(msc_a->c.fi, ran_msg), }; - int rc; if (!an_apdu.msg) return -EIO; - rc = _msub_role_dispatch(msc_a->c.msub, to_role, to_role_event, &an_apdu, file, line); - msgb_free(an_apdu.msg); - return rc; + return _msub_role_dispatch(msc_a->c.msub, to_role, to_role_event, &an_apdu, file, line); } int msc_a_tx_dtap_to_i(struct msc_a *msc_a, struct msgb *dtap) diff --git a/src/libmsc/msc_a_remote.c b/src/libmsc/msc_a_remote.c index 84eff07..e4474f4 100644 --- a/src/libmsc/msc_a_remote.c +++ b/src/libmsc/msc_a_remote.c @@ -179,8 +179,6 @@ return; msc_a_remote_msg_up_to_remote_msc(msc_a, MSC_ROLE_T, OSMO_GSUP_MSGT_E_PREPARE_HANDOVER_ERROR, &an_apdu); - msgb_free(an_apdu.msg); - return; } /* [MSC-A---------------------] [MSC-B---------------------] diff --git a/src/libmsc/msc_t.c b/src/libmsc/msc_t.c index af0ddaa..43bc74e 100644 --- a/src/libmsc/msc_t.c +++ b/src/libmsc/msc_t.c @@ -145,7 +145,6 @@ return; msub_role_dispatch(msc_t->c.msub, MSC_ROLE_A, MSC_A_EV_FROM_T_PREPARE_HANDOVER_FAILURE, &an_apdu); - msgb_free(an_apdu.msg); } static int msc_t_ho_request_decode_and_store_cb(struct osmo_fsm_inst *msc_t_fi, void *data, @@ -238,7 +237,6 @@ static int msc_t_send_stored_ho_request__decode_cb(struct osmo_fsm_inst *msc_t_fi, void *data, const struct ran_msg *ran_dec) { - int rc; struct an_apdu an_apdu; struct msc_t *msc_t = msc_t_priv(msc_t_fi); struct osmo_sockaddr_str *rtp_ran_local = data; @@ -263,9 +261,7 @@ }; if (!an_apdu.msg) return -EIO; - rc = msc_t_down_l2_co(msc_t, &an_apdu, true); - msgb_free(an_apdu.msg); - return rc; + return msc_t_down_l2_co(msc_t, &an_apdu, true); } /* The MGW endpoint is created, we know our AoIP Transport Layer Address and can send the Handover Request to the RAN @@ -472,9 +468,7 @@ if (!an_apdu.msg) return -EIO; /* Send to remote MSC via msc_a_remote role */ - rc = msub_role_dispatch(msc_t->c.msub, MSC_ROLE_A, MSC_A_EV_FROM_T_PREPARE_HANDOVER_RESPONSE, &an_apdu); - msgb_free(an_apdu.msg); - return rc; + return msub_role_dispatch(msc_t->c.msub, MSC_ROLE_A, MSC_A_EV_FROM_T_PREPARE_HANDOVER_RESPONSE, &an_apdu); } static int msc_t_wait_ho_request_ack_decode_cb(struct osmo_fsm_inst *msc_t_fi, void *data, diff --git a/src/libmsc/msub.c b/src/libmsc/msub.c index 112703a..e4dd332 100644 --- a/src/libmsc/msub.c +++ b/src/libmsc/msub.c @@ -544,6 +544,8 @@ *conn_p = NULL; } +/* NOTE: the resulting message buffer will be attached to OTC_SELECT, so its lifetime + * is limited by the current select() loop iteration. Use talloc_steal() to avoid this. */ struct msgb *msc_role_ran_encode(struct osmo_fsm_inst *fi, const struct ran_msg *ran_msg) { struct msc_role_common *c = fi->priv; @@ -556,6 +558,8 @@ msg = c->ran->ran_encode(fi, ran_msg); if (!msg) LOGPFSML(fi, LOGL_ERROR, "Failed to encode %s\n", ran_msg_type_name(ran_msg->msg_type)); + else + talloc_steal(OTC_SELECT, msg); return msg; } -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26460 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I741e082badc32ba9a97c1495c894e1d22e122e3a Gerrit-Change-Number: 26460 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de> Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-MessageType: merged -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211207/72044f0a/attachment.htm>