Change in osmo-pcu[master]: pdch: Fix null MS access gprs_rlcmac_pdch::rcv_control_ack

pespin gerrit-no-reply at lists.osmocom.org
Fri Jun 25 11:57:50 UTC 2021


pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/24747 )

Change subject: pdch: Fix null MS access gprs_rlcmac_pdch::rcv_control_ack
......................................................................

pdch: Fix null MS access gprs_rlcmac_pdch::rcv_control_ack

If bts_ms_by_tlli() at the start of the function fails, ms could be
NULL. As a result "ms->nacc" access at the end of the function would
crash.
Solution:
In the function, we get the related expected TBF from pdch_ulc, and we only
continue if a TBF is found. Since tbf objects are always expected to
have a GprsMs, simply gather it from there.

Change-Id: I666ed5d157f42e74956fa49fc9eea85d27e63d44
---
M src/pdch.cpp
1 file changed, 6 insertions(+), 3 deletions(-)

Approvals:
  Jenkins Builder: Verified
  dexter: Looks good to me, but someone else must approve
  fixeria: Looks good to me, approved



diff --git a/src/pdch.cpp b/src/pdch.cpp
index 8e0a24a..9321384 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -304,7 +304,7 @@
 {
 	struct gprs_rlcmac_tbf *tbf, *new_tbf;
 	uint32_t tlli = packet->TLLI;
-	GprsMs *ms = bts_ms_by_tlli(bts(), tlli, GSM_RESERVED_TMSI);
+	GprsMs *ms;
 	gprs_rlcmac_ul_tbf *ul_tbf;
 	enum pdch_ulc_tbf_poll_reason reason;
 	struct pdch_ulc_node *poll;
@@ -314,6 +314,7 @@
 		LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "PACKET CONTROL ACK with "
 			"unknown FN=%u TLLI=0x%08x (TRX %d TS %d)\n",
 			fn, tlli, trx_no(), ts_no);
+		ms = bts_ms_by_tlli(bts(), tlli, GSM_RESERVED_TMSI);
 		if (ms)
 			LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "PACKET CONTROL ACK with "
 				"unknown TBF corresponds to MS with IMSI %s, TA %d, "
@@ -332,6 +333,8 @@
 	tbf->n_reset(N3101);
 
 	tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
+	/* Gather MS from TBF, since it may be NULL or may have been merged during update_ms */
+	ms = tbf->ms();
 
 	LOGPTBF(tbf, LOGL_DEBUG, "FN=%" PRIu32 " Rx Packet Control Ack (reason=%s)\n",
 		fn, get_value_string(pdch_ulc_tbf_poll_reason_names, reason));
@@ -353,7 +356,7 @@
 		tbf->n_reset(N3105);
 		TBF_SET_ASS_STATE_DL(tbf, GPRS_RLCMAC_DL_ASS_NONE);
 
-		new_tbf = tbf->ms() ? ms_dl_tbf(tbf->ms()) : NULL;
+		new_tbf = ms_dl_tbf(ms);
 		if (!new_tbf) {
 			LOGPDCH(this, DRLCMAC, LOGL_ERROR, "Got ACK, but DL "
 				"TBF is gone TLLI=0x%08x\n", tlli);
@@ -385,7 +388,7 @@
 		tbf->n_reset(N3105);
 		TBF_SET_ASS_STATE_UL(tbf, GPRS_RLCMAC_UL_ASS_NONE);
 
-		new_tbf = tbf->ms() ? ms_ul_tbf(tbf->ms()) : NULL;
+		new_tbf = ms_ul_tbf(ms);
 		if (!new_tbf) {
 			LOGPDCH(this, DRLCMAC, LOGL_ERROR, "Got ACK, but UL "
 				"TBF is gone TLLI=0x%08x\n", tlli);

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/24747
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I666ed5d157f42e74956fa49fc9eea85d27e63d44
Gerrit-Change-Number: 24747
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210625/4329ffbc/attachment.htm>


More information about the gerrit-log mailing list