From 21859dc53e11f56e76a5db0f105ee65a20d09ab9 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Sun, 6 Aug 2017 14:21:15 +0200 Subject: [PATCH 1/1] vss: Propagate return value of afh_get_chunk(). With dynamic chunks, afh_get_chunk() may fail. Currently vss_get_chunk() prints an error message in this case and returns the null pointer. However, some callers of vss_get_chunk() happily dereference the returned pointer without checking for NULL. This patch modifies vss_get_chunk() to return int and teaches all callers to check the return value. For the udp and dccp transport, we disable the fec client temporarily in the error case while for the http transport we log the error (but otherwise ignore it), and try to continue with the next chunk. This flaw was noticed by the clang static analyzer. --- vss.c | 54 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/vss.c b/vss.c index cf51f43f..6aac9d59 100644 --- a/vss.c +++ b/vss.c @@ -345,7 +345,7 @@ static int initialize_fec_client(struct fec_client *fc, struct vss_task *vsst) return 1; } -static void vss_get_chunk(int chunk_num, struct vss_task *vsst, +static int vss_get_chunk(int chunk_num, struct vss_task *vsst, char **buf, size_t *sz) { int ret; @@ -362,31 +362,32 @@ static void vss_get_chunk(int chunk_num, struct vss_task *vsst, assert(vsst->header_buf); *buf = vsst->header_buf; /* stripped header */ *sz = vsst->header_len; - return; + return 0; } ret = afh_get_chunk(chunk_num, &mmd->afd.afhi, mmd->afd.audio_format_id, vsst->map, vsst->mapsize, (const char **)buf, sz, &vsst->afh_context); if (ret < 0) { - PARA_WARNING_LOG("could not get chunk %d: %s\n", - chunk_num, para_strerror(-ret)); *buf = NULL; *sz = 0; } + return ret; } -static void compute_group_size(struct vss_task *vsst, struct fec_group *g, +static int compute_group_size(struct vss_task *vsst, struct fec_group *g, int max_bytes) { char *buf; size_t len; - int i, max_chunks = PARA_MAX(1LU, 150 / tv2ms(vss_chunk_time())); + int ret, i, max_chunks = PARA_MAX(1LU, 150 / tv2ms(vss_chunk_time())); if (g->first_chunk == 0) { g->num_chunks = 1; - vss_get_chunk(0, vsst, &buf, &len); + ret = vss_get_chunk(0, vsst, &buf, &len); + if (ret < 0) + return ret; g->bytes = len; - return; + return 0; } g->num_chunks = 0; @@ -404,7 +405,9 @@ static void compute_group_size(struct vss_task *vsst, struct fec_group *g, break; if (chunk_num >= mmd->afd.afhi.chunks_total) /* eof */ break; - vss_get_chunk(chunk_num, vsst, &buf, &len); + ret = vss_get_chunk(chunk_num, vsst, &buf, &len); + if (ret < 0) + return ret; if (g->bytes + len > max_bytes) break; /* Include this chunk */ @@ -412,6 +415,7 @@ static void compute_group_size(struct vss_task *vsst, struct fec_group *g, g->num_chunks++; } assert(g->num_chunks); + return 1; } /* @@ -473,7 +477,9 @@ static int compute_slice_size(struct fec_client *fc, struct vss_task *vsst) if (!need_audio_header(fc, vsst)) { max_group_bytes = k * max_slice_bytes; g->num_header_slices = 0; - compute_group_size(vsst, g, max_group_bytes); + ret = compute_group_size(vsst, g, max_group_bytes); + if (ret < 0) + return ret; g->slice_bytes = DIV_ROUND_UP(g->bytes, k); if (g->slice_bytes == 0) g->slice_bytes = 1; @@ -489,7 +495,9 @@ static int compute_slice_size(struct fec_client *fc, struct vss_task *vsst) h = vsst->header_len; max_group_bytes = (k - num_slices(h, max_slice_bytes, n - k)) * max_slice_bytes; - compute_group_size(vsst, g, max_group_bytes); + ret = compute_group_size(vsst, g, max_group_bytes); + if (ret < 0) + return ret; d = g->bytes; if (d == 0) { g->slice_bytes = DIV_ROUND_UP(h, k); @@ -596,7 +604,9 @@ static int setup_next_fec_group(struct fec_client *fc, struct vss_task *vsst) * Setup data slices. Note that for ogg streams chunk 0 points to a * buffer on the heap rather than to the mapped audio file. */ - vss_get_chunk(g->first_chunk, vsst, &buf, &len); + ret = vss_get_chunk(g->first_chunk, vsst, &buf, &len); + if (ret < 0) + return ret; for (p = buf; i < g->num_header_slices + data_slices; i++) { if (p + g->slice_bytes > buf + g->bytes) { /* @@ -1020,7 +1030,7 @@ err: */ static void vss_send(struct vss_task *vsst) { - int i; + int i, ret; bool fec_active = false; struct timeval due; struct fec_client *fc, *tmp_fc; @@ -1063,16 +1073,22 @@ static void vss_send(struct vss_task *vsst) mmd->events++; set_mmd_offset(); } - vss_get_chunk(mmd->current_chunk, vsst, &buf, &len); - for (i = 0; senders[i].name; i++) { + ret = vss_get_chunk(mmd->current_chunk, vsst, &buf, &len); + if (ret < 0) { + PARA_ERROR_LOG("could not get chunk %lu: %s\n", + mmd->current_chunk, para_strerror(-ret)); + } else { /* * We call ->send() even if len is zero because senders * might have data queued which can be sent now. */ - if (!senders[i].send) - continue; - senders[i].send(mmd->current_chunk, mmd->chunks_sent, - buf, len, vsst->header_buf, vsst->header_len); + for (i = 0; senders[i].name; i++) { + if (!senders[i].send) + continue; + senders[i].send(mmd->current_chunk, + mmd->chunks_sent, buf, len, + vsst->header_buf, vsst->header_len); + } } mmd->chunks_sent++; mmd->current_chunk++; -- 2.30.2