Subject: Re: [PATCH] avformat/rtpdec: fix side_data leak when a packet is
rejected
ASan verification detail (./configure --toolchain=gcc-asan; minimal RTSP
source injecting H.264 NAL type 30 followed by a valid NAL, x400):
Without the patch:
SUMMARY: AddressSanitizer: 89600 byte(s) leaked in 1200 allocation(s)
Direct leak 19200 B / 400 obj: av_packet_new_side_data
-> rtp_set_prft (rtpdec.c:600)
-> finalize_packet (rtpdec.c:645)
-> rtp_parse_packet_internal (rtpdec.c:757)
-> ff_rtp_parse_packet (rtpdec.c:932)
-> ff_rtsp_fetch_packet (rtsp.c:2465)
Indirect leak 38400 B / 400 obj: rtp_add_sr_sidedata (rtpdec.c:617)
-> finalize_packet (rtpdec.c:634) [AV_PKT_DATA_RTCP_SR]
Indirect leak 32000 B / 400 obj: rtp_set_prft PRFT payload
(rtpdec.c:600)
With the patch: identical input -> LeakSanitizer reports zero leaks.
zengweiyu <[email protected]> 于2026年7月27日周一 16:34写道：
> From: 曾维宇 <[email protected]>
>
> rtp_parse_packet_internal() calls finalize_packet() unconditionally right
> after the depacketizer, even when the latter returned an error.
> finalize_packet() attaches side data to the packet - AV_PKT_DATA_PRFT via
> rtp_set_prft() and AV_PKT_DATA_RTCP_SR via rtp_add_sr_sidedata().
>
> The rejected packet is never delivered, but the same AVPacket is reused in
> two places without being unreferenced first:
>
> * the queue-drain loop in ff_rtp_parse_packet():
> while (rv < 0 && has_next_packet(s))
> rv = rtp_parse_queued_packet(s, pkt);
> * the RTSP "redo" loop in ff_rtsp_fetch_packet(), which parses the next
> RTP packet into the same AVPacket on error.
>
> When the next (successful) depacketizer then calls av_new_packet() on that
> reused packet, get_packet_defaults() runs memset(pkt, 0, sizeof(*pkt)),
> which zeroes pkt->side_data (and side_data_elems) without freeing it. The
> side data attached by the previous, failed finalize_packet() call is
> therefore leaked - one allocation per bad->good transition.
>
> For H.264 this is reachable whenever a malformed NAL unit (undefined type,
> STAP-B/MTAP/FU-B, truncated FU-A, oversized STAP-A NAL, or empty payload)
> arrives after the first RTCP SR, followed by a valid packet.
>
> Fix it by unreferencing the packet on the error path, before
> finalize_packet()
> runs, so the caller always observes a clean packet regardless of what the
> depacketizer left behind.
>
> Verified with an ASan build (--toolchain=gcc-asan) against a minimal RTSP
> source injecting H.264 NAL type 30 followed by a valid NAL: 89600 bytes in
> 1200 allocations leaked without the patch, zero with it.
>
> Signed-off-by: 曾维宇 <[email protected]>
> ---
> libavformat/rtpdec.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> index 5872c0f59c..c0736b5619 100644
> --- a/libavformat/rtpdec.c
> +++ b/libavformat/rtpdec.c
> @@ -753,6 +753,18 @@ static int rtp_parse_packet_internal(RTPDemuxContext
> *s, AVPacket *pkt,
> return AVERROR(EINVAL);
> }
>
> + if (rv < 0) {
> + /* The depacketizer rejected this packet; it will not be
> delivered.
> + * Unreference whatever it may already have written, and prevent
> + * finalize_packet() from attaching side data to it, so nothing is
> + * leaked when the packet is reused. Both the RTSP "redo" path and
> + * the queue-drain loop in ff_rtp_parse_packet() reuse this
> AVPacket,
> + * and a later av_new_packet() would otherwise memset() the
> + * side_data pointer away without freeing it. */
> + av_packet_unref(pkt);
> + return rv;
> + }
> +
> // now perform timestamp things....
> finalize_packet(s, pkt, timestamp);
>
> --
> 2.42.0.windows.2
>
>
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
