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

[rmudgett]

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

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/20110823/cb0182bb/attachment.htm>