[asterisk-dev] ASTERISK-26978 - rtp: Crash in ast_rtp_codecs_payload_code()

[Joshua Colp]

On Wed, May 24, 2017, at 12:12 PM, Ross Beer wrote:
> >>
> 
> >> Therefore I have added the following code to check for this:
> >>
> >>
> >>         if (format1->codec == NULL || format2->codec == NULL) {
> >> return AST_FORMAT_CMP_NOT_EQUAL;
> >> }
> >>
> >> The question is, should 'codec' be NULL if 'format1' and 'format2' are
> .> not NULL? Is adding the above check, the correct fix?
> 
> >A format can't be created and remain valid without a codec being present
> >on it. A format itself is a codec + extra data about it. Identifying how
> >it became NULL and why the format is no longer valid would uncover the
> >real fix.
> 
> Does the format object need locking so that anything acting on it doesn't
> have the object pulled from under it?
> 
> My theory is that a channel is attempting to unallocated the 'format'
> object while it is trying to be compared. It, therefore, makes it through
> the NULL checks on 'format1' and 'format2' but is in the process of being
> freed.

Once created a format is considered immutable - it can't be changed.
Only a new one based on it can be created. The only reason it would be
destroyed is if the reference count is not correct. Anything having a
pointer to it should hold a reference.

> 
> There are quite a few backtraces on this and the linked issue. Would
> anyone be willing to take a look, my C skills are not that good?

The issue is in queue to be looked at by us (Digium). I have no
timeframe on that. Perhaps someone else in the community will take a
look.

-- 
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org