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/.
daniel gerrit-no-reply at lists.osmocom.org
Review at https://gerrit.osmocom.org/1212
gbproxy: Check whether gbproxy_update_link_state_after() deletes the link_info
In case the link_info is deleted we have to stop handling the stored messages
inside link_info. Not doing so can lead to invalid memory being accessed.
Change-Id: Ieb8503e9e94e7a5ac450ad8aa1713ec4f21cdea5
Ticket: OW#3049
Sponsored-by: On-Waves ehf
---
M openbsc/include/openbsc/gb_proxy.h
M openbsc/src/gprs/gb_proxy.c
M openbsc/src/gprs/gb_proxy_tlli.c
3 files changed, 24 insertions(+), 12 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/12/1212/1
diff --git a/openbsc/include/openbsc/gb_proxy.h b/openbsc/include/openbsc/gb_proxy.h
index c396d2b..e10894f 100644
--- a/openbsc/include/openbsc/gb_proxy.h
+++ b/openbsc/include/openbsc/gb_proxy.h
@@ -208,7 +208,7 @@
struct gbproxy_link_info *gbproxy_update_link_state_dl(
struct gbproxy_peer *peer, time_t now,
struct gprs_gb_parse_context *parse_ctx);
-void gbproxy_update_link_state_after(
+int gbproxy_update_link_state_after(
struct gbproxy_peer *peer, struct gbproxy_link_info *link_info,
time_t now, struct gprs_gb_parse_context *parse_ctx);
int gbproxy_remove_stale_link_infos(struct gbproxy_peer *peer, time_t now);
diff --git a/openbsc/src/gprs/gb_proxy.c b/openbsc/src/gprs/gb_proxy.c
index 111f052..1f21208 100644
--- a/openbsc/src/gprs/gb_proxy.c
+++ b/openbsc/src/gprs/gb_proxy.c
@@ -318,7 +318,7 @@
link_info->vu_gen_tx_bss = GBPROXY_INIT_VU_GEN_TX;
}
-static void gbproxy_flush_stored_messages(struct gbproxy_peer *peer,
+static int gbproxy_flush_stored_messages(struct gbproxy_peer *peer,
struct msgb *msg,
time_t now,
struct gbproxy_link_info* link_info,
@@ -349,8 +349,13 @@
peer, link_info, &len_change,
&tmp_parse_ctx);
- gbproxy_update_link_state_after(peer, link_info, now,
- &tmp_parse_ctx);
+ rc = gbproxy_update_link_state_after(peer, link_info, now,
+ &tmp_parse_ctx);
+ if (rc == 1) {
+ LOGP(DLLC, LOGL_NOTICE, "link_info deleted while flushing stored messages\n");
+ msgb_free(stored_msg);
+ return -1;
+ }
rc = gbprox_relay2sgsn(peer->cfg, stored_msg,
msgb_bvci(msg), link_info->sgsn_nsei);
@@ -364,6 +369,8 @@
parse_ctx->llc_msg_name : "BSSGP");
msgb_free(stored_msg);
}
+
+ return 0;
}
static int gbproxy_gsm48_to_peer(struct gbproxy_peer *peer,
@@ -466,8 +473,9 @@
gsm48_hdr_msg_type(parse_ctx->g48_hdr) == GSM48_MT_GMM_ID_RESP;
/* The IMSI is now available */
- gbproxy_flush_stored_messages(peer, msg, now, link_info,
- parse_ctx);
+ if (gbproxy_flush_stored_messages(peer, msg, now, link_info,
+ parse_ctx) < 0)
+ return !is_ident_resp;
gbproxy_reset_imsi_acquisition(link_info);
diff --git a/openbsc/src/gprs/gb_proxy_tlli.c b/openbsc/src/gprs/gb_proxy_tlli.c
index 0aa0632..3b3b976 100644
--- a/openbsc/src/gprs/gb_proxy_tlli.c
+++ b/openbsc/src/gprs/gb_proxy_tlli.c
@@ -348,18 +348,18 @@
gbproxy_attach_link_info(peer, now, link_info);
}
-static void gbproxy_unregister_link_info(struct gbproxy_peer *peer,
+static int gbproxy_unregister_link_info(struct gbproxy_peer *peer,
struct gbproxy_link_info *link_info)
{
if (!link_info)
- return;
+ return 1;
if (link_info->tlli.ptmsi == GSM_RESERVED_TMSI && !link_info->imsi_len) {
LOGP(DGPRS, LOGL_INFO,
"Removing TLLI %08x from list (P-TMSI or IMSI are not set)\n",
link_info->tlli.current);
gbproxy_delete_link_info(peer, link_info);
- return;
+ return 1;
}
link_info->tlli.current = 0;
@@ -371,7 +371,7 @@
gbproxy_reset_link(link_info);
- return;
+ return 0;
}
int gbproxy_imsi_matches(struct gbproxy_config *cfg,
@@ -668,12 +668,13 @@
return link_info;
}
-void gbproxy_update_link_state_after(
+int gbproxy_update_link_state_after(
struct gbproxy_peer *peer,
struct gbproxy_link_info *link_info,
time_t now,
struct gprs_gb_parse_context *parse_ctx)
{
+ int rc = 0;
if (parse_ctx->invalidate_tlli && link_info) {
int keep_info =
peer->cfg->keep_link_infos == GBPROX_KEEP_ALWAYS ||
@@ -684,11 +685,12 @@
if (keep_info) {
LOGP(DGPRS, LOGL_INFO, "Unregistering TLLI %08x\n",
link_info->tlli.current);
- gbproxy_unregister_link_info(peer, link_info);
+ rc = gbproxy_unregister_link_info(peer, link_info);
} else {
LOGP(DGPRS, LOGL_INFO, "Removing TLLI %08x from list\n",
link_info->tlli.current);
gbproxy_delete_link_info(peer, link_info);
+ rc = 1;
}
} else if (parse_ctx->to_bss && parse_ctx->tlli_enc &&
parse_ctx->new_ptmsi_enc && link_info) {
@@ -714,6 +716,8 @@
}
gbproxy_remove_stale_link_infos(peer, now);
+
+ return rc;
}
--
To view, visit https://gerrit.osmocom.org/1212
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb8503e9e94e7a5ac450ad8aa1713ec4f21cdea5
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann at sysmocom.de>