[PATCH] Leaks and segfaults in smsbox
Peter Christensen <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi, In relation to the thread "Memory leak in smsbox", here's a patch which corrects some of the errors. The patch reorganizes the error goto-fields so they are as followed: fielderror1: At this point, only receiver have been allocated fielderror2: receiver, allowed, and denied have been allocated (allowed and denied may contain elements from receiver) fielderror3: receiver, allowed, denied, newfrom, and msg have been allocated (allowed and denied contains elements from receiver) error: receiver, allowed, denied, newfrom, and failed_id have been allocated (allowed is empty, and denied and failed_id may contain elements from receiver) Besides, the "Number(s) has/have been denied by white- and/or black-lists." error have been moved to before SMS transmission is attempted, and the logic is now gwlist_len(allowed) == 0 (No recipient have been allowed) This patch overrules my original patch for the memory leak. For the record; While monitoring its memory consumption, I've flooded smsbox with messages for each possible error condition (except conditions where recipients are added to the failed_id list. Don't know how to provoke that condition), and there seems to be no memory leaks, double frees, or segmentation faults. -- Med venlig hilsen / Best regards Peter Christensen Developer ------------------ Cool Systems ApS Tel: +45 2888 1600 @ : [email protected] www: www.coolsystems.dk
kannel.smsbox.fix.diff
(text/x-patch, 6.4 KB)
diff -Nru gateway/gw/smsbox.c gateway.smsbox/gw/smsbox.c
--- gateway/gw/smsbox.c 2005-12-09 03:14:31.000000000 +0100
+++ gateway.smsbox/gw/smsbox.c 2006-01-23 17:51:55.000000000 +0100
@@ -1990,11 +1990,11 @@
*/
if (udh != NULL && (octstr_len(udh) != octstr_get_char(udh, 0) + 1)) {
returnerror = octstr_create("UDH field misformed, rejected");
- goto fielderror2;
+ goto fielderror1;
}
if (udh != NULL && octstr_len(udh) > MAX_SMS_OCTETS) {
returnerror = octstr_create("UDH field is too long, rejected");
- goto fielderror2;
+ goto fielderror1;
}
/*
@@ -2108,6 +2108,12 @@
del = gwlist_delete_matching(allowed, receiv, octstr_item_match);
}
+ /* have all receivers been denied by list rules?! */
+ if (gwlist_len(allowed) == 0) {
+ returnerror = octstr_create("Number(s) has/have been denied by white- and/or black-lists.");
+ goto fielderror2;
+ }
+
if (urltrans_faked_sender(t) != NULL) {
/* discard previous from */
newfrom = octstr_duplicate(urltrans_faked_sender(t));
@@ -2144,7 +2150,7 @@
msg->sms.account = account ? octstr_duplicate(account) : NULL;
} else {
returnerror = octstr_create("Account field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
}
msg->sms.msgdata = text ? octstr_duplicate(text) : octstr_create("");
@@ -2156,7 +2162,7 @@
if(octstr_len(dlr_url)) {
if(octstr_len(dlr_url) < 8) { /* http(s):// */
returnerror = octstr_create("DLR-URL field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
} else {
Octstr *tmp;
tmp = octstr_copy(dlr_url, 0, 7);
@@ -2168,7 +2174,7 @@
if(octstr_case_compare(tmp, octstr_imm("https://")) != 0) {
returnerror = octstr_create("DLR-URL field misformed, rejected");
O_DESTROY(tmp);
- goto fielderror;
+ goto fielderror3;
}
#ifdef HAVE_LIBSSL
msg->sms.dlr_url = octstr_duplicate(dlr_url);
@@ -2187,49 +2193,49 @@
if ( dlr_mask < -1 || dlr_mask > 31 ) { /* 00011111 */
returnerror = octstr_create("DLR-Mask field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.dlr_mask = dlr_mask;
if ( mclass < -1 || mclass > 3 ) {
returnerror = octstr_create("MClass field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.mclass = mclass;
if ( pid < -1 || pid > 255 ) {
returnerror = octstr_create("PID field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.pid = pid;
if ( rpi < -1 || rpi > 2) {
returnerror = octstr_create("RPI field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.rpi = rpi;
if ( alt_dcs < -1 || alt_dcs > 1 ) {
returnerror = octstr_create("Alt-DCS field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.alt_dcs = alt_dcs;
if ( mwi < -1 || mwi > 7 ) {
returnerror = octstr_create("MWI field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.mwi = mwi;
if ( coding < -1 || coding > 2 ) {
returnerror = octstr_create("Coding field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.coding = coding;
if ( compress < -1 || compress > 1 ) {
returnerror = octstr_create("Compress field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.compress = compress;
@@ -2244,19 +2250,19 @@
if ( validity < -1 ) {
returnerror = octstr_create("Validity field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.validity = validity;
if ( deferred < -1 ) {
returnerror = octstr_create("Deferred field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.deferred = deferred;
if (priority != SMS_PARAM_UNDEFINED && (priority < 0 || priority > 3)) {
returnerror = octstr_create("Priority field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.priority = priority;
@@ -2279,7 +2285,7 @@
if (charset_processing(charset, msg->sms.msgdata, msg->sms.coding) == -1) {
returnerror = octstr_create("Charset or body misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.receiver = NULL;
@@ -2322,19 +2328,12 @@
msg_destroy(msg);
- gwlist_destroy(receiver, octstr_destroy_item);
- gwlist_destroy(allowed, octstr_destroy_item);
-
- /* have all receivers been denied by list rules?! */
- if (no_recv == gwlist_len(denied)) {
- returnerror = octstr_create("Number(s) has/have been denied by white- and/or black-lists.");
- goto fielderror2;
- }
if (gwlist_len(failed_id) > 0)
goto error;
- gwlist_destroy(failed_id, octstr_destroy_item);
+ gwlist_destroy(failed_id, NULL);
+ gwlist_destroy(allowed, NULL);
octstr_destroy(newfrom);
*status = HTTP_ACCEPTED;
returnerror = octstr_create("Sent.");
@@ -2348,8 +2347,9 @@
while ((receiv = gwlist_extract_first(denied)) != NULL) {
octstr_format_append(returnerror, " %s", octstr_get_cstr(receiv));
}
- }
- gwlist_destroy(denied, octstr_destroy_item);
+ }
+ gwlist_destroy(denied, NULL);
+ gwlist_destroy(receiver, octstr_destroy_item);
/*
* Append number of splits to returned body.
@@ -2361,11 +2361,17 @@
return returnerror;
-fielderror:
- octstr_destroy(newfrom);
+fielderror3:
msg_destroy(msg);
-
+ octstr_destroy(newfrom);
+
fielderror2:
+ gwlist_destroy(allowed, NULL);
+ gwlist_destroy(denied, NULL);
+
+fielderror1:
+ gwlist_destroy(receiver, octstr_destroy_item);
+
alog("send-SMS request failed - %s",
octstr_get_cstr(returnerror));
@@ -2374,7 +2380,6 @@
error:
error(0, "sendsms_request: failed");
- octstr_destroy(from);
*status = HTTP_INTERNAL_SERVER_ERROR;
returnerror = octstr_create("Sending failed.");
@@ -2389,9 +2394,11 @@
}
}
- octstr_destroy(receiv);
- gwlist_destroy(failed_id, octstr_destroy_item);
- gwlist_destroy(denied, octstr_destroy_item);
+ gwlist_destroy(failed_id, NULL);
+ gwlist_destroy(allowed, NULL);
+ gwlist_destroy(denied, NULL);
+ gwlist_destroy(receiver, octstr_destroy_item);
+ octstr_destroy(newfrom);
return returnerror;
}