[asterisk-dev] [Code Review]: Fix potential memory allocation failure crashes in config.c.

[rmudgett]

> On Aug. 23, 2011, 10:07 p.m., Tilghman Lesher wrote:
> > /branches/1.8/include/asterisk/config.h, lines 80-81
> > <https://reviewboard.asterisk.org/r/1378/diff/1/?file=18572#file18572line80>
> >
> >     Since this is 1.8, you really shouldn't be changing the order of elements here, as it destroys ABI compatibility.  If you want to do this in trunk, that's another matter entirely.

done


> On Aug. 23, 2011, 10:07 p.m., Tilghman Lesher wrote:
> > /branches/1.8/main/config.c, lines 1043-1045
> > <https://reviewboard.asterisk.org/r/1378/diff/1/?file=18573#file18573line1043>
> >
> >     Might as well go ahead and delete this.

done


- rmudgett


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


On Aug. 23, 2011, 11:32 a.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1378/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2011, 11:32 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Fix potential memory allocation failure crashes in config.c.
> 
> * Added required checks to the returned memory allocation pointers to
> prevent crashes.
> 
> * Made ast_include_rename() create a replacement ast_variable list node if
> the new filename is longer than the available space.  Fixes potential
> crash and memory leak.
> 
> * Factored out ast_variable_move() from ast_variable_update() so
> ast_include_rename() can also use it when creating a replacement
> ast_variable list node.
> 
> * Made the filename stuffed at the end of the struct a minimum allocated
> size in ast_variable_new() in case ast_include_rename() changes the stored
> filename.
> 
> * Constify struct char pointers pointing to strings stuffed at the end of
> the struct for: ast_variable, cache_file_mtime, and ast_config_map.
> 
> * Factored out cfmtime_new() to remove inlined code and allow some struct
> pointers to become const.
> 
> * Removed the list lock from struct cache_file_mtime that was never used.
> 
> * Added doxygen comments to several structure elements and better
> documented what strings are stuffed at the struct end char array.
> 
> * Reworked ast_config_text_file_save() and set_fn() to handle allocation
> failure of the include file scratch pad object tracking blank lines.
> 
> * Made ast_config_text_file_save() fn[] declared with PATH_MAX to ensure
> it is long enough for any filename with path.  Also reduced the number of
> container fileset buckets from a rediculus 180,000 to 1023.
> 
> JIRA AST-618
> 
> 
> This addresses bug AST-618.
>     https://issues.asterisk.org/jira/browse/AST-618
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/include/asterisk/config.h 333000 
>   /branches/1.8/main/config.c 333000 
> 
> Diff: https://reviewboard.asterisk.org/r/1378/diff
> 
> 
> Testing
> -------
> 
> 1) Started asterisk which loads config files.
> 2) Changed voicemail password which writes a new voicemail.conf.
> 3) Executed the AMI action UpdateConfig which reads/modifies/writes the specified config file.  It is the only caller of ast_include_rename().
> 
> Things still work and valgrind was not unhappy.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110824/a385879a/attachment.htm>