[PATCH] openbsc[master]: subscr_update_expire_lu(): fix (obscure) segfault

[Neels Hofmeyr]

Review at  https://gerrit.osmocom.org/1850

subscr_update_expire_lu(): fix (obscure) segfault

To be paranoid, catch a NULL subscriber and/or bts in
subscr_update_expire_lu(): print an error log and avoid segfault.
(I'm not sure this would really happen in a normal situation.)

During aggressive testing of Paging timeout, I came across this segfault in
msc_release_connection() when conn->expire_timer_stopped is set but
conn->subscr is NULL, at the subscr dereference after:

        if (conn->expire_timer_stopped)
                subscr_update_expire_lu(conn->subscr, conn->bts);

I brought this situation about by a fabricated Paging fault, i.e. in
gsm48_rx_rr_pag_resp() return 0 and don't call gsm48_handle_paging_resp() at
all. Thus conn->subscr is still NULL when expire_timer_stopped is 1.

When looking at CM Service Request handling, the conn->subscr is set before
setting expire_timer_stopped = 1, which is a saner thing to do. But without my
mad 'return 0', there is in fact no way to have a NULL subscriber there.

It looks like all other code paths already do the same, but it's not that
obvious (e.g. _gsm48_rx_mm_serv_req_sec_cb()). So rather catch this case of
NULL conn->subscr, and while at it catch NULL bts as well.

Change-Id: I430dd952b2b928bea7f8360f1e01bb3cccb0a395
---
M openbsc/src/libmsc/gsm_subscriber.c
1 file changed, 10 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/50/1850/1

diff --git a/openbsc/src/libmsc/gsm_subscriber.c b/openbsc/src/libmsc/gsm_subscriber.c
index 4ec0ead..568f1c5 100644
--- a/openbsc/src/libmsc/gsm_subscriber.c
+++ b/openbsc/src/libmsc/gsm_subscriber.c
@@ -276,6 +276,16 @@
 {
 	int rc;
 
+	if (!s) {
+		LOGP(DMM, LOGL_ERROR, "LU Expiration but NULL subscriber\n");
+		return -1;
+	}
+	if (!bts) {
+		LOGP(DMM, LOGL_ERROR, "%s: LU Expiration but NULL bts\n",
+		     subscr_name(s));
+		return -1;
+	}
+
 	/* Table 10.5.33: The T3212 timeout value field is coded as the
 	 * binary representation of the timeout value for
 	 * periodic updating in decihours. Mark the subscriber as

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I430dd952b2b928bea7f8360f1e01bb3cccb0a395
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>