Re: [FFmpeg-devel] [PATCH] avformat/oggenc: Don't free AVStream's priv_data, fix memleak
Andreas Rheinhardt: > Andreas Rheinhardt: >> For FLAC, Speed, Opus and VP8 the Ogg muxer allocates two buffers >> for building the headers: The first for extradata in an Ogg-specific >> format and the second contains a Vorbiscomment. These buffers are >> reachable via pointers in the corresponding AVStream's priv_data. >> >> If an error happens during building the headers, the AVStream's >> priv_data would be freed. This is pointless in general as it would be >> freed generically anyway, but here it is actively harmful: If the second >> of the aforementioned allocations fails, the first buffer would leak >> upon freeing priv_data. >> >> This commit stops freeing priv_data manually, which allows the muxer to >> properly clean up in the deinit function. >> >> Signed-off-by: Andreas Rheinhardt >> --- >> libavformat/oggenc.c | 7 +-- >> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c >> index fe89f23e36..fbd14fedf9 100644 >> --- a/libavformat/oggenc.c >> +++ b/libavformat/oggenc.c >> @@ -546,7 +546,6 @@ static int ogg_init(AVFormatContext *s) >> &st->metadata); >> if (err) { >> av_log(s, AV_LOG_ERROR, "Error writing FLAC headers\n"); >> -av_freep(&st->priv_data); >> return err; >> } >> } else if (st->codecpar->codec_id == AV_CODEC_ID_SPEEX) { >> @@ -555,7 +554,6 @@ static int ogg_init(AVFormatContext *s) >>&st->metadata); >> if (err) { >> av_log(s, AV_LOG_ERROR, "Error writing Speex headers\n"); >> -av_freep(&st->priv_data); >> return err; >> } >> } else if (st->codecpar->codec_id == AV_CODEC_ID_OPUS) { >> @@ -564,7 +562,6 @@ static int ogg_init(AVFormatContext *s) >> &st->metadata, s->chapters, >> s->nb_chapters); >> if (err) { >> av_log(s, AV_LOG_ERROR, "Error writing Opus headers\n"); >> -av_freep(&st->priv_data); >> return err; >> } >> } else if (st->codecpar->codec_id == AV_CODEC_ID_VP8) { >> @@ -572,7 +569,6 @@ static int ogg_init(AVFormatContext *s) >> s->flags & AVFMT_FLAG_BITEXACT); >> if (err) { >> av_log(s, AV_LOG_ERROR, "Error writing VP8 headers\n"); >> -av_freep(&st->priv_data); >> return err; >> } >> } else { >> @@ -585,7 +581,7 @@ static int ogg_init(AVFormatContext *s) >>st->codecpar->codec_id == >> AV_CODEC_ID_VORBIS ? 30 : 42, >>(const uint8_t**)oggstream->header, >> oggstream->header_len) < 0) { >> av_log(s, AV_LOG_ERROR, "Extradata corrupted\n"); >> -av_freep(&st->priv_data); >> +oggstream->header[1] = NULL; >> return AVERROR_INVALIDDATA; >> } >> >> @@ -755,7 +751,6 @@ static void ogg_free(AVFormatContext *s) >> av_freep(&oggstream->header[0]); >> } >> av_freep(&oggstream->header[1]); >> -av_freep(&st->priv_data); >> } >> >> while (p) { >> > Will apply this tomorrow if there are no objections. > > - Andreas > Applied. - 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".
Re: [FFmpeg-devel] [PATCH] avformat/oggenc: Don't free AVStream's priv_data, fix memleak
Andreas Rheinhardt: > For FLAC, Speed, Opus and VP8 the Ogg muxer allocates two buffers > for building the headers: The first for extradata in an Ogg-specific > format and the second contains a Vorbiscomment. These buffers are > reachable via pointers in the corresponding AVStream's priv_data. > > If an error happens during building the headers, the AVStream's > priv_data would be freed. This is pointless in general as it would be > freed generically anyway, but here it is actively harmful: If the second > of the aforementioned allocations fails, the first buffer would leak > upon freeing priv_data. > > This commit stops freeing priv_data manually, which allows the muxer to > properly clean up in the deinit function. > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/oggenc.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c > index fe89f23e36..fbd14fedf9 100644 > --- a/libavformat/oggenc.c > +++ b/libavformat/oggenc.c > @@ -546,7 +546,6 @@ static int ogg_init(AVFormatContext *s) > &st->metadata); > if (err) { > av_log(s, AV_LOG_ERROR, "Error writing FLAC headers\n"); > -av_freep(&st->priv_data); > return err; > } > } else if (st->codecpar->codec_id == AV_CODEC_ID_SPEEX) { > @@ -555,7 +554,6 @@ static int ogg_init(AVFormatContext *s) >&st->metadata); > if (err) { > av_log(s, AV_LOG_ERROR, "Error writing Speex headers\n"); > -av_freep(&st->priv_data); > return err; > } > } else if (st->codecpar->codec_id == AV_CODEC_ID_OPUS) { > @@ -564,7 +562,6 @@ static int ogg_init(AVFormatContext *s) > &st->metadata, s->chapters, > s->nb_chapters); > if (err) { > av_log(s, AV_LOG_ERROR, "Error writing Opus headers\n"); > -av_freep(&st->priv_data); > return err; > } > } else if (st->codecpar->codec_id == AV_CODEC_ID_VP8) { > @@ -572,7 +569,6 @@ static int ogg_init(AVFormatContext *s) > s->flags & AVFMT_FLAG_BITEXACT); > if (err) { > av_log(s, AV_LOG_ERROR, "Error writing VP8 headers\n"); > -av_freep(&st->priv_data); > return err; > } > } else { > @@ -585,7 +581,7 @@ static int ogg_init(AVFormatContext *s) >st->codecpar->codec_id == > AV_CODEC_ID_VORBIS ? 30 : 42, >(const uint8_t**)oggstream->header, > oggstream->header_len) < 0) { > av_log(s, AV_LOG_ERROR, "Extradata corrupted\n"); > -av_freep(&st->priv_data); > +oggstream->header[1] = NULL; > return AVERROR_INVALIDDATA; > } > > @@ -755,7 +751,6 @@ static void ogg_free(AVFormatContext *s) > av_freep(&oggstream->header[0]); > } > av_freep(&oggstream->header[1]); > -av_freep(&st->priv_data); > } > > while (p) { > Will apply this tomorrow if there are no objections. - 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".
[FFmpeg-devel] [PATCH] avformat/oggenc: Don't free AVStream's priv_data, fix memleak
For FLAC, Speed, Opus and VP8 the Ogg muxer allocates two buffers for building the headers: The first for extradata in an Ogg-specific format and the second contains a Vorbiscomment. These buffers are reachable via pointers in the corresponding AVStream's priv_data. If an error happens during building the headers, the AVStream's priv_data would be freed. This is pointless in general as it would be freed generically anyway, but here it is actively harmful: If the second of the aforementioned allocations fails, the first buffer would leak upon freeing priv_data. This commit stops freeing priv_data manually, which allows the muxer to properly clean up in the deinit function. Signed-off-by: Andreas Rheinhardt --- libavformat/oggenc.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c index fe89f23e36..fbd14fedf9 100644 --- a/libavformat/oggenc.c +++ b/libavformat/oggenc.c @@ -546,7 +546,6 @@ static int ogg_init(AVFormatContext *s) &st->metadata); if (err) { av_log(s, AV_LOG_ERROR, "Error writing FLAC headers\n"); -av_freep(&st->priv_data); return err; } } else if (st->codecpar->codec_id == AV_CODEC_ID_SPEEX) { @@ -555,7 +554,6 @@ static int ogg_init(AVFormatContext *s) &st->metadata); if (err) { av_log(s, AV_LOG_ERROR, "Error writing Speex headers\n"); -av_freep(&st->priv_data); return err; } } else if (st->codecpar->codec_id == AV_CODEC_ID_OPUS) { @@ -564,7 +562,6 @@ static int ogg_init(AVFormatContext *s) &st->metadata, s->chapters, s->nb_chapters); if (err) { av_log(s, AV_LOG_ERROR, "Error writing Opus headers\n"); -av_freep(&st->priv_data); return err; } } else if (st->codecpar->codec_id == AV_CODEC_ID_VP8) { @@ -572,7 +569,6 @@ static int ogg_init(AVFormatContext *s) s->flags & AVFMT_FLAG_BITEXACT); if (err) { av_log(s, AV_LOG_ERROR, "Error writing VP8 headers\n"); -av_freep(&st->priv_data); return err; } } else { @@ -585,7 +581,7 @@ static int ogg_init(AVFormatContext *s) st->codecpar->codec_id == AV_CODEC_ID_VORBIS ? 30 : 42, (const uint8_t**)oggstream->header, oggstream->header_len) < 0) { av_log(s, AV_LOG_ERROR, "Extradata corrupted\n"); -av_freep(&st->priv_data); +oggstream->header[1] = NULL; return AVERROR_INVALIDDATA; } @@ -755,7 +751,6 @@ static void ogg_free(AVFormatContext *s) av_freep(&oggstream->header[0]); } av_freep(&oggstream->header[1]); -av_freep(&st->priv_data); } while (p) { -- 2.20.1 ___ 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".