[Serusers] Deadly embrace with rtpproxy - Caused by coding error? Possible fix attached

[Frank Durda IV]

An update to an issue I brought up in December and had nothing new
to report until now.

To recap, I have rtpproxy in a two-interface configuration (which based
on comments here few people actually use), and the problem was that
rtpproxy was refusing to forward audio to either party until it had
received at least one audio packet from both parties.  If either
party also had a similar policy, you had a deadly embrace and
a dead-air call.  No ring-back (for 183 calls), and no call audio.

With the assistance of some people here, I got the stock debug plus
some of my own going and determined that the "pre-filling caller/callee
address with..." events never took place, so the address data didn't
get filled in until the first RTP packets arrived from both parties,
when a plain "filling caller/callee" would occur.  Then the audio
flow in both directions would commence.

Both the INVITE and the 183/200 messages always had perfectly sane
SDP payloads, but this didn't seem to matter.

I finally got the time to go through the command recived code leading up
to the decision to prefill or not with a high level of debugging.
I noted what appears to be a code flaw in rtpproxy.  The version
that I am looking at is "main.c,v 1.62 2008/02/04 08:38:05 sobomax".

In particular, there is a array in the structure "rtpp_session" called
"int untrusted_addr[2]".  Despite being an array, two of the three
references to untrusted_addr that exist (all in main.c) treat it as
though it was a non-array element of the structure, so when it
was tested for zero or non-zero state in two if() statements, what
actually got tested was the memory address of the start of the array
(always non-zero on my platforms), and not the content of either
array element.

As a further confirmation that something was wacky here, the third
reference to untrusted_addr did specify which element it was
placing the value "1" into, like it expected the other pieces of
code to be able to view that value later.

It appears that the impact of these possible incorrect tests would
not really matter unless two interfaces are in use, so it is possible
this is a coding error that has gone undetected since this bit of
code was originally written.


I changed the two tests that looked improper to examine the same
array element as that of the other items located in the same
structure that the if() statement was also testing, and rtpproxy
immediately started prefilling the addresses for the session
in response to the INVITE, 183 and 200 SDP payloads when
two interfaces are in use.

This appears to have corrected the problem I was encountering.

Attached is the tw-line-change context diff.  I would appreciate if
someone more familiar with rtpproxy would bless the change.  The
patched code certainly makes more sense than what was there
previously, and does seem to do the right things now.   Thanks!





-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: main_c_diff.txt
URL: <http://lists.sip-router.org/pipermail/sr-users/attachments/20090319/db85ee8b/attachment.txt>