diff options
author | Vadim Yanitskiy <axilirator@gmail.com> | 2019-03-28 21:25:14 +0700 |
---|---|---|
committer | Vadim Yanitskiy <axilirator@gmail.com> | 2019-04-01 12:02:57 +0000 |
commit | 96262a7ca67115482cfb89f9954413638b1d5dcc (patch) | |
tree | a34e6e63d1b5f4c371600ef967da2501a1ad5c73 /src/libmsc/sms_queue.c | |
parent | 18f1138a6d2298977f30d907d5cd9b512523cfe1 (diff) |
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
Diffstat (limited to 'src/libmsc/sms_queue.c')
-rw-r--r-- | src/libmsc/sms_queue.c | 7 |
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; } |