[asterisk-dev] [asterisk-commits] tilghman: trunk r228798 - in /trunk: addons/channels/ main/ res/

[Tilghman Lesher]

On Monday 09 November 2009 02:58:41 am Alec Davis wrote:
> Tilghman.
> trunk/addons/cdr_mysql.c
>
> Although changed, going by the diff attached here doesn't appear any
> different?
> But the commit comment suggests that it should be.
>
> Alec
>
>
> -----Original Message-----
> From: asterisk-commits-bounces at lists.digium.com
> [mailto:asterisk-commits-bounces at lists.digium.com] On Behalf Of SVN commits
> to the Asterisk project
> Sent: Monday, 9 November 2009 8:38 p.m.
> To: asterisk-commits at lists.digium.com; svn-commits at lists.digium.com
> Subject: [asterisk-commits] tilghman: trunk r228798 - in /trunk:
> addons/channels/ main/ res/
>
> Author: tilghman
> Date: Mon Nov  9 01:37:52 2009
> New Revision: 228798
>
> URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=228798
> Log:
> Fix various problems detected with Valgrind.
>  * chan_console accessed pvts after deallocation.
>  * cdr_mysql stored a pointer that was freed by realloc()
>  * The module loader did not check usecount on shutdown, which led to
> chan_iax2  reading a timer that was already unloaded.
>  * The event subsystem sometimes creates an event with no IEs.  Due to a
> corner  condition, the code would read beyond the memory boundary.
>  * res_pktccops did not correctly check whether its monitor thread was
> started.
> (closes issue #16062)
>  Reported by: alexanderheinz
>  Patches:
>        20091109__issue16062.diff.txt uploaded by tilghman (license 14)
> Tested by: tilghman
>
> Modified:
>     trunk/addons/cdr_mysql.c
>     trunk/channels/chan_console.c
>     trunk/main/event.c
>     trunk/main/loader.c
>     trunk/res/res_pktccops.c
>
> Modified: trunk/addons/cdr_mysql.c
> URL:
> http://svnview.digium.com/svn/asterisk/trunk/addons/cdr_mysql.c?view=diff&r
>e v=228798&r1=228797&r2=228798
> ===========================================================================
>= ==
> --- trunk/addons/cdr_mysql.c (original)
> +++ trunk/addons/cdr_mysql.c Mon Nov  9 01:37:52 2009
> @@ -364,15 +364,15 @@
>  		return -1;
>  	}
>
> +	tmp = ast_variable_retrieve(cfg, category, variable);
> +
> +	ast_str_set(field, 0, "%s", tmp ? tmp : def);
> +
>  	us->str = *field;
>
>  	AST_LIST_LOCK(&unload_strings);
>  	AST_LIST_INSERT_HEAD(&unload_strings, us, entry);
>  	AST_LIST_UNLOCK(&unload_strings);
> -
> -	tmp = ast_variable_retrieve(cfg, category, variable);
> -
> -	ast_str_set(field, 0, "%s", tmp ? tmp : def);
>
>  	return 0;
>  }

What you aren't seeing is that ast_str_set() may (and in most cases
does) do a realloc() on 'field', which causes the prior pointer to be freed.
Thus, the unload strings list previously contained a stale pointer.  By
rearranging the code, the pointer that gets stored is the reallocated pointer.

-- 
Tilghman