From patchwork Sun Dec 31 21:35:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carl Eugen Hoyos X-Patchwork-Id: 7055 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.79.195 with SMTP id r64csp12973636jad; Sun, 31 Dec 2017 13:35:58 -0800 (PST) X-Google-Smtp-Source: ACJfBouZuH50BUxVQGQL+4Pf+i4UYQtFVRWAUDQhp9kSgvnmKHVHOwlOoQYujai6FC01XMX9u7OF X-Received: by 10.28.63.14 with SMTP id m14mr32056475wma.116.1514756158232; Sun, 31 Dec 2017 13:35:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1514756158; cv=none; d=google.com; s=arc-20160816; b=jawyPiQpAUi6CcuyCPDnRgYLjwq8AHxHqCHezqa8UHVxvJBREXR8lk1CpKcQEFYlVV 8TJbXDA7K/XSG4+MJat9o1wEmGaM3cBCIJZD2/STt+86TEt/dEKPoH2ZL4jNGSBcPdeU jaw96FOUSoZegW3dwXYhT7WHZOzy4FGk0yl5quinSlKOUG/o4Ji1UdIvIU242vIy2Izi kiDewus+cQ7IdoKDiClbtY2NmuFEe5z0V6ilnAbNbRmj3UN4nYTHAUgEuPnGNdFcDF3p M6CPGAxPLlcFmTwdpDcnwQRBU+hBEJr+janblYeYb7p/fTCt2ILKBPxjnbNdNilOI4lq I2Kg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:references:in-reply-to:mime-version :dkim-signature:delivered-to:arc-authentication-results; bh=fZ3i+njqTlopP2tV12y4JOpOK0uRP5xdQSPIDJdbnSA=; b=Iab1vmkm/s+i+YpDdWkkdFxmbfiT1CUkC5ddDNr1lUwpJJsUZJgag2z8lnt1eP+7fr 7T9hUSy5p+IozZS0S4/o46OagLYqhddKLCm1WMyQdOhgiF3wFPjT7lkoX2dbekELs3S9 MAswM2BsUGx6lW+cr7NZPKn8Y5bx2ZG15B9vPyOHqVQPWBQ+qkRewfJIVfPzGETfgH9r oGXDb1AMSYSielT056me2UQC741eR/D78Jqojjnki70msbrZtHqALs7OaPH8lCPdUns/ xubVVuRz1XTPOA7nC3dh+pxYoGmr2ut/s/DLEeXIVF4RJ6pqsJC1NuV5FyeGi1QYSV34 KVPg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=K3OcHwxW; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id r65si19113078wmf.10.2017.12.31.13.35.57; Sun, 31 Dec 2017 13:35:58 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=K3OcHwxW; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 257B5680765; Sun, 31 Dec 2017 23:35:41 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-it0-f45.google.com (mail-it0-f45.google.com [209.85.214.45]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B36A1680154 for ; Sun, 31 Dec 2017 23:35:34 +0200 (EET) Received: by mail-it0-f45.google.com with SMTP id p139so35537863itb.1 for ; Sun, 31 Dec 2017 13:35:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=VvlmajMMgv/aNtEda4oWyw4oLfNdcgM6WdQJVEMT1yE=; b=K3OcHwxWOfiWgN6pegjJizOYHKQN8fuxRxpcn521usV6NrNB8PZQTBYd+FzWy2ScWR qGCXPAXKCvmv6uvN7XzAAyh3aoQWd7Mbbd0OvdU8eB/QOOHbkhAFP+1ViaUCXprS6/vQ maoytljgCaAWm/7Rlfs+QyRDjXjNGydxCQsHe311c7RnbSUUVeeX1XBnCdhOgYdKsmfY 7SDTvgF0FMCClMd46Mb3zrf98FsZ4oAQvPGN6Rrf/vXK06hIErFsgrD+12/LaIcQKgUE ahD4SXqet24RzN9bisqreeK50Q0Xq/tAnFch3tFrPhbiYNj5VulOutixYF5O6TkG2BFW AjQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=VvlmajMMgv/aNtEda4oWyw4oLfNdcgM6WdQJVEMT1yE=; b=WNcDKAKGTgQd4hqzIwGgGf+KhD7KttbPCl61S8r6//AMzut/G/GmH+j8k1Pbm7lrDF StsmuEtDNvLdvoGAgQJcc2Wfo0053YeqZZH1jSBRO/tB/9XX1pDGZeuKkliNiSDpXQ6F icIbcx3nNT7oJkFjpqHavWPl4p3HRfxQkjbqW7tEXsvyTWkeVSW1h6fe16myWg4CEKyi tB77WcP1HhnDLXMVYjOOKasVJ0wVy+QF+ki1XQjqTGJTsO9OT3bEYd2Fh+V67azPvotD ogjmYOuCWx1l8AB3pgvZ/iNo8kxqCJFUMQ3v2pA+GT8viDyQAfTu6o8Vv9qbGNKdOwTN 5FBg== X-Gm-Message-State: AKGB3mLFI8V59sU1JvENKxNT9BfR78UNOFov6wt+CS9NWz9rE5aMZNIU S66d6oPIxbp8iTEGxD2LO6JeyP24HfDoztGUJ5E= X-Received: by 10.36.123.134 with SMTP id q128mr55487599itc.80.1514756148701; Sun, 31 Dec 2017 13:35:48 -0800 (PST) MIME-Version: 1.0 Received: by 10.2.119.211 with HTTP; Sun, 31 Dec 2017 13:35:28 -0800 (PST) In-Reply-To: References: <20171128203239.GC4636@nb4> <20171231211702.GQ4926@michaelspb> From: Carl Eugen Hoyos Date: Sun, 31 Dec 2017 22:35:28 +0100 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH]lavf/mov: Do not blindly allocate stts entries X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 2017-12-31 22:26 GMT+01:00 Carl Eugen Hoyos : > 2017-12-31 22:17 GMT+01:00 Michael Niedermayer : >> On Sat, Dec 30, 2017 at 02:36:39PM +0100, Carl Eugen Hoyos wrote: >>> 2017-12-29 23:37 GMT+01:00 Carl Eugen Hoyos : >>> > 2017-11-28 21:32 GMT+01:00 Michael Niedermayer : >>> >> On Mon, Nov 27, 2017 at 05:24:14AM +0100, Carl Eugen Hoyos wrote: >>> > >>> >>> for (i = 0; i < entries && !pb->eof_reached; i++) { >>> >>> - int sample_duration; >>> >>> + int sample_duration, ret; >>> >>> unsigned int sample_count; >>> >>> + if (i > sc->stts_count) { >>> >>> + ret = av_reallocp_array(&sc->stts_data, >>> >>> + FFMIN(sc->stts_count * 2LL, entries), >>> >>> + sizeof(*sc->stts_data)); >>> >> >>> >> this should use a variant of av_fast_realloc >>> > >>> > Do you prefer the new patch? >>> > The old variant here looks slightly saner to me. >>> >>> Attached is what you possibly had in mind. >>> >>> Please review, Carl Eugen >> >>> mov.c | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> cc7986179fe0ddc394457e8543d9ae907b49373c 0001-lavf-mov-Use-av_fast_realloc-in-mov_read_stts.patch >>> From f5fcd9ed1e5ce604c358a3787f1977277005ebb5 Mon Sep 17 00:00:00 2001 >>> From: Carl Eugen Hoyos >>> Date: Sat, 30 Dec 2017 14:34:41 +0100 >>> Subject: [PATCH] lavf/mov: Use av_fast_realloc() in mov_read_stts(). >>> >>> Avoids large allocations for short files with invalid stts entry. >>> Fixes bugzilla 1102. >>> --- >>> libavformat/mov.c | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index 2064473..1e97652 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -2850,13 +2850,22 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>> av_log(c->fc, AV_LOG_WARNING, "Duplicated STTS atom\n"); >>> av_free(sc->stts_data); >>> sc->stts_count = 0; >>> - sc->stts_data = av_malloc_array(entries, sizeof(*sc->stts_data)); >>> - if (!sc->stts_data) >>> + if (entries >= INT_MAX / sizeof(*sc->stts_data)) >>> return AVERROR(ENOMEM); >> >> this leaves a stale pointer on error in sc->stts_data > > New patch attached. Which would not have worked as intended, new variant attached. Carl Eugen From 9eaf0b56245820194e8e1bee0e3730f3c7362158 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos Date: Sun, 31 Dec 2017 22:30:57 +0100 Subject: [PATCH] lavf/mov: Use av_fast_realloc() in mov_read_stts(). Avoids large allocations for short files with invalid stts entry. Fixes bugzilla 1102. --- libavformat/mov.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 2064473..22faecf 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2830,7 +2830,7 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) { AVStream *st; MOVStreamContext *sc; - unsigned int i, entries; + unsigned int i, entries, alloc_size = 0; int64_t duration=0; int64_t total_sample_count=0; @@ -2848,15 +2848,24 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) if (sc->stts_data) av_log(c->fc, AV_LOG_WARNING, "Duplicated STTS atom\n"); - av_free(sc->stts_data); + av_freep(&sc->stts_data); sc->stts_count = 0; - sc->stts_data = av_malloc_array(entries, sizeof(*sc->stts_data)); - if (!sc->stts_data) + if (entries >= INT_MAX / sizeof(*sc->stts_data)) return AVERROR(ENOMEM); for (i = 0; i < entries && !pb->eof_reached; i++) { int sample_duration; unsigned int sample_count; + unsigned min_entries = FFMIN(FFMAX(i, 1024 * 1024), entries); + MOVStts *stts_data = av_fast_realloc(sc->stts_data, &alloc_size, + min_entries * sizeof(*sc->stts_data)); + if (!stts_data) { + av_freep(&sc->stts_data); + sc->stts_count = 0; + return AVERROR(ENOMEM); + } + sc->stts_count = min_entries; + sc->stts_data = stts_data; sample_count=avio_rb32(pb); sample_duration = avio_rb32(pb); -- 1.7.10.4