diff options
author | Neels Hofmeyr <neels@hofmeyr.de> | 2019-03-10 04:41:27 +0100 |
---|---|---|
committer | Neels Hofmeyr <neels@hofmeyr.de> | 2019-04-12 06:27:10 +0200 |
commit | ac729eb5a8295d0c1fc2dea3fb0f56413654c13e (patch) | |
tree | bb8968730c096a2889fefb536842f731ad5bb4e9 /include | |
parent | 49eea32ab4158b2b80a3862513363f237fb49939 (diff) |
add caller-owns-msgb variant osmo_sccp_user_sap_down_nofree()
Add osmo_sccp_user_sap_down_nofree(), which is identical to
osmo_sccp_user_sap_down(), but doesn't imply a msgb_free().
To implement that, sccp_sclc_user_sap_down_nofree() with the same msgb
semantics is required.
Rationale:
Avoiding msgb leaks is easiest if the caller retains ownership of the msgb.
Take this hypothetical chain where leaks are obviously avoided:
void send()
{
msg = msgb_alloc();
dispatch(msg);
msgb_free(msg);
}
void dispatch(msg)
{
osmo_fsm_inst_dispatch(fi, msg);
}
void fi_on_event(fi, data)
{
if (socket_is_ok)
socket_write((struct msgb*)data);
}
void socket_write(msgb)
{
if (!ok1)
return;
if (ok2) {
if (!ok3)
return;
write(sock, msg->data);
}
}
However, if the caller passes ownership down to the msgb consumer, things
become nightmarishly complex:
void send()
{
msg = msgb_alloc();
rc = dispatch(msg);
/* dispatching event failed? */
if (rc)
msgb_free(msg);
}
int dispatch(msg)
{
if (osmo_fsm_inst_dispatch(fi, msg))
return -1;
if (something_else())
return -1; // <-- double free!
}
void fi_on_event(fi, data)
{
if (socket_is_ok) {
socket_write((struct msgb*)data);
else
/* socket didn't consume? */
msgb_free(data);
}
int socket_write(msgb)
{
if (!ok1)
return -1; // <-- leak!
if (ok2) {
if (!ok3)
goto out;
write(sock, msg->data);
}
out:
msgb_free(msg);
return -2;
}
If any link in this call chain fails to be aware of the importance to return a
failed RC or to free a msgb if the chain is broken, or to not return a failed
RC if the msgb is consumed, we have a hidden msgb leak or double free.
This is the case with osmo_sccp_user_sap_down(). In new osmo-msc, passing data
through various FSM instances, there is high potential for leak/double-free
bugs. A very large brain is required to track down every msgb path.
osmo_sccp_user_sap_down_nofree() makes this problem trivial to solve even for
humans.
Change-Id: Ic818efa78b90f727e1a94c18b60d9a306644f340
Diffstat (limited to 'include')
-rw-r--r-- | include/osmocom/sigtran/sccp_sap.h | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/include/osmocom/sigtran/sccp_sap.h b/include/osmocom/sigtran/sccp_sap.h index 84d762c..f64b7aa 100644 --- a/include/osmocom/sigtran/sccp_sap.h +++ b/include/osmocom/sigtran/sccp_sap.h @@ -263,6 +263,7 @@ osmo_sccp_user_bind(struct osmo_sccp_instance *inst, const char *name, osmo_prim_cb prim_cb, uint16_t ssn); int osmo_sccp_user_sap_down(struct osmo_sccp_user *scu, struct osmo_prim_hdr *oph); +int osmo_sccp_user_sap_down_nofree(struct osmo_sccp_user *scu, struct osmo_prim_hdr *oph); struct osmo_ss7_instance * osmo_sccp_addr_by_name(struct osmo_sccp_addr *dest_addr, |