[Home]

Summary:ASTERISK-05563: [patch] SIP Call-Id/tag generation is not unique
Reporter:Terry Wilson (twilson)Labels:
Date Opened:2005-11-10 16:08:42.000-0600Date Closed:2008-01-15 15:55:53.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20051110__sip_randomness.diff.txt
Description:On a heavily loaded server, we have noticed that SIP calls that happen very close to each other (< .1 second) often have the same SIP Call-ID and .  This is *very bad* as two calls get to overwrite each other's sip_pvt structure, when one hangs up they both do, etc.

****** ADDITIONAL INFORMATION ******

If you look at build_callid in chan_sip.c it uses the rand() function which is not thread safe.  Two different threads can end up getting the same number back.
Comments:By: Tilghman Lesher (tilghman) 2005-11-10 17:54:57.000-0600

Patch uploaded to replace calls to rand() with calls to random().

By: Kevin P. Fleming (kpfleming) 2005-11-10 18:02:39.000-0600

Can you point me to some documentation showing that random() is known to be thread-safe? I can't seem to find any with a quick look...

By: Tilghman Lesher (tilghman) 2005-11-10 18:48:24.000-0600

kpfleming:

I read the source to glibc; it's definitely thread-safe:  there's a mutex around the routine.

The source comes from FreeBSD, but the FreeBSD version is not thread-safe.  While Linux has random_r(), FreeBSD does not.  I might need to do an import of the routine into utils.c, simply so we have a threadsafe version.

By: Kevin P. Fleming (kpfleming) 2005-11-10 18:54:45.000-0600

Well, first off, that could be a licensing concern... we need to verify that situation.

Second, when I was talking to twilson about this on IRC today, he proposed the possibility of using a mutex wrapper; I'm not thrilled about how that will impact performance, but there is no simpler solution at this time.

So, let's go a simpler route: how about just adding a mutex lock to chan_sip itself so that _it_ won't call rand() from two threads at the same time? We don't really care whether chan_sip and some other module receive the same value, as long as two chan_sip threads don't.

By: Tilghman Lesher (tilghman) 2005-11-10 20:12:33.000-0600

There is no licensing concern with code from FreeBSD -- it's BSD licensed, which is compatible with our own (as long as the attribution remains intact).

Note that the editline library and the db1 library included with Asterisk are both under exactly the same license, verbatim.

By: Kevin P. Fleming (kpfleming) 2005-11-10 20:13:55.000-0600

Ahh, I thought you were referring to taking the code from glibc, not FreeBSD... didn't read closely enough.

By: Kevin P. Fleming (kpfleming) 2005-11-10 20:15:52.000-0600

Since the FreeBSD version of random() is known to be non thread-safe, are we risking a portability problem here?

By: Tilghman Lesher (tilghman) 2005-11-10 20:19:20.000-0600

Not if we force it to be our own implementation, i.e. ast_random().

By: Kevin P. Fleming (kpfleming) 2005-11-10 20:24:58.000-0600

True. I'd be willing to let that get in before the final release if you're comfortable putting it together in the next couple of days. This is an important bug to get fixed.

By: Kevin P. Fleming (kpfleming) 2005-11-11 22:08:06.000-0600

Fixed in CVS HEAD.

The random() source code in glibc is a GPL-licensed modified version of the original BSD source; as such, we can't pull it into Asterisk.

As a quick fix, I added a mutex around rand() calls in chan_sip only; we can determine if there is a better performing solution in the future.

By: Digium Subversion (svnbot) 2008-01-15 15:55:53.000-0600

Repository: asterisk
Revision: 7086

U   trunk/ChangeLog
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r7086 | kpfleming | 2008-01-15 15:55:53 -0600 (Tue, 15 Jan 2008) | 2 lines

issue ASTERISK-5563

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=7086