pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/41398?usp=email )
Change subject: sccplite: Avoid msgb_free() during ss7 rx_unknown_cb() ......................................................................
sccplite: Avoid msgb_free() during ss7 rx_unknown_cb()
Since its original addition, API osmo_ss7_register_rx_unknown_cb() states that the msgb is owned by the caller. Hence, osmo-bsc shall not free the msgb. Older versions of libosmo-sigtran (before f16fdf58059b2dea8e940c45c11779c7d4634c3f) had a buggy implementation and actually leaked the msgb.
Related: SYS#6876 Change-Id: I0ce97e3e5a8399d401750dd67494cfd5cf2108e5 --- M TODO-RELEASE M src/osmo-bsc/bsc_ctrl.c M src/osmo-bsc/osmo_bsc_mgcp.c M src/osmo-bsc/osmo_bsc_sigtran.c 4 files changed, 12 insertions(+), 19 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/98/41398/1
diff --git a/TODO-RELEASE b/TODO-RELEASE index 0eae0e8..400e0e0 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -9,3 +9,4 @@ #library what description / commit summary line libosmo-mgcp-client bump_dep Depend on I0d58e6d84418f50670c8ab7cf8490af3bc2f5c26 libosmo-sigtran >2.1.0 osmo_sccp_addr_{create,update}() +libosmo-sigtran minor_release>2.1.0 Avoid leaking msgb during asp_rx_unknown() diff --git a/src/osmo-bsc/bsc_ctrl.c b/src/osmo-bsc/bsc_ctrl.c index 931a8ae..2ba7af8 100644 --- a/src/osmo-bsc/bsc_ctrl.c +++ b/src/osmo-bsc/bsc_ctrl.c @@ -712,7 +712,7 @@ }
/* receive + process a CTRL command from the piggy-back on the IPA/SCCPlite link. - * Transfers msg ownership. */ + * msg is owned by the caller. */ int bsc_sccplite_rx_ctrl(struct osmo_ss7_asp *asp, struct msgb *msg) { struct ctrl_cmd *cmd; @@ -725,7 +725,6 @@ /* prase raw (ASCII) CTRL command into ctrl_cmd */ cmd = ctrl_cmd_parse3(asp, msg, &parse_failed); OSMO_ASSERT(cmd); - msgb_free(msg); if (cmd->type == CTRL_TYPE_ERROR && parse_failed) goto send_reply;
diff --git a/src/osmo-bsc/osmo_bsc_mgcp.c b/src/osmo-bsc/osmo_bsc_mgcp.c index d7de4d2..4114cc3 100644 --- a/src/osmo-bsc/osmo_bsc_mgcp.c +++ b/src/osmo-bsc/osmo_bsc_mgcp.c @@ -91,7 +91,7 @@ return 0; }
-/* We received an IPA-encapsulated MGCP message from a MSC. Transfers msg ownership. */ +/* We received an IPA-encapsulated MGCP message from a MSC. msg owned by caller. */ int bsc_sccplite_rx_mgcp(struct osmo_ss7_asp *asp, struct msgb *msg) { struct bsc_msc_data *msc; @@ -107,16 +107,14 @@ osmo_ss7_asp_get_name(asp), msg->l2h);
msc = msc_from_asp(asp); - if (!msc) { - rc = 0; - goto free_msg_ret; - } + if (!msc) + return 0;
rc = parse_local_endpoint_name(rcv_ep_local_name, sizeof(rcv_ep_local_name), (const char *)msg->l2h); if (rc < 0) { LOGP(DMSC, LOGL_ERROR, "(%s:) Received IPA-encapsulated MGCP: Failed to parse CIC\n", osmo_ss7_asp_get_name(asp)); - goto free_msg_ret; + return rc; }
/* Lookup which conn attached to the MSC holds an MGW endpoint with the @@ -145,8 +143,7 @@ if (!mgcp_cli) { LOGP(DMSC, LOGL_ERROR, "(%s:) Received IPA-encapsulated MGCP: Failed to find associated MGW\n", osmo_ss7_asp_get_name(asp)); - rc = 0; - goto free_msg_ret; + return 0; }
rc = osmo_sockaddr_str_from_str(&osa_str, mgcp_client_remote_addr_str(mgcp_cli), @@ -154,7 +151,7 @@ if (rc < 0) { LOGP(DMSC, LOGL_ERROR, "(%s:) Received IPA-encapsulated MGCP: Failed to parse MGCP address %s:%u\n", osmo_ss7_asp_get_name(asp), mgcp_client_remote_addr_str(mgcp_cli), mgcp_client_remote_port(mgcp_cli)); - goto free_msg_ret; + return rc; }
LOGP(DMSC, LOGL_NOTICE, "%s: Forwarding IPA-encapsulated MGCP to MGW at " OSMO_SOCKADDR_STR_FMT "\n", @@ -164,7 +161,7 @@ if (rc < 0) { LOGP(DMSC, LOGL_ERROR, "(%s:) Received IPA-encapsulated MGCP: Failed to parse MGCP address " OSMO_SOCKADDR_STR_FMT "\n", osmo_ss7_asp_get_name(asp), OSMO_SOCKADDR_STR_FMT_ARGS_NOT_NULL(&osa_str)); - goto free_msg_ret; + return rc; } dest_len = osmo_sockaddr_size(&osa);
@@ -172,8 +169,6 @@ * to be large enough to deal with whatever small/infrequent MGCP messages */ rc = sendto(msc->mgcp_ipa.ofd.fd, msgb_l2(msg), msgb_l2len(msg), 0, &osa.u.sa, dest_len);
-free_msg_ret: - msgb_free(msg); return rc; }
diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c index 88174cc..cc6ac5e 100644 --- a/src/osmo-bsc/osmo_bsc_sigtran.c +++ b/src/osmo-bsc/osmo_bsc_sigtran.c @@ -792,13 +792,12 @@ }
/* this function receives all messages received on an ASP for a PPID / StreamID that - * libosmo-sigtran doesn't know about, such as piggy-backed CTRL and/or MGCP */ + * libosmo-sigtran doesn't know about, such as piggy-backed CTRL and/or MGCP. + * msg is owned by the caller, ie. ownership is not transferred to this callback. */ static int asp_rx_unknown(struct osmo_ss7_asp *asp, int ppid_mux, struct msgb *msg) { - if (osmo_ss7_asp_get_proto(asp) != OSMO_SS7_ASP_PROT_IPA) { - msgb_free(msg); + if (osmo_ss7_asp_get_proto(asp) != OSMO_SS7_ASP_PROT_IPA) return 0; - }
switch (ppid_mux) { case IPAC_PROTO_OSMO: @@ -814,6 +813,5 @@ default: break; } - msgb_free(msg); return 0; /* OSMO_SS7_UNKNOWN? */ }
-
pespin