Commit 8c6ee762 authored by Andreas Rheinhardt's avatar Andreas Rheinhardt Committed by Paul B Mahol

lavf/webm_chunk: Fix NULL dereference

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: 's avatarAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
parent 2601eef8
...@@ -173,7 +173,7 @@ static int chunk_start(AVFormatContext *s) ...@@ -173,7 +173,7 @@ static int chunk_start(AVFormatContext *s)
return 0; return 0;
} }
static int chunk_end(AVFormatContext *s) static int chunk_end(AVFormatContext *s, int flush)
{ {
WebMChunkContext *wc = s->priv_data; WebMChunkContext *wc = s->priv_data;
AVFormatContext *oc = wc->avf; AVFormatContext *oc = wc->avf;
...@@ -184,11 +184,14 @@ static int chunk_end(AVFormatContext *s) ...@@ -184,11 +184,14 @@ static int chunk_end(AVFormatContext *s)
char filename[MAX_FILENAME_SIZE]; char filename[MAX_FILENAME_SIZE];
AVDictionary *options = NULL; AVDictionary *options = NULL;
if (wc->chunk_start_index == wc->chunk_index) if (!oc->pb)
return 0; return 0;
if (flush)
// Flush the cluster in WebM muxer. // Flush the cluster in WebM muxer.
oc->oformat->write_packet(oc, NULL); oc->oformat->write_packet(oc, NULL);
buffer_size = avio_close_dyn_buf(oc->pb, &buffer); buffer_size = avio_close_dyn_buf(oc->pb, &buffer);
oc->pb = NULL;
ret = get_chunk_filename(s, 0, filename); ret = get_chunk_filename(s, 0, filename);
if (ret < 0) if (ret < 0)
goto fail; goto fail;
...@@ -199,7 +202,6 @@ static int chunk_end(AVFormatContext *s) ...@@ -199,7 +202,6 @@ static int chunk_end(AVFormatContext *s)
goto fail; goto fail;
avio_write(pb, buffer, buffer_size); avio_write(pb, buffer, buffer_size);
ff_format_io_close(s, &pb); ff_format_io_close(s, &pb);
oc->pb = NULL;
fail: fail:
av_dict_free(&options); av_dict_free(&options);
av_free(buffer); av_free(buffer);
...@@ -221,27 +223,19 @@ static int webm_chunk_write_packet(AVFormatContext *s, AVPacket *pkt) ...@@ -221,27 +223,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 // For video, a new chunk is started only on key frames. For audio, a new
// chunk is started based on chunk_duration. // chunk is started based on chunk_duration. Also, a new chunk is started
if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && // unconditionally if there is no currently open chunk.
if (!oc->pb || (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
(pkt->flags & AV_PKT_FLAG_KEY)) || (pkt->flags & AV_PKT_FLAG_KEY)) ||
(st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && (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; wc->duration_written = 0;
if ((ret = chunk_end(s)) < 0 || (ret = chunk_start(s)) < 0) { if ((ret = chunk_end(s, 1)) < 0 || (ret = chunk_start(s)) < 0) {
goto fail; return ret;
} }
} }
ret = oc->oformat->write_packet(oc, pkt); 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; return ret;
} }
...@@ -250,12 +244,20 @@ static int webm_chunk_write_trailer(AVFormatContext *s) ...@@ -250,12 +244,20 @@ static int webm_chunk_write_trailer(AVFormatContext *s)
{ {
WebMChunkContext *wc = s->priv_data; WebMChunkContext *wc = s->priv_data;
AVFormatContext *oc = wc->avf; AVFormatContext *oc = wc->avf;
int ret;
if (!oc->pb) {
ret = chunk_start(s);
if (ret < 0)
goto fail;
}
oc->oformat->write_trailer(oc); oc->oformat->write_trailer(oc);
chunk_end(s); ret = chunk_end(s, 0);
fail:
oc->streams = NULL; oc->streams = NULL;
oc->nb_streams = 0; oc->nb_streams = 0;
avformat_free_context(oc); avformat_free_context(oc);
return 0; return ret;
} }
#define OFFSET(x) offsetof(WebMChunkContext, x) #define OFFSET(x) offsetof(WebMChunkContext, x)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment