[FFmpeg-devel] [PATCH] avformat/mpegts: Don't leave context in inconsistent state upon error

[Marton Balint]

On Mon, 10 Aug 2020, Andreas Rheinhardt wrote:

> Up until now, opening a section filter works as follows: A filter is
> opened and (on success) attached to the MpegTSContext. Then a buffer for
> said filter is allocated and upon success attached to the section
> filter; on error, the filter is simply freed without removing it from
> the MpegTSContext, leaving the latter in an inconsistent state. This
> leads to use-after-frees lateron.
>
> This commit fixes this by allocating the buffer first; the filter is
> only opened if the buffer could be successfully allocated.

LGTM, thanks.

Marton

>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> At first I wanted to use a flexible array member for this buffer (do we
> actually support any compiler that doesn't support flexible array
> members?), yet for some reason unknown to me a structure containing a
> flexible array member must not be contained in another structure or
> union (not even at the end, although it makes sense to do so). GCC gives
> a warning in pedantic mode for this.
>
> libavformat/mpegts.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index c6fd3e1cef..f71f18a57d 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -510,20 +510,22 @@ static MpegTSFilter *mpegts_open_section_filter(MpegTSContext *ts,
> {
>     MpegTSFilter *filter;
>     MpegTSSectionFilter *sec;
> +    uint8_t *section_buf = av_mallocz(MAX_SECTION_SIZE);
> 
> -    if (!(filter = mpegts_open_filter(ts, pid, MPEGTS_SECTION)))
> +    if (!section_buf)
>         return NULL;
> +
> +    if (!(filter = mpegts_open_filter(ts, pid, MPEGTS_SECTION))) {
> +        av_free(section_buf);
> +        return NULL;
> +    }
>     sec = &filter->u.section_filter;
>     sec->section_cb  = section_cb;
>     sec->opaque      = opaque;
> -    sec->section_buf = av_mallocz(MAX_SECTION_SIZE);
> +    sec->section_buf = section_buf;
>     sec->check_crc   = check_crc;
>     sec->last_ver    = -1;
> 
> -    if (!sec->section_buf) {
> -        av_free(filter);
> -        return NULL;
> -    }
>     return filter;
> }
> 
> -- 
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".