rtp_relay: give the caller its own copy of the route engine's body

[Lt-Flash]

Summary

Fix a crash in the rtp_relay route engine: the body produced by the script route was handed to the caller without a copy, while the caller takes ownership of it and frees it. Reported as #4255.

Details

rtp_relay_route_fill_body() returned the script's return value by assignment:

if (body)
    *body = val.rs;

but script_return_set() (route.c) stores that value in a single pkg allocation with an inline buffer:

v = pkg_malloc(sizeof(*v) + val.rs.len);
v->val = val;
v->val.rs.s = v->buf;

struct script_return_value is pv_value_t val (24) + next (8) + char buf[0], so val.rs.s points 32 bytes into a block the core owns and releases itself in script_return_free().

The callers of the offer/answer engine hooks do take ownership of the body they get back:

• rtp_relay_reinvite() calls pkg_free(body->s) when release_body is set - this is the rtp_relay:update MI path;
• the reply path passes it to replace_lump_rpl(..., LUMP_RPL_NODUP), and free_lump_rpl() frees a NODUP text.

The rtpengine engine already honours that contract: it fills the body from bencode_dictionary_get_str_dup(), a real allocation base. The route engine did not, so the free ran on an interior pointer - f_malloc reads the fragment header from p - sizeof(struct fm_frag), which lands inside the allocation's own header, and then follows it. With F_MALLOC that is a SIGSEGV in fm_remove_free(), with Q_MALLOC an abort ("fragm ... beginning overwritten").

check_double_free() cannot catch this one: frag_seems_valid() only tests that f->pf lies inside the block, and here f->pf reads val.rs.s, which is the pointer being freed, so it is always in range.

Solution

Duplicate the returned value into the caller's str with pkg_str_dup(), so what the caller frees is a real allocation base - the same contract the rtpengine engine already implements.

Also drop the second rtp_relay_replace_body(sess->msg, body) in rtp_relay_route_offer(): rtp_relay_route_fill_body() has already replaced the message body using a copy of its own, and del_lump() does not detect the overlapping deletion, so now that the caller owns the returned buffer, passing it to a lump as well would make the message free it a second time.

Verified on master (0a33489dc5), two prefixes from the same revision differing only by this patch. 20 calls established with rtp_relay_engage("route") and held, then one rtp_relay:update:

build	-a F_MALLOC	-a Q_MALLOC
unpatched	SIGSEGV in fm_remove_free()	abort, "beginning overwritten"
patched	survives, 20/20 sessions	survives, 20/20 sessions

In the patched runs the offer route ran 40 times (20 initial engagements + 20 re-INVITE offers), so the freeing path was exercised once per context.

Compatibility

No script or configuration change, and no change to the engine hook contract - this only brings the route engine in line with what the rtpengine engine already does.

One pre-existing problem this does not touch: media_exchange_get_offer_sdp() overwrites the returned body on its success path without releasing it, so it leaks with any engine that returns an owned buffer (rtpengine today, route after this change). That belongs to media_exchange and is left alone here to keep this fix minimal.

Closing issues

closes #4255