[FFmpeg-devel] [PATCH] lavc/huffyuvdec: fix mem leak in case of init failure

[Lukasz Marek]

On 23.11.2014 11:42, Michael Niedermayer wrote:
> On Sun, Nov 23, 2014 at 12:58:30AM +0100, Lukasz Marek wrote:
>> Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
>> ---
>>   libavcodec/huffyuvdec.c | 24 ++++++++++++++++--------
>>   1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c
>> index 3b2b0f7..5535323 100644
>> --- a/libavcodec/huffyuvdec.c
>> +++ b/libavcodec/huffyuvdec.c
>> @@ -275,7 +275,7 @@ static int read_old_huffman_tables(HYuvContext *s)
>>   static av_cold int decode_init(AVCodecContext *avctx)
>>   {
>>       HYuvContext *s = avctx->priv_data;
>> -    int ret;
>> +    int ret, i;
>>
>>       ff_huffyuvdsp_init(&s->hdsp);
>>       memset(s->vlc, 0, 4 * sizeof(VLC));
>> @@ -327,7 +327,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>
>>           if ((ret = read_huffman_tables(s, avctx->extradata + 4,
>>                                          avctx->extradata_size - 4)) < 0)
>> -            return ret;
>> +            goto error;
>>       } else {
>>           switch (avctx->bits_per_coded_sample & 7) {
>>           case 1:
>> @@ -355,7 +355,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>           s->context       = 0;
>>
>>           if ((ret = read_old_huffman_tables(s)) < 0)
>> -            return ret;
>> +            goto error;
>>       }
>>
>>       if (s->version <= 2) {
>> @@ -383,7 +383,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>               s->alpha = 1;
>>               break;
>>           default:
>> -            return AVERROR_INVALIDDATA;
>> +            ret = AVERROR_INVALIDDATA;
>> +            goto error;
>>           }
>>           av_pix_fmt_get_chroma_sub_sample(avctx->pix_fmt,
>>                                            &s->chroma_h_shift,
>> @@ -520,7 +521,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>               avctx->pix_fmt = AV_PIX_FMT_YUVA420P16;
>>               break;
>>           default:
>> -            return AVERROR_INVALIDDATA;
>> +            ret = AVERROR_INVALIDDATA;
>> +            goto error;
>>           }
>>       }
>>
>> @@ -528,21 +530,27 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>
>>       if ((avctx->pix_fmt == AV_PIX_FMT_YUV422P || avctx->pix_fmt == AV_PIX_FMT_YUV420P) && avctx->width & 1) {
>>           av_log(avctx, AV_LOG_ERROR, "width must be even for this colorspace\n");
>> -        return AVERROR_INVALIDDATA;
>> +        ret = AVERROR_INVALIDDATA;
>> +        goto error;
>>       }
>>       if (s->predictor == MEDIAN && avctx->pix_fmt == AV_PIX_FMT_YUV422P &&
>>           avctx->width % 4) {
>>           av_log(avctx, AV_LOG_ERROR, "width must be a multiple of 4 "
>>                  "for this combination of colorspace and predictor type.\n");
>> -        return AVERROR_INVALIDDATA;
>> +        ret = AVERROR_INVALIDDATA;
>> +        goto error;
>>       }
>>
>>       if ((ret = ff_huffyuv_alloc_temp(s)) < 0) {
>>           ff_huffyuv_common_end(s);
>> -        return ret;
>> +        goto error;
>>       }
>>
>>       return 0;
>> +  error:
>> +    for (i = 0; i < 8; i++)
>> +        ff_free_vlc(&s->vlc[i]);
>
> i think calling decode_end() is better than duplicating what it does
> if it works

I have no opinion about that, but changed. I had to move decode_end above.
I could add prototype but from these 2 I prefer move it as it is not long.

Perfectly decode end callback could be called in avcodec_open2 on fail 
but I'm not sure every codec under any circumstances is secure to call 
it twice.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavc-huffyuvdec-fix-mem-leak-in-case-of-init-failure.patch
Type: text/x-patch
Size: 3874 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141123/1121b585/attachment.bin>