Paul B Mahol:
> On 7/12/19, Andreas Rheinhardt <andreas.rheinha...@gmail.com> wrote:
>> 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.
>
> Will apply first patch from this thread.
> Is this OK?
>
If the first patch you are referring to is
https://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242903.html then
it is OK.
- 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".
