[sr-dev] Remote-Party-ID Parsed Header Crash

[Daniel-Constantin Mierla]

If you want a solution to avoid the issue via config: in the
request_route, store the RPID in an avp and update the acc parameter to
use the avp instead of the rpid variable.

As for code, a solution could be resetting the 'hdr->parsed' pointers to
NULL that are in private memory after memcpy(&tmsg, req,
sizeof(sip_msg_t)); -- using same kind of loop like at the end of
acc_onreply(), but wihout clean_hdr_field(). In this way it ensure that
the acc is using only what it parses and it is freeing only those headers.

Cheers,
Daniel


On 14/01/2017 22:59, Joshua Colp wrote:
> On Sat, Jan 14, 2017, at 04:09 PM, Joshua Colp wrote:
>> On Sat, Jan 14, 2017, at 02:43 PM, Joshua Colp wrote:
>>
>>> Testing is in progress and so far so good. I do think the code in
>>> acc_onreply that cleans up the parsed header is not correct, though.
>>> It's referencing the shared memory memory instead of the locally scoped
>>> one where any parsed headers (should) live.
>> 10,000 calls and zero problems. Going to push it further. I did try
>> implementing the change to free the alloced headers on preq and it went
>> very very poorly. Something still doesn't feel right with this over all,
>> even with the fix.
> Another crash occurred when freeing the Remote-Party-ID parsed value:
>
> (gdb) bt
> #0  0x000000000055adf6 in parse_addr_spec (buffer=0x7fff0d5d04a0
> "\365?", end=0x7fff0d5d0410 "`8J\245\360\177", to_b=0x7fff0d5d04a0,
> allow_comma_sep=32767)
>     at parser/parse_addr_spec.c:912
> #1  0x000000000055ae62 in free_to_params (tb=0x55ae62) at
> parser/parse_addr_spec.c:921
> #2  0x000000000054fde2 in clean_hdr_field (hf=0x7ff08e7e6248) at
> parser/hf.c:142
> #3  0x00007ff0a5274ec5 in acc_onreply (t=0x7ff08e902830,
> req=0x7ff08e7e5230, reply=0x7ff0ac9f01a8, code=200) at acc_logic.c:511
> #4  0x00007ff0a52753ec in tmcb_func (t=0x7ff08e902830, type=512,
> ps=0x7fff0d5d0d40) at acc_logic.c:583
> #5  0x00007ff0a746f478 in run_trans_callbacks_internal
> (cb_lst=0x7ff08e9028a0, type=512, trans=0x7ff08e902830,
> params=0x7fff0d5d0d40) at t_hooks.c:290
> #6  0x00007ff0a746f68a in run_trans_callbacks_with_buf (type=512,
> rbuf=0x7ff08e9028f0, req=0x7ff08e7e5230, repl=0x7ff0ac9f01a8, flags=200)
> at t_hooks.c:336
> #7  0x00007ff0a74a1c06 in relay_reply (t=0x7ff08e902830,
> p_msg=0x7ff0ac9f01a8, branch=0, msg_status=200,
> cancel_data=0x7fff0d5d10a0, do_put_on_wait=1)
>     at t_reply.c:2001
> #8  0x00007ff0a74a40b7 in reply_received (p_msg=0x7ff0ac9f01a8) at
> t_reply.c:2499
> #9  0x000000000045d837 in do_forward_reply (msg=0x7ff0ac9f01a8, mode=0)
> at forward.c:777
> #10 0x000000000045e0f8 in forward_reply (msg=0x7ff0ac9f01a8) at
> forward.c:860
> #11 0x00000000004a5887 in receive_msg (
>     buf=0x9245e0 "SIP/2.0 200 OK\r\nVia: SIP/2.0/UDP
>     128.177.41.40:5060;branch=z9hG4bK16.def8e32b85c1f9a00abfd00f56e513ff.0,
>     SIP/2.0/UDP
>     128.177.41.62:5061;rport=5061;branch=z9hG4bK-13531-8230-0\r\nFrom:
>     sipp <sip:sipp at 128"..., len=603, rcv_info=0x7fff0d5d1420) at
>     receive.c:273
> #12 0x000000000053c838 in udp_rcv_loop () at udp_server.c:531
> #13 0x000000000046d42b in main_loop () at main.c:1617
> #14 0x00000000004704d3 in main (argc=11, argv=0x7fff0d5d1758) at
> main.c:2533
>
> (gdb) print *hf
> $2 = {type = HDR_RPID_T, name = {
>     s = 0x7ff08e7e5a54 "Remote-Party-ID: \"XXXXXXXXXX\"
>     <sip:XXXXXXXXXX at XX.XXX.XXX.XXX>;party=calling;privacy=off;screen=no\r\nContact:
>     sip:sipp at XXX.XXX.XX.XX:5061\r\nMax-Forwards: 69\r\nSubject:
>     Performance Test\r\nContent-Type: appl"..., len = 15}, body = {
>     s = 0x7ff08e7e5a65 "\"XXXXXXXXXX\"
>     <sip:XXXXXXXXXX at XX.XXX.XXX.XXX>;party=calling;privacy=off;screen=no\r\nContact:
>     sip:sipp at XXX.XXX.XX.XX:5061\r\nMax-Forwards: 69\r\nSubject:
>     Performance Test\r\nContent-Type: application/sdp\r\nCont"..., len =
>     80}, len = 99, parsed = 0x0, next = 0x7ff08e7e6288}
>
> So it does indeed appear as though the change is incomplete. I'm going
> to see if I can figure out an additional fix, and if there's any
> additional information I can provide I'll try my best.
>

-- 
Daniel-Constantin Mierla
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio World Conference - May 8-10, 2017 - www.kamailioworld.com