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

3 files changed, 27 insertions, 13 deletions

diff --git a/openbsc/include/openbsc/gb_proxy.h b/openbsc/include/openbsc/gb_proxy.h
index c396d2bd1..e10894fc3 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_ul(
 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 111f05208..d95139f8d 100644
--- a/openbsc/src/gprs/gb_proxy.c
+++ b/openbsc/src/gprs/gb_proxy.c
@@ -318,7 +318,7 @@ static void gbproxy_reset_imsi_acquisition(struct gbproxy_link_info* link_info)
 	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 @@ static void gbproxy_flush_stored_messages(struct gbproxy_peer *peer,
 				    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 @@ static void gbproxy_flush_stored_messages(struct gbproxy_peer *peer,
 			     parse_ctx->llc_msg_name : "BSSGP");
 		msgb_free(stored_msg);
 	}
+
+	return 0;
 }
 
 static int gbproxy_gsm48_to_peer(struct gbproxy_peer *peer,
@@ -465,9 +472,12 @@ static int gbproxy_imsi_acquisition(struct gbproxy_peer *peer,
 			gsm48_hdr_pdisc(parse_ctx->g48_hdr) == GSM48_PDISC_MM_GPRS &&
 			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);
+		/* The IMSI is now available. If flushing the messages fails,
+		 * then link_info has been deleted and we should return
+		 * immediately. */
+		if (gbproxy_flush_stored_messages(peer, msg, now, link_info,
+					      parse_ctx) < 0)
+			return 0;
 
 		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 0aa06327e..3b3b976a5 100644
--- a/openbsc/src/gprs/gb_proxy_tlli.c
+++ b/openbsc/src/gprs/gb_proxy_tlli.c
@@ -348,18 +348,18 @@ static void gbproxy_touch_link_info(struct gbproxy_peer *peer,
 	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 @@ static void gbproxy_unregister_link_info(struct gbproxy_peer *peer,
 
 	gbproxy_reset_link(link_info);
 
-	return;
+	return 0;
 }
 
 int gbproxy_imsi_matches(struct gbproxy_config *cfg,
@@ -668,12 +668,13 @@ struct gbproxy_link_info *gbproxy_update_link_state_dl(
 	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 @@ void gbproxy_update_link_state_after(
 		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 @@ void gbproxy_update_link_state_after(
 	}
 
 	gbproxy_remove_stale_link_infos(peer, now);
+
+	return rc;
 }