From a1613695d1472e61c85706a8daeb2f83ba5364dd Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Sun, 19 Feb 2017 01:14:39 +0100 Subject: 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 --- openbsc/src/libmsc/gsm_subscriber.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'openbsc/src') diff --git a/openbsc/src/libmsc/gsm_subscriber.c b/openbsc/src/libmsc/gsm_subscriber.c index 4ec0eadf9..568f1c5d0 100644 --- a/openbsc/src/libmsc/gsm_subscriber.c +++ b/openbsc/src/libmsc/gsm_subscriber.c @@ -276,6 +276,16 @@ int subscr_update_expire_lu(struct gsm_subscriber *s, struct gsm_bts *bts) { 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 -- cgit v1.2.3