Andreas Rheinhardt:
> Currently, adding a (separately allocated) element to a list of pointers
> works by first reallocating the array of pointers and (on success)
> incrementing its size and only then allocating the new element.
> If the latter allocation fails, the size is inconsistent, i.e.
> array[nb_array_elems - 1] is NULL. Our cleanup code crashes in such
> scenarios.
>
> Fix this by adding an auxiliary function that atomically allocates
> and adds a new element to a list of pointers.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
> ---
> fftools/cmdutils.c | 16 ++++++++++++++++
> fftools/cmdutils.h | 17 +++++++++++++++++
> fftools/ffmpeg_filter.c | 17 ++++-------------
> fftools/ffmpeg_opt.c | 22 ++++++----------------
> 4 files changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 45322f8c71..1464b122df 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -2214,6 +2214,22 @@ void *grow_array(void *array, int elem_size, int
> *size, int new_size)
> return array;
> }
>
> +void *allocate_array_elem(void *ptr, size_t elem_size, int *nb_elems)
> +{
> + void *new_elem, **array = (void**)ptr;
> +
> + if (*nb_elems == INT_MAX) {
> + av_log(NULL, AV_LOG_ERROR, "Array too big.\n");
> + exit_program(1);
> + }
> + new_elem = av_mallocz(elem_size);
> + if (!new_elem)
> + exit_program(1);
> + GROW_ARRAY(array, *nb_elems);
> + array[*nb_elems - 1] = new_elem;
> + return array;
> +}
> +
> double get_rotation(int32_t *displaymatrix)
> {
> double theta = 0;
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index 64dd7266bc..ae78e60f4c 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -628,11 +628,28 @@ FILE *get_preset_file(char *filename, size_t
> filename_size,
> */
> void *grow_array(void *array, int elem_size, int *size, int new_size);
>
> +/**
> + * Atomically add a new element to an array of pointers, i.e. allocate
> + * a new entry, reallocate the array of pointers and make the new last
> + * member of this array point to the newly allocated buffer.
> + * Calls exit() on failure.
> + *
> + * @param array array of pointers to reallocate
> + * @param elem_size size of the new element to allocate
> + * @param nb_elems pointer to the number of elements of the array array;
> + * *nb_elems will be incremented by one by this function.
> + * @return reallocated array
> + */
> +void *allocate_array_elem(void *array, size_t elem_size, int *nb_elems);
> +
> #define media_type_string av_get_media_type_string
>
> #define GROW_ARRAY(array, nb_elems)\
> array = grow_array(array, sizeof(*array), &nb_elems, nb_elems + 1)
>
> +#define ALLOC_ARRAY_ELEM(array, nb_elems)\
> + array = allocate_array_elem(array, sizeof(*array[0]), &nb_elems)
> +
> #define GET_PIX_FMT_NAME(pix_fmt)\
> const char *name = av_get_pix_fmt_name(pix_fmt);
>
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index dab0f28819..75194fe66e 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -164,18 +164,14 @@ int init_simple_filtergraph(InputStream *ist,
> OutputStream *ost)
> exit_program(1);
> fg->index = nb_filtergraphs;
>
> - GROW_ARRAY(fg->outputs, fg->nb_outputs);
> - if (!(fg->outputs[0] = av_mallocz(sizeof(*fg->outputs[0]))))
> - exit_program(1);
> + ALLOC_ARRAY_ELEM(fg->outputs, fg->nb_outputs);
> fg->outputs[0]->ost = ost;
> fg->outputs[0]->graph = fg;
> fg->outputs[0]->format = -1;
>
> ost->filter = fg->outputs[0];
>
> - GROW_ARRAY(fg->inputs, fg->nb_inputs);
> - if (!(fg->inputs[0] = av_mallocz(sizeof(*fg->inputs[0]))))
> - exit_program(1);
> + ALLOC_ARRAY_ELEM(fg->inputs, fg->nb_inputs);
> fg->inputs[0]->ist = ist;
> fg->inputs[0]->graph = fg;
> fg->inputs[0]->format = -1;
> @@ -280,9 +276,7 @@ static void init_input_filter(FilterGraph *fg,
> AVFilterInOut *in)
> ist->decoding_needed |= DECODING_FOR_FILTER;
> ist->st->discard = AVDISCARD_NONE;
>
> - GROW_ARRAY(fg->inputs, fg->nb_inputs);
> - if (!(fg->inputs[fg->nb_inputs - 1] =
> av_mallocz(sizeof(*fg->inputs[0]))))
> - exit_program(1);
> + ALLOC_ARRAY_ELEM(fg->inputs, fg->nb_inputs);
> fg->inputs[fg->nb_inputs - 1]->ist = ist;
> fg->inputs[fg->nb_inputs - 1]->graph = fg;
> fg->inputs[fg->nb_inputs - 1]->format = -1;
> @@ -318,10 +312,7 @@ int init_complex_filtergraph(FilterGraph *fg)
> init_input_filter(fg, cur);
>
> for (cur = outputs; cur;) {
> - GROW_ARRAY(fg->outputs, fg->nb_outputs);
> - fg->outputs[fg->nb_outputs - 1] =
> av_mallocz(sizeof(*fg->outputs[0]));
> - if (!fg->outputs[fg->nb_outputs - 1])
> - exit_program(1);
> + ALLOC_ARRAY_ELEM(fg->outputs, fg->nb_outputs);
>
> fg->outputs[fg->nb_outputs - 1]->graph = fg;
> fg->outputs[fg->nb_outputs - 1]->out_tmp = cur;
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index a27263b879..ffba7010e3 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1265,11 +1265,8 @@ static int open_input_file(OptionsContext *o, const
> char *filename)
> /* dump the file content */
> av_dump_format(ic, nb_input_files, filename, 0);
>
> - GROW_ARRAY(input_files, nb_input_files);
> - f = av_mallocz(sizeof(*f));
> - if (!f)
> - exit_program(1);
> - input_files[nb_input_files - 1] = f;
> + ALLOC_ARRAY_ELEM(input_files, nb_input_files);
> + f = input_files[nb_input_files - 1];
>
> f->ctx = ic;
> f->ist_index = nb_input_streams - ic->nb_streams;
> @@ -2261,11 +2258,8 @@ static int open_output_file(OptionsContext *o, const
> char *filename)
> }
> }
>
> - GROW_ARRAY(output_files, nb_output_files);
> - of = av_mallocz(sizeof(*of));
> - if (!of)
> - exit_program(1);
> - output_files[nb_output_files - 1] = of;
> + ALLOC_ARRAY_ELEM(output_files, nb_output_files);
> + of = output_files[nb_output_files - 1];
>
> of->ost_index = nb_output_streams;
> of->recording_time = o->recording_time;
> @@ -3262,9 +3256,7 @@ static int opt_audio_qscale(void *optctx, const char
> *opt, const char *arg)
>
> static int opt_filter_complex(void *optctx, const char *opt, const char *arg)
> {
> - GROW_ARRAY(filtergraphs, nb_filtergraphs);
> - if (!(filtergraphs[nb_filtergraphs - 1] =
> av_mallocz(sizeof(*filtergraphs[0]))))
> - return AVERROR(ENOMEM);
> + ALLOC_ARRAY_ELEM(filtergraphs, nb_filtergraphs);
> filtergraphs[nb_filtergraphs - 1]->index = nb_filtergraphs - 1;
> filtergraphs[nb_filtergraphs - 1]->graph_desc = av_strdup(arg);
> if (!filtergraphs[nb_filtergraphs - 1]->graph_desc)
> @@ -3281,9 +3273,7 @@ static int opt_filter_complex_script(void *optctx,
> const char *opt, const char *
> if (!graph_desc)
> return AVERROR(EINVAL);
>
> - GROW_ARRAY(filtergraphs, nb_filtergraphs);
> - if (!(filtergraphs[nb_filtergraphs - 1] =
> av_mallocz(sizeof(*filtergraphs[0]))))
> - return AVERROR(ENOMEM);
> + ALLOC_ARRAY_ELEM(filtergraphs, nb_filtergraphs);
> filtergraphs[nb_filtergraphs - 1]->index = nb_filtergraphs - 1;
> filtergraphs[nb_filtergraphs - 1]->graph_desc = graph_desc;
>
>
Will apply this patchset tonight unless there are objections.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
