[FFmpeg-cvslog] lavf: sanity check size in av_get/append_packet().

[Reimar Döffinger]

Hello,
I don't really like that arbitrary, non-configurable limit at all.
We already have a configurable, user-settable allocation limit for the av_malloc etc. functions in libavutil, isn't that good enough?

Reimar

On 16 Mar 2013, at 15:48, git at videolan.org (Anton Khirnov) wrote:

> ffmpeg | branch: master | Anton Khirnov <anton at khirnov.net> | Tue Feb 19 17:20:35 2013 +0100| [aa3c77998404cc32233cb76e961ca27db8565459] | committer: Anton Khirnov
> 
> lavf: sanity check size in av_get/append_packet().
> 
> To avoid allocating ridiculous amounts of memory for corrupted files,
> read the input in chunks limited to filesize or an arbitrary large
> amount when that is not known (chosen to be 50M).
> 
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=aa3c77998404cc32233cb76e961ca27db8565459
> ---
> 
> libavformat/utils.c |   68 +++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 49 insertions(+), 19 deletions(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 75e92ae..284d992 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -214,38 +214,68 @@ AVInputFormat *av_find_input_format(const char *short_name)
>     return NULL;
> }
> 
> +/* an arbitrarily chosen "sane" max packet size -- 50M */
> +#define SANE_CHUNK_SIZE (50000000)
> 
> -int av_get_packet(AVIOContext *s, AVPacket *pkt, int size)
> +/*
> + * Read the data in sane-sized chunks and append to pkt.
> + * Return the number of bytes read or an error.
> + */
> +static int append_packet_chunked(AVIOContext *s, AVPacket *pkt, int size)
> {
> -    int ret= av_new_packet(pkt, size);
> +    int64_t chunk_size = size;
> +    int orig_pos       = pkt->pos; // av_grow_packet might reset pos
> +    int orig_size      = pkt->size;
> +    int ret = 0;
> 
> -    if(ret<0)
> -        return ret;
> +    do {
> +        int prev_size = pkt->size;
> +        int read_size;
> +
> +        /*
> +         * When the caller requests a lot of data, limit it to the amount left
> +         * in file or SANE_CHUNK_SIZE when it is not known
> +         */
> +        if (size > SANE_CHUNK_SIZE) {
> +            int64_t filesize = avio_size(s) - avio_tell(s);
> +            chunk_size = FFMAX(filesize, SANE_CHUNK_SIZE);
> +        }
> +        read_size = FFMIN(size, chunk_size);
> +
> +        ret = av_grow_packet(pkt, read_size);
> +        if (ret < 0)
> +            break;
> +
> +        ret = avio_read(s, pkt->data + prev_size, read_size);
> +        if (ret != read_size) {
> +            av_shrink_packet(pkt, prev_size + FFMAX(ret, 0));
> +            break;
> +        }
> 
> -    pkt->pos= avio_tell(s);
> +        size -= read_size;
> +    } while (size > 0);
> 
> -    ret= avio_read(s, pkt->data, size);
> -    if(ret<=0)
> +    pkt->pos = orig_pos;
> +    if (!pkt->size)
>         av_free_packet(pkt);
> -    else
> -        av_shrink_packet(pkt, ret);
> +    return pkt->size > orig_size ? pkt->size - orig_size : ret;
> +}
> 
> -    return ret;
> +int av_get_packet(AVIOContext *s, AVPacket *pkt, int size)
> +{
> +    av_init_packet(pkt);
> +    pkt->data = NULL;
> +    pkt->size = 0;
> +    pkt->pos  = avio_tell(s);
> +
> +    return append_packet_chunked(s, pkt, size);
> }
> 
> int av_append_packet(AVIOContext *s, AVPacket *pkt, int size)
> {
> -    int ret;
> -    int old_size;
>     if (!pkt->size)
>         return av_get_packet(s, pkt, size);
> -    old_size = pkt->size;
> -    ret = av_grow_packet(pkt, size);
> -    if (ret < 0)
> -        return ret;
> -    ret = avio_read(s, pkt->data + old_size, size);
> -    av_shrink_packet(pkt, old_size + FFMAX(ret, 0));
> -    return ret;
> +    return append_packet_chunked(s, pkt, size);
> }
> 
> 
> 
> _______________________________________________
> ffmpeg-cvslog mailing list
> ffmpeg-cvslog at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog