[FFmpeg-devel] [PATCH 2/3] lavf/webm_chunk: Fix NULL dereference
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Fri Jul 12 01:02:00 EEST 2019
Andreas Rheinhardt:
> Andreas Rheinhardt:
>> Andreas Rheinhardt:
>>> Andreas Rheinhardt:
>>>> The earlier version of the webm_chunk muxer had several bugs:
>>>>
>>>> 1. If the first packet of an audio stream didn't have a PTS of zero,
>>>> then no chunk will be started before a packet is delivered to the
>>>> underlying Matroska/WebM muxer, i.e. the AVFormatContext used to write
>>>> these packets had a NULL as AVIOContext for output. This is behind the
>>>> crash in ticket #5752.
>>>>
>>>> 2. If an error happens during writing a packet, the underlyimg
>>>> Matroska/WebM muxer context is freed. This leads to a use-after-free
>>>> coupled with a double-free in webm_chunk_write_trailer (which supposes
>>>> that the underlying AVFormatContext is still valid).
>>>>
>>>> 3. Even when no error occurs at all, webm_chunk_write_trailer is still
>>>> buggy: After the underlying Matroska/WebM muxer has written its trailer,
>>>> ending the chunk implicitly flushes it again which is illegal at this
>>>> point.
>>>>
>>>> These bugs have been fixed.
>>>>
>>>> Fixes #5752.
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>>> ---
>>>> libavformat/webm_chunk.c | 44 +++++++++++++++++++++-------------------
>>>> 1 file changed, 23 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c
>>>> index 2c99753b5b..391fee721a 100644
>>>> --- a/libavformat/webm_chunk.c
>>>> +++ b/libavformat/webm_chunk.c
>>>> @@ -168,7 +168,7 @@ static int chunk_start(AVFormatContext *s)
>>>> return 0;
>>>> }
>>>>
>>>> -static int chunk_end(AVFormatContext *s)
>>>> +static int chunk_end(AVFormatContext *s, int flush)
>>>> {
>>>> WebMChunkContext *wc = s->priv_data;
>>>> AVFormatContext *oc = wc->avf;
>>>> @@ -179,11 +179,14 @@ static int chunk_end(AVFormatContext *s)
>>>> char filename[MAX_FILENAME_SIZE];
>>>> AVDictionary *options = NULL;
>>>>
>>>> - if (wc->chunk_start_index == wc->chunk_index)
>>>> + if (!oc->pb)
>>>> return 0;
>>>> - // Flush the cluster in WebM muxer.
>>>> - oc->oformat->write_packet(oc, NULL);
>>>> +
>>>> + if (flush)
>>>> + // Flush the cluster in WebM muxer.
>>>> + oc->oformat->write_packet(oc, NULL);
>>>> buffer_size = avio_close_dyn_buf(oc->pb, &buffer);
>>>> + oc->pb = NULL;
>>>> ret = get_chunk_filename(s, 0, filename);
>>>> if (ret < 0)
>>>> goto fail;
>>>> @@ -194,7 +197,6 @@ static int chunk_end(AVFormatContext *s)
>>>> goto fail;
>>>> avio_write(pb, buffer, buffer_size);
>>>> ff_format_io_close(s, &pb);
>>>> - oc->pb = NULL;
>>>> fail:
>>>> av_dict_free(&options);
>>>> av_free(buffer);
>>>> @@ -216,27 +218,19 @@ static int webm_chunk_write_packet(AVFormatContext *s, AVPacket *pkt)
>>>> }
>>>>
>>>> // For video, a new chunk is started only on key frames. For audio, a new
>>>> - // chunk is started based on chunk_duration.
>>>> - if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
>>>> + // chunk is started based on chunk_duration. Also, a new chunk is started
>>>> + // unconditionally if there is no currently open chunk.
>>>> + if (!oc->pb || (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
>>>> (pkt->flags & AV_PKT_FLAG_KEY)) ||
>>>> (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO &&
>>>> - (pkt->pts == 0 || wc->duration_written >= wc->chunk_duration))) {
>>>> + wc->duration_written >= wc->chunk_duration)) {
>>>> wc->duration_written = 0;
>>>> - if ((ret = chunk_end(s)) < 0 || (ret = chunk_start(s)) < 0) {
>>>> - goto fail;
>>>> + if ((ret = chunk_end(s, 1)) < 0 || (ret = chunk_start(s)) < 0) {
>>>> + return ret;
>>>> }
>>>> }
>>>>
>>>> ret = oc->oformat->write_packet(oc, pkt);
>>>> - if (ret < 0)
>>>> - goto fail;
>>>> -
>>>> -fail:
>>>> - if (ret < 0) {
>>>> - oc->streams = NULL;
>>>> - oc->nb_streams = 0;
>>>> - avformat_free_context(oc);
>>>> - }
>>>>
>>>> return ret;
>>>> }
>>>> @@ -245,12 +239,20 @@ static int webm_chunk_write_trailer(AVFormatContext *s)
>>>> {
>>>> WebMChunkContext *wc = s->priv_data;
>>>> AVFormatContext *oc = wc->avf;
>>>> + int ret;
>>>> +
>>>> + if (!oc->pb) {
>>>> + ret = chunk_start(s);
>>>> + if (ret < 0)
>>>> + goto fail;
>>>> + }
>>>> oc->oformat->write_trailer(oc);
>>>> - chunk_end(s);
>>>> + ret = chunk_end(s, 0);
>>>> +fail:
>>>> oc->streams = NULL;
>>>> oc->nb_streams = 0;
>>>> avformat_free_context(oc);
>>>> - return 0;
>>>> + return ret;
>>>> }
>>>>
>>>> #define OFFSET(x) offsetof(WebMChunkContext, x)
>>>>
>>>>
>>> Ping for the last two patches of this patchset (i.e. this one and
>>> https://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242902.html)
>>>
>>> - Andreas
>>>
>> And another ping for these two patches.
>>
>> - Andreas
>>
>
> Ping #3.
>
> - Andreas
>
Ping.
- Andreas
More information about the ffmpeg-devel
mailing list