| Summary: | ASTERISK-06359: __list_prev not treated properly in list deletion in AST_LIST_TRAVERSE_SAFE_BEGIN | ||
| Reporter: | Andrew Moise (chops) | Labels: | |
| Date Opened: | 2006-02-19 00:05:34.000-0600 | Date Closed: | 2006-02-23 13:43:11.000-0600 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Core/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) 2006-02-19-list-prev.patch | |
| Description: | The "__list__prev" variable used to iterate over a linked list in AST_LIST_TRAVERSE_SAFE_BEGIN isn't protected against being set to a now-removed member. For example, if you traverse a list with elements (one, two, three): AST_LIST_TRAVERSE_SAFE_BEGIN; (__list_prev == NULL, var == one) AST_LIST_REMOVE_CURRENT; (__list_prev == NULL, var == one) free(one); (__list_prev == NULL, var == one) AST_LIST_TRAVERSE_SAFE_BEGIN; (the second iteration of the for loop) (__list_prev == one, var == two) AST_LIST_REMOVE_CURRENT; ... and within that macro, the line: __list_prev->field.next = __list_next; ... gets executed, which (a) leaves the list in an incorrect state (because it should be head->first that gets updated to 'three' instead), and (b) accesses freed memory within 'one'. The attached patch changes the safe list traversal macros to be, AFAICS, correct. It also corrects AST_LIST_INSERT_BEFORE_CURRENT, which was also not updating __list_prev, with similar results to the above. | ||
| Comments: | By: Tilghman Lesher (tilghman) 2006-02-19 12:16:13.000-0600 Given that these macros are also in 1.2, this would be a candidate for inclusion in the release branch. By: Matt O'Gorman (mogorman) 2006-02-23 13:43:03.000-0600 should already be fixed in trunk and svn 1.2 , was fixed as it caused a bug in trunk with voicemail sorry didnt notice your patch please see bug 6557 committed into trunk 10766 and 1.2 10863 thanks though | ||