libmsc/sms_queue.c: fix memleak in smsq_take_next_sms()

A memleak has been noticed after executing some of TTCN-3 test
cases. For example, the following ones:

  - MSC_Tests.TC_lu_and_mo_sms,
  - MSC_Tests.TC_lu_and_mt_sms.

The key point is that MSC_Tests.TC_lu_and_mo_sms basically sends
a MO SMS to a non-attached subscriber with MSISDN 12345, so this
message is getting stored in the SMSC's database.

As soon as the SMSC's queue is triggered, sms_submit_pending() would
retrieve pending messages from the database by calling function
smsq_take_next_sms() in loop and attempt to deliver them.

This function in it's turn checks whether the subscriber is attached
or not. If not, the allocated 'gsm_sms' structure would not be
free()ed! Therefore, every time smsq_take_next_sms() is called,
one 'gsm_sms' structure for an unattached subscriber is leaked.

Furthermore, there is a unit test called 'sms_queue_test', that
actually does cover smsq_take_next_sms() and was designed to
catch some potential memory leaks, but...

In order to avoid emulating the low-level SQLite API, the unit
test by design overwrites some functions of libmsc, including
db_sms_get_next_unsent_rr_msisdn(), that is being called by
smsq_take_next_sms().

The problem is that the original function in libmsc does
allocate a 'gsm_sms' structure on heap (using talloc), while
the overwriting function did this statically, returning a
pointer to stack. This critical difference made it impossible
to spot the memleak in smsq_take_next_sms() during the
unit test execution.

Let's refactor 'sms_queue_test' to use dynamic memory allocation,
and finally fix the evil memleak in smsq_take_next_sms().

Change-Id: Iad5e4d84d8d410ea43d5907e9ddf6e5fdb55bc7a
Closes: OS#3860

1 files changed, 6 insertions, 1 deletions

diff --git a/src/libmsc/sms_queue.c b/src/libmsc/sms_queue.c
index c924ddecf..274c71295 100644
--- a/src/libmsc/sms_queue.c
+++ b/src/libmsc/sms_queue.c
@@ -226,8 +226,13 @@ struct gsm_sms *smsq_take_next_sms(struct gsm_network *net,
 		osmo_strlcpy(last_msisdn, sms->dst.addr, last_msisdn_buflen);
 
 		/* Is the subscriber attached? If not, go to next SMS */
-		if (!sms->receiver || !sms->receiver->lu_complete)
+		if (!sms->receiver || !sms->receiver->lu_complete) {
+			LOGP(DLSMS, LOGL_DEBUG,
+			     "Subscriber %s is not attached, skipping SMS %llu\n",
+			     vlr_subscr_msisdn_or_name(sms->receiver), sms->id);
+			sms_free(sms);
 			continue;
+		}
 
 		return sms;
 	}