[PATCH] osmo-pcu[master]: fix PACCH paging: don't return early in case of NULL TBF

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Apr 27 02:37:34 UTC 2017


Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/2420

to look at the new patch set (#2).

fix PACCH paging: don't return early in case of NULL TBF

Commit b78a4a6dfef217c538d45949a6ae725e22a36b05 tried to fix a NULL dereference
error, but apparently was overly eager to return, because it looked like all
code paths would dereference the tbf.

In fact the code path further above, for msg != NULL, has "always" dereferenced
the tbf, but the lower code path, the one effecting the paging, has only
started to dereference tbf since shortly before the overly eager fix: in
da7250ad2c1cd5ddc7d3c6e10435a00b357ef8f7, to "update the dl ctrl msg counter
for ms". It seems that this tbf dereference in the paging path is bogus and the
cause for the segfault that made me write the early exit fix.

Fix that fix:

Do not exit early if tbf == NULL, stay in there to be able to reach the paging
path below.

In case of a message to be sent, assume that tbf is present, and verify: print
an error message and abort if there is a msg but no tbf, so that we will see
the error if I'm wrong there.

In case of no message, go on to send pending pagings, but do not attempt to
count ctrl messages for a tbf -- IIUC there will never be a tbf if we're
paging.

This should avoid segfaults while keeping PACCH paging intact.

Tweak a comment for and add a blank line above the paging section.

Related: OS#2176 CID#158969
Change-Id: Ib79f4a945e211a13ac7d1e511cc37b0940ac6202
---
M src/gprs_rlcmac_sched.cpp
1 file changed, 7 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/20/2420/2

diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp
index 3b940f4..b40ff9a 100644
--- a/src/gprs_rlcmac_sched.cpp
+++ b/src/gprs_rlcmac_sched.cpp
@@ -178,11 +178,13 @@
 		}
 	}
 
-	if (!tbf)
-		return NULL;
-
 	/* any message */
 	if (msg) {
+		if (!tbf) {
+			LOGP(DRLCMACSCHED, LOGL_ERROR,
+			     "Control message to be scheduled, but no TBF (TRX=%d, TS=%d)\n", trx, ts);
+			return NULL;
+		}
 		tbf->rotate_in_list();
 		LOGP(DRLCMACSCHED, LOGL_DEBUG, "Scheduling control "
 			"message at RTS for %s (TRX=%d, TS=%d)\n",
@@ -191,14 +193,12 @@
 		tbf->ms()->update_dl_ctrl_msg();
 		return msg;
 	}
-	/* schedule PACKET PAGING REQUEST */
+
+	/* schedule PACKET PAGING REQUEST, if any are pending */
 	msg = pdch->packet_paging_request();
 	if (msg) {
 		LOGP(DRLCMACSCHED, LOGL_DEBUG, "Scheduling paging request "
 			"message at RTS for (TRX=%d, TS=%d)\n", trx, ts);
-
-		/* Updates the dl ctrl msg counter for ms */
-		tbf->ms()->update_dl_ctrl_msg();
 		return msg;
 	}
 

-- 
To view, visit https://gerrit.osmocom.org/2420
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib79f4a945e211a13ac7d1e511cc37b0940ac6202
Gerrit-PatchSet: 2
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list