[asterisk-dev] [Code Review]: Fix a variety of memory leaks

Matt Jordan reviewboard at asterisk.org
Wed May 16 11:32:49 CDT 2012 


> On May 16, 2012, 10:16 a.m., Tilghman Lesher wrote:
> > /branches/1.8/apps/app_voicemail.c, lines 12527-12532
> > <https://reviewboard.asterisk.org/r/1922/diff/1/?file=27955#file27955line12527>
> >
> >     This changes the codepath.  It's possible that pwconf would load without a "password" field, and thus it should return to reading the password from the configuration file, not return with a blank password.

The only thing this changes is whether or not the LOG_NOTICE is displayed - if no password value is available in the retrieved file, this method does not attempt to put anything into the password buffer, which this code change did not affect.

Since it should still display that notice, I'll change this to do that.


> On May 16, 2012, 10:16 a.m., Tilghman Lesher wrote:
> > /branches/1.8/funcs/func_odbc.c, lines 538-543
> > <https://reviewboard.asterisk.org/r/1922/diff/1/?file=27961#file27961line538>
> >
> >     You should ast_free(resultset) here, too.

Thanks!


> On May 16, 2012, 10:16 a.m., Tilghman Lesher wrote:
> > /branches/1.8/main/cli.c, line 2347
> > <https://reviewboard.asterisk.org/r/1922/diff/1/?file=27962#file27962line2347>
> >
> >     Red blob, while you're here.

Done


> On May 16, 2012, 10:16 a.m., Tilghman Lesher wrote:
> > /branches/1.8/main/editline/readline.c, lines 614-615
> > <https://reviewboard.asterisk.org/r/1922/diff/1/?file=27965#file27965line614>
> >
> >     Braces

Done


> On May 16, 2012, 10:16 a.m., Tilghman Lesher wrote:
> > /branches/1.8/main/editline/readline.c, line 552
> > <https://reviewboard.asterisk.org/r/1922/diff/1/?file=27965#file27965line552>
> >
> >     Since libeditline is replaced from time to time as we get fixes from upstream, any patches to our copy of libeditline should also be submitted upstream.
> 
> Kevin Fleming wrote:
>     I can't remember the last time we actually did that, though :-)
> 
> Tilghman Lesher wrote:
>     It probably doesn't happen enough.  We've probably got a few bugs worth patching from upstream.  I doubt there's any security issues attached to the library, but it would suck if there were, and we didn't catch it until years later.

I'll give that a shot


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1922/#review6221
-----------------------------------------------------------


On May 16, 2012, 9:31 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1922/
> -----------------------------------------------------------
> 
> (Updated May 16, 2012, 9:31 a.m.)
> 
> 
> Review request for Asterisk Developers, otherwiseguy, rmudgett, and opticron.
> 
> 
> Summary
> -------
> 
> This fixes a number of memory leaks in core modules (and a few modules that are extended support, but were easy to fix) that were found by a static analysis tool.  A brief summary of the changes:
> 
> * app_minivm: free ast_str objects on off nominal paths
> * app_page: free the ast_dial object if the requested channel technology cannot be appended to the dialing structure
> * app_queue: if a penalty rule failed to match any existing rule list names, the created rule would not be inserted and its memory would be leaked
> * app_read: dispose of the created silence detector in the presence of off nominal circumstances
> * app_voicemail: dispose of an allocated unique ID field for MWI event un-subscribe requests in off nominal paths; dispose of configuration objects when using the secret.conf option
> * chan_dahdi: dispose of the allocated frame produced by ast_dsp_process
> * chan_iax2: properly unref peer in CLI command "iax2 unregister"
> * chan_sip: dispose of the allocated frame produced by sip_rtp_read's call of ast_dsp_process; free memory in parse unit tests
> * func_dialgroup: properly deref ao2 object grhead in nominal path of dialgroup_read
> * func_odbc: free resultset in off nominal paths of odbc_read
> * cli: free match_list in off nominal paths of CLI match completion
> * config: free comment_buffer/list_buffer when configuration file load is unchanged; free the same buffers any time they were created and config files were processed
> * data: free XML nodes in various places
> * enum: free context buffer in off nominal paths
> * features: free ast_call_feature in off nominal paths of applicationmap config processing
> * netsock2: users of ast_sockaddr_resolve pass in an ast_sockaddr struct that is allocated by the method.  Failures in ast_sockaddr_resolve could result in the users of the method not knowing whether or not the buffer was allocated.  The method will now not allocate the ast_sockaddr struct if it will return failure.
> * pbx: cleanup hash table traversals in off nominal paths; free ignore pattern buffer if it already exists for the specified context
> * xmldoc: cleanup various nodes when we no longer need them
> * main/editline: various cleanup of pointers not being freed before being assigned to other memory, cleanup along off nominal paths
> * menuselect/mxml: cleanup of value buffer for an attribute when that attribute did not specify a value
> * res_calendar*: responses are allocated via the various *_request method returns and should not be allocated in the various write_event methods; ensure attendee buffer is freed if no data exists in the parsed node; ensure that calendar objects are de-ref'd appropriately
> * res_jabber: free buffer in off nominal path
> * res_musiconhold: close the DIR* object in off nominal paths
> * res_rtp_asterisk: if we run out of ports, close the rtp socket object and free the rtp object
> * res_srtp: if we fail to create the session in libsrtp, destroy the temporary ast_srtp object
> 
> 
> This addresses bug ASTERISK-19665.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19665
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/apps/app_minivm.c 366648 
>   /branches/1.8/apps/app_page.c 366648 
>   /branches/1.8/apps/app_queue.c 366648 
>   /branches/1.8/apps/app_record.c 366648 
>   /branches/1.8/apps/app_voicemail.c 366648 
>   /branches/1.8/channels/chan_dahdi.c 366648 
>   /branches/1.8/channels/chan_iax2.c 366648 
>   /branches/1.8/channels/chan_sip.c 366648 
>   /branches/1.8/channels/sip/config_parser.c 366648 
>   /branches/1.8/funcs/func_dialgroup.c 366648 
>   /branches/1.8/funcs/func_odbc.c 366648 
>   /branches/1.8/main/cli.c 366648 
>   /branches/1.8/main/config.c 366648 
>   /branches/1.8/main/data.c 366648 
>   /branches/1.8/main/editline/readline.c 366648 
>   /branches/1.8/main/editline/term.c 366648 
>   /branches/1.8/main/editline/tokenizer.c 366648 
>   /branches/1.8/main/enum.c 366648 
>   /branches/1.8/main/features.c 366648 
>   /branches/1.8/main/netsock2.c 366648 
>   /branches/1.8/main/pbx.c 366648 
>   /branches/1.8/main/xmldoc.c 366648 
>   /branches/1.8/res/res_calendar.c 366648 
>   /branches/1.8/res/res_calendar_caldav.c 366648 
>   /branches/1.8/res/res_calendar_exchange.c 366648 
>   /branches/1.8/res/res_calendar_icalendar.c 366648 
>   /branches/1.8/res/res_jabber.c 366648 
>   /branches/1.8/res/res_musiconhold.c 366648 
>   /branches/1.8/res/res_rtp_asterisk.c 366648 
>   /branches/1.8/res/res_srtp.c 366648 
> 
> Diff: https://reviewboard.asterisk.org/r/1922/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120516/344b28fc/attachment-0001.htm>

More information about the asterisk-dev mailing list