Summary:ASTERISK-22483: AST_LIST_INSERT_TAIL doesn't set field.next on added entry
Reporter:Brian Scott (brianscott)Labels:
Date Opened:2013-09-07 03:35:39Date Closed:2013-10-08 13:28:01
Versions:11.5.1 Frequency of
Environment:FreeBSD 9.1, i386Attachments:( 0) linkedlists.diff
Description:AST_LIST_INSERT_TAIL macro in include/asterisk/linkedlists.h doesn't set field.next to NULL when adding new entries to the tail of the list.
Multiple calls to AST_CONFIG (for different files) in the dialplan causes Asterisk to crash in func_config with a memory error.

Code appears to assume that malloc'ed memory would be initialised to NULL. This isn't a safe assumption on some systems (in this case FreeBSD).

A trivial patch is available or you can work out where the (elm)->field.next = NULL; goes for yourself.

Comments:By: Rusty Newton (rnewton) 2013-09-10 09:45:05.505-0500

Patches are welcome. Thanks for reporting the issue. If you want to submit the patch and get credit for it, please go ahead!

Please observe the requirements and process noted here: https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines#AsteriskIssueGuidelines-PatchandCodesubmission

By: Kinsey Moore (kmoore) 2013-10-08 12:17:27.521-0500

Looking over the patch and the comments above the macro, it appears that the absence of NULL setting is intentional. I'm going to spend some time digging into this to see if there's a good reason behind it since it will be a significant behavior change in the linked list API.

Coming at this from the other side, those pointers should probably already be set to NULL upon allocation and I'm going to attempt to reproduce this to find out where the allocations are coming from.

By: Kinsey Moore (kmoore) 2013-10-08 12:19:36.326-0500

Could you provide a backtrace for this issue?

By: Kinsey Moore (kmoore) 2013-10-08 13:09:21.965-0500

I have reproduced this and I have a patch that fixes the problem. There were two places in func_config.c that used ast_malloc to allocate a list entry. This is bad. Those places now use ast_calloc and I can no longer get it to crash. The fix will be committed to 1.8, 11, 12, and trunk shortly.