| Summary: | ASTERISK-24535: stringfields: Fix regression from fix for unintentional memory retention and another issue exposed by the fix | ||||
| Reporter: | Corey Farrell (coreyfarrell) | Labels: | |||
| Date Opened: | 2014-11-18 08:24:41.000-0600 | Date Closed: | 2014-11-19 13:32:26.000-0600 | ||
| Priority: | Major | Regression? | Yes | ||
| Status: | Closed/Complete | Components: | Core/AstMM | ||
| Versions: | SVN 12.7.0 13.0.0 | Frequency of Occurrence | |||
| Related Issues: |
| ||||
| Environment: | Attachments: | ||||
| Description: | This addresses a regression in the previous fix that applies to all versions. When __ast_string_field_ptr_grow is called by ast_string_field_ptr_set_by_fields, it needs to be passed &target, not __p__. In the current code if __ast_string_field_ptr_grow actually does anything, it will cause *__p__ != target. Unfortunately in this case *__p__ points to the new address, target to the old. This may cause too much data to be written to the old space for the string, and this could cause memory corruption. This was the first thing I noticed when attempting to troubleshoot res/parking/dynamic_parking_variables unit test failure, but did not fix the test.
Also needed to get the unit test working again is a fix for ast_string_fields_copy in 12+. The existing code relies on the fact that __ast_string_field_release_active never previously zeroed pool->used, so keeping the existing pointer was ok. Setting each field to __ast_string_field_empty after releasing the memory seems to resolve the issue. | ||||
| Comments: | By: Corey Farrell (coreyfarrell) 2014-11-18 08:27:02.189-0600 This is a regression caused by ASTERISK-24307. By: Corey Farrell (coreyfarrell) 2014-11-18 12:34:10.696-0600 It appears that the issue I thought applied to 1.8/11 does not exist. So this issue applies to 12+ only. | ||||