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.rheinha...@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 _______________________________________________ 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".