From 96262a7ca67115482cfb89f9954413638b1d5dcc Mon Sep 17 00:00:00 2001 From: Vadim Yanitskiy Date: Thu, 28 Mar 2019 21:25:14 +0700 Subject: 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 --- src/libmsc/sms_queue.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src') 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; } -- cgit v1.2.3