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? */
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/41398?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I0ce97e3e5a8399d401750dd67494cfd5cf2108e5
Gerrit-Change-Number: 41398
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
