PR #25476 opened by Kilian (lusoris) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25476 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25476.patch
These patches address GCC 14.4.0 and 16.2.1 -Wformat-truncation in libavformat/rtsp.c, -Wimplicit-fallthrough in libavformat/nspdec.c and -Wstack-usage against -Wstack-usage=122880 in libavformat/mpegts.c. What the patch does: - `avformat/rtsp: check setup header construction and send errors`: builds transport and command headers with fixed-buffer AVBPrint, enlarges the command stack buffer, rejects incomplete headers and checks send errors before reading replies. - `avformat/nspdec: mark intentional unsupported-chunk fallthrough`: annotates the existing transition to the mono-channel cases with av_fallthrough; no functional change. - `avformat/mpegts: reduce automatic program storage`: saves only the stream count and stream array and swaps program records in 256-byte chunks; stream matching and program ordering for valid tables are unchanged. Behaviour change: incomplete RTSP headers and setup send failures return errors; successful requests with previously fitting headers are unchanged. Credit to #20857 for independently fixing the same transport-header truncation by reducing the transport buffer to 2048 bytes. The RTSP commit can be dropped in its favour. Base: FFmpeg master 46f23be8b1. Testing: - GCC 14.4.0 (`--enable-gpl --enable-vulkan`) and GCC 16.2.1 (`--enable-gpl --enable-vulkan --enable-vaapi --enable-libdrm --enable-nvenc`), targets all, alltools, examples, testprogs and checkasm: master 46f23be8b1 shows 1 -Wformat-truncation warning in libavformat/rtsp.c; 1 -Wimplicit-fallthrough warning in libavformat/nspdec.c; 2 -Wstack-usage warnings in libavformat/mpegts.c with each compiler; with these patches there are none and no new ones. - FATE with GCC 16.2.1, tests whose name contains `mpegts` or `rtsp`, and `fate-ts-*` (34 tests): the same results on master and with these patches; 9 fail on both sides only because their samples are missing from my local FATE sample copy, all others pass. - FATE has no test for the nsp demuxer. Split out of #25111 so that each part reaches the reviewers for its area. >From cf7de617c92f8c0ed67aedeab0cb4c992aa28a7b Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Thu, 15 Oct 2026 22:06:37 +0200 Subject: [PATCH 1/3] avformat/rtsp: check setup header construction and send errors GCC 14.4.0 and 16.2.1 report -Wformat-truncation when formatting the transport header. Truncation can leave incomplete setup headers. Build transport and command headers with fixed-buffer AVBPrint, enlarge the command buffer and reject incomplete results. Check the setup send result before reading the reply. Successful requests whose headers previously fit retain the same header text. Signed-off-by: lusoris <[email protected]> --- libavformat/rtsp.c | 67 +++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 29f219c366a..232e5b513e0 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1631,7 +1631,7 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port, int rtx = 0, j, i, err, interleave = 0, port_off = 0; RTSPStream *rtsp_st; RTSPMessageHeader reply1, *reply = &reply1; - char cmd[MAX_URL_SIZE]; + char cmd[2 * MAX_URL_SIZE]; const char *trans_pref; memset(&reply1, 0, sizeof(reply1)); @@ -1657,6 +1657,9 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port, for (j = rt->rtp_port_min + port_off, i = 0; i < rt->nb_rtsp_streams; ++i) { char transport[MAX_URL_SIZE]; + AVBPrint transport_bp; + + av_bprint_init_for_buffer(&transport_bp, transport, sizeof(transport)); /* * WMS serves all UDP data over a single connection, the RTX, which @@ -1714,17 +1717,14 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port, rtp_opened: port = ff_rtp_get_local_rtp_port(rtsp_st->rtp_handle); have_port: - av_strlcpy(transport, trans_pref, sizeof(transport)); - av_strlcat(transport, - rt->server_type == RTSP_SERVER_SATIP ? ";" : "/UDP;", - sizeof(transport)); + av_bprintf(&transport_bp, "%s%s", trans_pref, + rt->server_type == RTSP_SERVER_SATIP ? ";" : "/UDP;"); if (rt->server_type != RTSP_SERVER_REAL) - av_strlcat(transport, "unicast;", sizeof(transport)); - av_strlcatf(transport, sizeof(transport), - "client_port=%d", port); + av_bprintf(&transport_bp, "unicast;"); + av_bprintf(&transport_bp, "client_port=%d", port); if (rt->transport == RTSP_TRANSPORT_RTP && !(rt->server_type == RTSP_SERVER_WMS && i > 0)) - av_strlcatf(transport, sizeof(transport), "-%d", port + 1); + av_bprintf(&transport_bp, "-%d", port + 1); } /* RTP/TCP */ @@ -1737,44 +1737,55 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port, s->streams[rtsp_st->stream_index]->codecpar->codec_type == AVMEDIA_TYPE_DATA)) continue; - snprintf(transport, sizeof(transport) - 1, - "%s/TCP;", trans_pref); + av_bprintf(&transport_bp, "%s/TCP;", trans_pref); if (rt->transport != RTSP_TRANSPORT_RDT) - av_strlcat(transport, "unicast;", sizeof(transport)); - av_strlcatf(transport, sizeof(transport), - "interleaved=%d-%d", - interleave, interleave + 1); + av_bprintf(&transport_bp, "unicast;"); + av_bprintf(&transport_bp, "interleaved=%d-%d", + interleave, interleave + 1); interleave += 2; } else if (lower_transport == RTSP_LOWER_TRANSPORT_UDP_MULTICAST) { - snprintf(transport, sizeof(transport) - 1, - "%s/UDP;multicast", trans_pref); + av_bprintf(&transport_bp, "%s/UDP;multicast", trans_pref); } else { err = AVERROR(EINVAL); goto fail; // transport would be uninitialized } if (s->oformat) { - av_strlcat(transport, ";mode=record", sizeof(transport)); + av_bprintf(&transport_bp, ";mode=record"); } else if (rt->server_type == RTSP_SERVER_REAL || rt->server_type == RTSP_SERVER_WMS) - av_strlcat(transport, ";mode=play", sizeof(transport)); - snprintf(cmd, sizeof(cmd), - "Transport: %s\r\n", - transport); + av_bprintf(&transport_bp, ";mode=play"); + if (!av_bprint_is_complete(&transport_bp)) { + av_log(s, AV_LOG_ERROR, "RTSP transport header is too long\n"); + err = AVERROR(ENAMETOOLONG); + goto fail; + } + + AVBPrint cmd_bp; + av_bprint_init_for_buffer(&cmd_bp, cmd, sizeof(cmd)); + av_bprintf(&cmd_bp, "Transport: %s\r\n", transport); if (rt->accept_dynamic_rate) - av_strlcat(cmd, "x-Dynamic-Rate: 0\r\n", sizeof(cmd)); + av_bprintf(&cmd_bp, "x-Dynamic-Rate: 0\r\n"); if (CONFIG_RTPDEC && i == 0 && rt->server_type == RTSP_SERVER_REAL) { char real_res[41], real_csum[9]; ff_rdt_calc_response_and_checksum(real_res, real_csum, real_challenge); - av_strlcatf(cmd, sizeof(cmd), - "If-Match: %s\r\n" - "RealChallenge2: %s, sd=%s\r\n", - rt->session_id, real_res, real_csum); + av_bprintf(&cmd_bp, + "If-Match: %s\r\n" + "RealChallenge2: %s, sd=%s\r\n", + rt->session_id, real_res, real_csum); } - ff_rtsp_send_cmd(s, "SETUP", rtsp_st->control_url, cmd, reply, NULL); + if (!av_bprint_is_complete(&cmd_bp)) { + av_log(s, AV_LOG_ERROR, "RTSP SETUP header is too long\n"); + err = AVERROR(ENAMETOOLONG); + goto fail; + } + err = ff_rtsp_send_cmd(s, "SETUP", rtsp_st->control_url, cmd, reply, + NULL); + if (err < 0) + goto fail; if (reply->status_code == 461 /* Unsupported protocol */ && i == 0) { err = 1; goto fail; -- 2.54.0 >From cb27bf37364f488afda79c88e5af3880a0747563 Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Thu, 15 Oct 2026 22:06:37 +0200 Subject: [PATCH 2/3] avformat/nspdec: mark intentional unsupported-chunk fallthrough GCC 14.4.0 and 16.2.1 report -Wimplicit-fallthrough after the unsupported-chunk warning. The switch deliberately continues into the mono-channel cases. Include the attributes header and annotate this transition with av_fallthrough. The warning and channel selection remain unchanged for the same input. Signed-off-by: lusoris <[email protected]> --- libavformat/nspdec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavformat/nspdec.c b/libavformat/nspdec.c index bb605426f66..fe830599671 100644 --- a/libavformat/nspdec.c +++ b/libavformat/nspdec.c @@ -19,6 +19,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ +#include "libavutil/attributes.h" #include "libavutil/avstring.h" #include "libavutil/intreadwrite.h" #include "avformat.h" @@ -78,6 +79,7 @@ static int nsp_read_header(AVFormatContext *s) case MKBETAG('S', 'D', '_', '7'): case MKBETAG('S', 'D', '_', '8'): av_log(s, AV_LOG_WARNING, "Unsupported chunk!\n"); + av_fallthrough; case MKBETAG('S', 'D', 'A', '_'): case MKBETAG('S', 'D', '_', 'A'): channels = 1; -- 2.54.0 >From 90e7b681a105ae3d5436cba0024c90aa6bda8125 Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Thu, 15 Oct 2026 22:06:37 +0200 Subject: [PATCH 3/3] avformat/mpegts: reduce automatic program storage GCC 14.4.0 and 16.2.1 report -Wstack-usage against FFmpeg's default -Wstack-usage=122880 limit. Copying and swapping whole program records requires large automatic temporaries. Retain only the stream count and stream array for matching after a program map update, and swap program records in 256-byte chunks. Stream matching and program ordering for valid tables are unchanged. Signed-off-by: lusoris <[email protected]> --- libavformat/mpegts.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c index 8c7df85173d..cae0e45d488 100644 --- a/libavformat/mpegts.c +++ b/libavformat/mpegts.c @@ -142,6 +142,11 @@ struct Program { int pmt_found; }; +struct ProgramSnapshot { + unsigned int nb_streams; + struct Stream streams[MAX_STREAMS_PER_PROGRAM]; +}; + struct MpegTSContext { const AVClass *class; /* user data */ @@ -332,6 +337,20 @@ static void clear_program(struct Program *p) p->pmt_found = 0; } +static void swap_programs(struct Program *a, struct Program *b) +{ + uint8_t tmp[256]; + size_t offset = 0; + + while (offset < sizeof(*a)) { + const size_t size = FFMIN(sizeof(tmp), sizeof(*a) - offset); + memcpy(tmp, (uint8_t *)a + offset, size); + memcpy((uint8_t *)a + offset, (uint8_t *)b + offset, size); + memcpy((uint8_t *)b + offset, tmp, size); + offset += size; + } +} + static void clear_programs(MpegTSContext *ts) { av_freep(&ts->prg); @@ -2545,7 +2564,8 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type } static AVStream *find_matching_stream(MpegTSContext *ts, int pid, unsigned int programid, - int stream_identifier, int pmt_stream_idx, struct Program *p) + int stream_identifier, int pmt_stream_idx, + const struct ProgramSnapshot *p) { AVFormatContext *s = ts->stream; AVStream *found = NULL; @@ -2741,7 +2761,7 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len { MpegTSContext *ts = filter->u.section_filter.opaque; MpegTSSectionFilter *tssf = &filter->u.section_filter; - struct Program old_program; + struct ProgramSnapshot old_program = { 0 }; SectionHeader h1, *h = &h1; PESContext *pes; AVStream *st; @@ -2777,15 +2797,15 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len return; prg = get_program(ts, h->id); - if (prg) - old_program = *prg; - else - clear_program(&old_program); - if (ts->skip_unknown_pmt && !prg) return; if (prg && prg->nb_pids && prg->pids[0] != ts->current_pid) return; + + if (prg) { + old_program.nb_streams = prg->nb_streams; + memcpy(old_program.streams, prg->streams, sizeof(old_program.streams)); + } if (!ts->skip_clear) clear_avprogram(ts, h->id); clear_program(prg); @@ -3033,7 +3053,7 @@ static void pat_cb(MpegTSFilter *filter, const uint8_t *section, int section_len clear_program(prg); add_pid_to_program(prg, pmt_pid); if (prg_idx > nb_prg) - FFSWAP(struct Program, ts->prg[nb_prg], ts->prg[prg_idx]); + swap_programs(&ts->prg[nb_prg], &ts->prg[prg_idx]); if (prg_idx >= nb_prg) nb_prg++; } else -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
