[FFmpeg-devel] [PATCH] mjpegdec: Properly fail on malloc failure

[Michael Niedermayer]

On Mon, May 09, 2016 at 01:58:57PM +0100, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> ---
>  libavcodec/mjpegdec.c | 56 +++++++++++++++++++++++++++------------------------
>  1 file changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index 7d38fc2..32e8cee 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -1876,34 +1876,35 @@ static int mjpeg_decode_com(MJpegDecodeContext *s)
>  {
>      int len = get_bits(&s->gb, 16);
>      if (len >= 2 && 8 * len - 16 <= get_bits_left(&s->gb)) {
> +        int i;
>          char *cbuf = av_malloc(len - 1);
> -        if (cbuf) {
> -            int i;
> -            for (i = 0; i < len - 2; i++)
> -                cbuf[i] = get_bits(&s->gb, 8);
> -            if (i > 0 && cbuf[i - 1] == '\n')
> -                cbuf[i - 1] = 0;
> -            else
> -                cbuf[i] = 0;
> +        if (!cbuf)
> +            return AVERROR(ENOMEM);
>  
> -            if (s->avctx->debug & FF_DEBUG_PICT_INFO)
> -                av_log(s->avctx, AV_LOG_INFO, "comment: '%s'\n", cbuf);
> -
> -            /* buggy avid, it puts EOI only at every 10th frame */
> -            if (!strncmp(cbuf, "AVID", 4)) {
> -                parse_avid(s, cbuf, len);
> -            } else if (!strcmp(cbuf, "CS=ITU601"))
> -                s->cs_itu601 = 1;
> -            else if ((!strncmp(cbuf, "Intel(R) JPEG Library, version 1", 32) && s->avctx->codec_tag) ||
> -                     (!strncmp(cbuf, "Metasoft MJPEG Codec", 20)))
> -                s->flipped = 1;
> -            else if (!strcmp(cbuf, "MULTISCOPE II")) {
> -                s->avctx->sample_aspect_ratio = (AVRational) { 1, 2 };
> -                s->multiscope = 2;
> -            }
> +        for (i = 0; i < len - 2; i++)
> +            cbuf[i] = get_bits(&s->gb, 8);
> +        if (i > 0 && cbuf[i - 1] == '\n')
> +            cbuf[i - 1] = 0;
> +        else
> +            cbuf[i] = 0;
>  
> -            av_free(cbuf);
> +        if (s->avctx->debug & FF_DEBUG_PICT_INFO)
> +            av_log(s->avctx, AV_LOG_INFO, "comment: '%s'\n", cbuf);
> +
> +        /* buggy avid, it puts EOI only at every 10th frame */
> +        if (!strncmp(cbuf, "AVID", 4)) {
> +            parse_avid(s, cbuf, len);
> +        } else if (!strcmp(cbuf, "CS=ITU601"))
> +            s->cs_itu601 = 1;
> +        else if ((!strncmp(cbuf, "Intel(R) JPEG Library, version 1", 32) && s->avctx->codec_tag) ||
> +                 (!strncmp(cbuf, "Metasoft MJPEG Codec", 20)))
> +            s->flipped = 1;
> +        else if (!strcmp(cbuf, "MULTISCOPE II")) {
> +            s->avctx->sample_aspect_ratio = (AVRational) { 1, 2 };
> +            s->multiscope = 2;
>          }
> +
> +        av_free(cbuf);
>      }
>  
>      return 0;
> @@ -2114,8 +2115,11 @@ int ff_mjpeg_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>          else if (start_code >= APP0 && start_code <= APP15)
>              mjpeg_decode_app(s);
>              /* Comment */
> -        else if (start_code == COM)
> -            mjpeg_decode_com(s);
> +        else if (start_code == COM) {
> +            ret = mjpeg_decode_com(s);
> +            if (ret < 0)
> +                return ret;

this possibly should be goto fail like most other failure cases
either way LGTM

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160509/212a3cca/attachment.sig>