On Sat, Apr 11, 2015 at 2:59 PM, Diego Biurrun <[email protected]> wrote:
>> @@ -104,6 +110,13 @@ static int decode_frame(AVCodecContext *avctx, void
>> *data, int *data_size,
>> sub->rects[0]->pict.data[0] = av_malloc(w * h);
>> sub->rects[0]->nb_colors = 4;
>> sub->rects[0]->pict.data[1] = av_mallocz(AVPALETTE_SIZE);
>> + if (!sub->rects[0]->pict.data[0] || sub->rects[0]->pict.data[1]) {
>> + av_freep(&sub->rects[0]->pict.data[1]);
>> + av_freep(&sub->rects[0]->pict.data[0]);
>> + av_freep(&sub->rects[0]);
>> + av_freep(&sub->rects);
>> + return AVERROR(ENOMEM);
>> + }
>
> OK with the missing ! fixed.
>
>
> Maybe you can push the above bits already so that this patch makes progress?
Well if I have to push, I prefer a single shot, otherwise, feel free
to do so, if that's ok.
>> --- a/libavcodec/eatgv.c
>> +++ b/libavcodec/eatgv.c
>> @@ -174,12 +174,15 @@ static int tgv_decode_inter(TgvContext *s, AVFrame
>> *frame,
>> /* allocate codebook buffers as necessary */
>> if (num_mvs > s->num_mvs) {
>> s->mv_codebook = av_realloc(s->mv_codebook, num_mvs*2*sizeof(int));
>> + if (!s->mv_codebook)
>> + return AVERROR(ENOMEM);
>> s->num_mvs = num_mvs;
>> }
>>
>> if (num_blocks_packed > s->num_blocks_packed) {
>> int err;
>> if ((err = av_reallocp(&s->block_codebook, num_blocks_packed * 16))
>> < 0) {
>> + av_freep(&s->mv_codebook);
>> s->num_blocks_packed = 0;
>> return err;
>> }
>
> This function has many more error returns, you leak memory in all those cases.
ok i'll send a separate patch
>> --- a/libavcodec/flacenc.c
>> +++ b/libavcodec/flacenc.c
>> @@ -626,6 +626,8 @@ static uint64_t calc_rice_params(RiceContext *rc, int
>> pmin, int pmax,
>> tmp_rc.coding_mode = rc->coding_mode;
>>
>> udata = av_malloc(n * sizeof(uint32_t));
>> + if (!udata)
>> + return AVERROR(ENOMEM);
>> for (i = 0; i < n; i++)
>> udata[i] = (2*data[i]) ^ (data[i]>>31);
>
> This function returns uint64_t, so this won't work as expected.
hm, what do you suggest here? Make calc_rice_params() take an
additional parameter to return the error?
>> --- a/libavcodec/huffyuvenc.c
>> +++ b/libavcodec/huffyuvenc.c
>> @@ -155,7 +155,7 @@ static av_cold int encode_init(AVCodecContext *avctx)
>> s->version = 2;
>>
>> avctx->coded_frame = av_frame_alloc();
>> - if (!avctx->coded_frame)
>> + if (!avctx->extradata || !avctx->stats_out || !avctx->coded_frame)
>> return AVERROR(ENOMEM);
>
> Looks like it leaks memory if some, but not all, of the allocations succeed.
This is now taken care of by FF_CODEC_CAP_INIT_CLEANUP.
>> --- a/libavcodec/jpeglsenc.c
>> +++ b/libavcodec/jpeglsenc.c
>> @@ -271,6 +271,8 @@ static int encode_picture_ls(AVCodecContext *avctx,
>> AVPacket *pkt,
>> }
>>
>> buf2 = av_malloc(pkt->size);
>> + if (!buf2)
>> + return AVERROR(ENOMEM);
>
> What happens with the packet allocated just above?
leak, separate patch coming
>> --- a/libavcodec/lclenc.c
>> +++ b/libavcodec/lclenc.c
>> @@ -135,6 +135,8 @@ static av_cold int encode_init(AVCodecContext *avctx)
>>
>> avctx->extradata= av_mallocz(8);
>> + if (!avctx->extradata)
>> + return AVERROR(ENOMEM);
>>
>> avctx->coded_frame = av_frame_alloc();
>> if (!avctx->coded_frame)
>
> Where is extradata freed if av_frame_alloc() fails?
This is now taken care of by FF_CODEC_CAP_INIT_CLEANUP.
>> --- a/libavcodec/libx264.c
>> +++ b/libavcodec/libx264.c
>> @@ -535,6 +535,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
>>
>> s = x264_encoder_headers(x4->enc, &nal, &nnal);
>> avctx->extradata = p = av_malloc(s);
>> + if (!p)
>> + return AVERROR(ENOMEM);
>>
>> @@ -542,6 +544,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
>> av_log(avctx, AV_LOG_INFO, "%s\n", nal[i].p_payload+25);
>> x4->sei_size = nal[i].i_payload;
>> x4->sei = av_malloc(x4->sei_size);
>> + if (!x4->sei)
>> + return AVERROR(ENOMEM);
>
> This leaks the above allocation, there is also the coded_frame staying around.
This is now taken care of by FF_CODEC_CAP_INIT_CLEANUP.
>> --- a/libavcodec/libxvid.c
>> +++ b/libavcodec/libxvid.c
>> @@ -593,11 +595,15 @@ static av_cold int xvid_encode_init(AVCodecContext
>> *avctx)
>> if (avctx->intra_matrix) {
>> intra = avctx->intra_matrix;
>> x->intra_matrix = av_malloc(sizeof(unsigned char) * 64);
>> + if (!x->intra_matrix)
>> + return AVERROR(ENOMEM);
>> } else
>> intra = NULL;
>> if (avctx->inter_matrix) {
>> inter = avctx->inter_matrix;
>> x->inter_matrix = av_malloc(sizeof(unsigned char) * 64);
>> + if (!x->inter_matrix)
>> + return AVERROR(ENOMEM);
>> } else
>> inter = NULL;
>
> Seems to leak the first allocation if the second fails.
This is now taken care of by FF_CODEC_CAP_INIT_CLEANUP.
>> --- a/libavcodec/pthread_frame.c
>> +++ b/libavcodec/pthread_frame.c
>> @@ -584,8 +584,15 @@ int ff_frame_thread_init(AVCodecContext *avctx)
>> }
>>
>> avctx->internal->thread_ctx = fctx =
>> av_mallocz(sizeof(FrameThreadContext));
>> + if (!fctx)
>> + return AVERROR(ENOMEM);
>>
>> fctx->threads = av_mallocz(sizeof(PerThreadContext) * thread_count);
>> + if (!fctx->threads) {
>> + av_freep(&avctx->internal->thread_ctx);
>> + return AVERROR(ENOMEM);
>> + }
>
> Just "av_freep(fctx);" seems simpler, but OK.
>
>> --- a/libavcodec/svq1enc.c
>> +++ b/libavcodec/svq1enc.c
>> @@ -293,6 +293,11 @@ static int svq1_encode_plane(SVQ1EncContext *s, int
>> plane,
>> s->motion_val16[plane] = av_mallocz((s->m.mb_stride *
>> (block_height + 2) + 1) *
>> 2 * sizeof(int16_t));
>> + if (!s->motion_val8[plane] || !s->motion_val16[plane]) {
>> + av_freep(&s->motion_val8[plane]);
>> + av_freep(&s->motion_val16[plane]);
>> + return AVERROR(ENOMEM);
>> + }
>> }
>
> There are other error returns that appear to leak this memory.
I see a single instance on line 367 or are there more?
This seem rather complex since it could still leak values from other
planes, how would you do it?
Thanks
--
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
