From fdac3d72ab5629ee075c5778491aa540bf32ab73 Mon Sep 17 00:00:00 2001 From: kasperiio <101901857+kasperiio@users.noreply.github.com> Date: Sat, 5 Sep 2026 19:51:56 +0300 Subject: [PATCH 1/2] fix a stale channel state, truncated audio settings and an RTCP send to fd 0 Four independent defects, found while bringing up a new platform but none of them platform-specific. media: a failed channel, encoder or bind left chnState[index].enable true, so the video thread kept polling a channel that was never created. server: audio bitrate, gain and srate were parsed into a short. 48000 does not fit, so setting a 48 kHz sample rate over the API silently stored -17536. server: a POST with no Content-Type dereferenced NULL in STARTS_WITH. rtcp: __rtcp_send_sr() sent on con->trans[track_id].server_port_rtp without checking the track was ever SETUP. An audio track a client never set up has fd 0, so the report went to stdin. --- src/media.c | 18 ++++++++++++++++++ src/rtsp/rtcp.h | 3 +++ src/server.c | 8 ++++---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/media.c b/src/media.c index f3d6f58f..8fde6cf9 100644 --- a/src/media.c +++ b/src/media.c @@ -548,8 +548,11 @@ int media_mjpeg_enable(void) { if (ret = create_channel(index, app_config.mjpeg_width, app_config.mjpeg_height, app_config.mjpeg_fps, 1)) + { + chnState[index].enable = false; HAL_ERROR("media", "Creating channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); + } { hal_vidconfig config; @@ -582,13 +585,19 @@ int media_mjpeg_enable(void) { } if (ret) + { + chnState[index].enable = false; HAL_ERROR("media", "Creating encoder %d failed with %#x!\n%s\n", index, ret, errstr(ret)); + } } if (ret = bind_channel(index, app_config.mjpeg_fps, 1)) + { + chnState[index].enable = false; HAL_ERROR("media", "Binding channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); + } return EXIT_SUCCESS; } @@ -620,8 +629,11 @@ int media_mp4_enable(void) { if (ret = create_channel(index, app_config.mp4_width, app_config.mp4_height, app_config.mp4_fps, 0)) + { + chnState[index].enable = false; HAL_ERROR("media", "Creating channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); + } { hal_vidconfig config; @@ -657,8 +669,11 @@ int media_mp4_enable(void) { } if (ret) + { + chnState[index].enable = false; HAL_ERROR("media", "Creating encoder %d failed with %#x!\n%s\n", index, ret, errstr(ret)); + } mp4_set_config(app_config.mp4_width, app_config.mp4_height, app_config.mp4_fps, app_config.audio_enable ? HAL_AUDCODEC_MP3 : HAL_AUDCODEC_UNSPEC, @@ -666,8 +681,11 @@ int media_mp4_enable(void) { } if (ret = bind_channel(index, app_config.mp4_fps, 0)) + { + chnState[index].enable = false; HAL_ERROR("media", "Binding channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); + } return EXIT_SUCCESS; } diff --git a/src/rtsp/rtcp.h b/src/rtsp/rtcp.h index 1e012802..ee11f64f 100644 --- a/src/rtsp/rtcp.h +++ b/src/rtsp/rtcp.h @@ -20,6 +20,9 @@ static inline int __rtcp_send_sr(struct connection_item_t *con, int track_id); ******************************************************************************/ static inline int __rtcp_send_sr(struct connection_item_t *con, int track_id) { + /* A track without a transport (never SETUP) has fd 0, which is stdin */ + if (!con->trans[track_id].server_port_rtp && !con->trans[track_id].is_tcp) return SUCCESS; + struct timeval tv; unsigned int ts_h, ts_l; int send_bytes; diff --git a/src/server.c b/src/server.c index 16340140..18ac507b 100644 --- a/src/server.c +++ b/src/server.c @@ -873,7 +873,7 @@ void respond_request(http_request_t *req) { char *key = split(&value, "="); if (!key || !*key || !value || !*value) continue; if (EQUALS(key, "bitrate")) { - short result = strtol(value, &remain, 10); + int result = strtol(value, &remain, 10); if (remain != value) app_config.audio_bitrate = result; } else if (EQUALS(key, "enable")) { @@ -882,11 +882,11 @@ void respond_request(http_request_t *req) { else if (EQUALS_CASE(value, "false") || EQUALS(value, "0")) app_config.audio_enable = 0; } else if (EQUALS(key, "gain")) { - short result = strtol(value, &remain, 10); + int result = strtol(value, &remain, 10); if (remain != value) app_config.audio_gain = result; } else if (EQUALS(key, "srate")) { - short result = strtol(value, &remain, 10); + int result = strtol(value, &remain, 10); if (remain != value) app_config.audio_srate = result; } @@ -1265,7 +1265,7 @@ void respond_request(http_request_t *req) { } if (EQUALS(req->method, "POST")) { char *type = request_header("Content-Type"); - if (STARTS_WITH(type, "multipart/form-data")) { + if (type && STARTS_WITH(type, "multipart/form-data")) { char *bound = strstr(type, "boundary=") + strlen("boundary="); char *payloadb = strstr(req->payload, bound); From fde38d4928f0c5673a80894735a250796f09df33 Mon Sep 17 00:00:00 2001 From: kasperiio <101901857+kasperiio@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:20:07 +0300 Subject: [PATCH 2/2] media, rtcp, server: address review of the previous commit media: clearing chnState[index].enable before HAL_ERROR returned made things worse, not better. media_mjpeg_disable() and media_mp4_disable() skip slots whose enable is already clear, so a failure after the channel or encoder had been created stranded them in the HAL, and take_next_free_channel() would then hand the same index out again. Roll back stage by stage instead: release the encoder and channel that were created, then clear the slot. rtcp: the new transport guard indexed con->trans[track_id] ahead of the bounds assertion that follows it, so an out-of-range track read out of bounds before the assertion could reject it. Moved below the assertion and expressed through the t pointer it already sets up. server: widening the audio settings from short to int stopped them being truncated but still stored whatever strtol() returned, including trailing garbage, saturated overflow and negatives - and audio_bitrate and audio_srate are unsigned, so a negative wrapped rather than being rejected. They are now accepted only as a complete number inside the same ranges the config loader enforces: bitrate 32-320, gain -60-30, srate 8000-96000. --- src/media.c | 18 ++++++++++++++++++ src/rtsp/rtcp.h | 7 ++++--- src/server.c | 36 ++++++++++++++++++++++++++---------- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/media.c b/src/media.c index 8fde6cf9..53e72abf 100644 --- a/src/media.c +++ b/src/media.c @@ -549,6 +549,7 @@ int media_mjpeg_enable(void) { if (ret = create_channel(index, app_config.mjpeg_width, app_config.mjpeg_height, app_config.mjpeg_fps, 1)) { + /* nothing was created yet, so only the slot needs releasing */ chnState[index].enable = false; HAL_ERROR("media", "Creating channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); @@ -586,6 +587,11 @@ int media_mjpeg_enable(void) { if (ret) { + /* The channel was created; release it before clearing the slot. + * media_*_disable() skips slots whose enable is already clear, so + * dropping the flag first would strand the channel in the HAL and + * still let take_next_free_channel() hand the index out again. */ + media_video_disable(index, 1); chnState[index].enable = false; HAL_ERROR("media", "Creating encoder %d failed with %#x!\n%s\n", index, ret, errstr(ret)); @@ -594,6 +600,9 @@ int media_mjpeg_enable(void) { if (ret = bind_channel(index, app_config.mjpeg_fps, 1)) { + /* channel and encoder exist and nothing is bound, so drop the encoder + * (the teardown order in media_*_disable) before releasing the slot */ + media_video_disable(index, 1); chnState[index].enable = false; HAL_ERROR("media", "Binding channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); @@ -630,6 +639,7 @@ int media_mp4_enable(void) { if (ret = create_channel(index, app_config.mp4_width, app_config.mp4_height, app_config.mp4_fps, 0)) { + /* nothing was created yet, so only the slot needs releasing */ chnState[index].enable = false; HAL_ERROR("media", "Creating channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); @@ -670,6 +680,11 @@ int media_mp4_enable(void) { if (ret) { + /* The channel was created; release it before clearing the slot. + * media_*_disable() skips slots whose enable is already clear, so + * dropping the flag first would strand the channel in the HAL and + * still let take_next_free_channel() hand the index out again. */ + media_video_disable(index, 0); chnState[index].enable = false; HAL_ERROR("media", "Creating encoder %d failed with %#x!\n%s\n", index, ret, errstr(ret)); @@ -682,6 +697,9 @@ int media_mp4_enable(void) { if (ret = bind_channel(index, app_config.mp4_fps, 0)) { + /* channel and encoder exist and nothing is bound, so drop the encoder + * (the teardown order in media_*_disable) before releasing the slot */ + media_video_disable(index, 0); chnState[index].enable = false; HAL_ERROR("media", "Binding channel %d failed with %#x!\n%s\n", index, ret, errstr(ret)); diff --git a/src/rtsp/rtcp.h b/src/rtsp/rtcp.h index ee11f64f..871895c5 100644 --- a/src/rtsp/rtcp.h +++ b/src/rtsp/rtcp.h @@ -20,9 +20,6 @@ static inline int __rtcp_send_sr(struct connection_item_t *con, int track_id); ******************************************************************************/ static inline int __rtcp_send_sr(struct connection_item_t *con, int track_id) { - /* A track without a transport (never SETUP) has fd 0, which is stdin */ - if (!con->trans[track_id].server_port_rtp && !con->trans[track_id].is_tcp) return SUCCESS; - struct timeval tv; unsigned int ts_h, ts_l; int send_bytes; @@ -34,6 +31,10 @@ static inline int __rtcp_send_sr(struct connection_item_t *con, int track_id) return FAILURE); t = &con->trans[track_id]; + /* A track that was never SETUP has no transport, and its fd is 0 - stdin. + * Checked after the bounds assertion so an invalid id cannot index first. */ + if (!t->server_port_rtp && !t->is_tcp) return SUCCESS; + ASSERT(gettimeofday(&tv,NULL) == 0, return FAILURE); ts_h = (unsigned int)tv.tv_sec + 2208988800U; diff --git a/src/server.c b/src/server.c index 18ac507b..3c0b985d 100644 --- a/src/server.c +++ b/src/server.c @@ -1,3 +1,4 @@ +#include #include "server.h" #define HTTP_MAX_CLIENTS 50 @@ -218,6 +219,22 @@ void send_h26x_to_client(char index, hal_vidstream *stream) { } } +/* Accept a query value only when it is a complete number inside the range the + * config loader enforces. audio_bitrate and audio_srate are unsigned, so a + * negative value would wrap rather than be rejected, and strtol() alone + * accepts trailing text and saturates on overflow. */ +static int query_ranged(const char *value, long min, long max, long *out) { + char *remain; + long result; + errno = 0; + result = strtol(value, &remain, 10); + if (remain == value || (remain && *remain) || errno == ERANGE || + result < min || result > max) + return 0; + *out = result; + return 1; +} + void send_mp4_to_client(char index, hal_vidstream *stream, char isH265) { for (unsigned int i = 0; i < stream->count; ++i) { hal_vidpack *pack = &stream->pack[i]; @@ -865,7 +882,6 @@ void respond_request(http_request_t *req) { if (EQUALS(req->uri, "/api/audio")) { if (req->query) { - char *remain; while (req->query) { char *value = split(&req->query, "&"); if (!value || !*value) continue; @@ -873,22 +889,22 @@ void respond_request(http_request_t *req) { char *key = split(&value, "="); if (!key || !*key || !value || !*value) continue; if (EQUALS(key, "bitrate")) { - int result = strtol(value, &remain, 10); - if (remain != value) - app_config.audio_bitrate = result; + long result; + if (query_ranged(value, 32, 320, &result)) + app_config.audio_bitrate = (unsigned int)result; } else if (EQUALS(key, "enable")) { if (EQUALS_CASE(value, "true") || EQUALS(value, "1")) app_config.audio_enable = 1; else if (EQUALS_CASE(value, "false") || EQUALS(value, "0")) app_config.audio_enable = 0; } else if (EQUALS(key, "gain")) { - int result = strtol(value, &remain, 10); - if (remain != value) - app_config.audio_gain = result; + long result; + if (query_ranged(value, -60, 30, &result)) + app_config.audio_gain = (int)result; } else if (EQUALS(key, "srate")) { - int result = strtol(value, &remain, 10); - if (remain != value) - app_config.audio_srate = result; + long result; + if (query_ranged(value, 8000, 96000, &result)) + app_config.audio_srate = (unsigned int)result; } }