Summary:ASTERISK-12398: [patch] avoid a leak in an error handling path
Reporter:Guillaume Knispel (gknispel_proformatique)Labels:
Date Opened:2008-07-17 07:15:33Date Closed:2008-07-17 10:37:45
Versions:Frequency of
Environment:Attachments:( 0) realtime_multi_handler_avoid_leak_on_error.patch
Description:In realtime_multi_handler(), query is not correctly freed if the ORDER BY sqlite_mprintf() fails.
Comments:By: Tilghman Lesher (tilghman) 2008-07-17 08:11:21

I think you misunderstand the code.  There is no memory leak, here.  If the sqlite_mprintf fails, then it returns NULL, which is a value that cannot be freed.

By: Guillaume Knispel (gknispel_proformatique) 2008-07-17 09:11:51

The last previous allocation of query is done either on line 1059 or on line 1073 (in tmp_str then the pointer is copied to query on line 1085)

On line 1092 tmp_str is allocated. query is used, but neither allocated nor freed.
Therefore if allocation on line 1092 fails, tmp_str will contain NULL but query will be left allocated because it is not freed before the return NULL;

realtime_multi_handler_avoid_leak_on_error.patch applies the same principle as what appears between lines 1073 to 1085.

BTW this bug was not present in the file we sent you initially http://bugs.digium.com/file_download.php?file_id=11311&type=bug

And rereading our initial code and what is now in Asterisk trunk, you can apply the same fix to realtime_update_handler(), realtime_destroy_handler() and there are similar errors at least in realtime_store_handler().

Indeed I agree that the code is a little hard to follow. I might partially rewrite it one day.

By: Digium Subversion (svnbot) 2008-07-17 10:37:44

Repository: asterisk
Revision: 131681

U   trunk/res/res_config_sqlite.c

r131681 | tilghman | 2008-07-17 10:37:41 -0500 (Thu, 17 Jul 2008) | 4 lines

Fix memory leak.
(Closes issue ASTERISK-12398)
Reported by gknispel_proformatique