[asterisk-users] More issues with Siren14 datalen == 0 packets

[Joshua Colp]

On Wed, Apr 12, 2017, at 10:09 AM, Richard Kenner wrote:
> > All patches need to go into JIRA with a license agreement to be
> > accepted.
> 
> Understood, but I was using it as an illustration.  Note, however, that,
> from a legal perspective, a patch such as this has no protectable IP (you
> can't copyright the only way of doing something) and the GNU projects
> have
> a formal rule that sufficiently-small patches need no assignments for
> that
> reason, which I suggest you may want to adopt as well.

Matt Fredrickson can pass this on and see.

> 
> > > Why is samples being used as a length instead of datalen?
> > 
> > Internally a signed linear factory operates in terms of samples, not the
> > data payload itself. I've also commented on your original issue in
> > regards to the siren codecs that it should NULL out the data pointer
> > itself. That is more commonly used.
> 
> But I don't think that it would have helped in either case, this one
> or in func_speex.c, because neither tests for a null data pointer either.

The feed function in slinfactory explicitly does not allow frames
without a data payload to be added to the queue. It would have prevented
this crash.

> 
> Can you explain the difference between "datalen" and "samples" in this
> context, shouldn't they always be related by a (small) linear factor?

Nope. The feed actually has a comment detailing a situation which
explains why we check:

        /* In some cases, we can be passed a frame which has no data in
        it, but
         * which has a positive number of samples defined. Once such
         situation is
         * when a jitter buffer is in use and the jitter buffer
         interpolates a frame.
         * The frame it produces has data set to NULL, datalen set to 0,
         and samples
         * set to either 160 or 240.
 
> Should I open a JIRA issue for this as well?

I think the underlying issue is that the data pointer is not NULL when
it sanely should be in the codec implementation.
 
> Can you suggest ways of searching for other possible occurrences of
> this bug?  These crashes are occurring during important conferences and
> are causing significant issues.

Not really. Frames are used a lot across Asterisk, so you have to follow
the flow based on the features in use.

-- 
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