From patchwork Sun Dec 31 21:26:55 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: 7054 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.79.195 with SMTP id r64csp12968033jad; Sun, 31 Dec 2017 13:27:25 -0800 (PST) X-Google-Smtp-Source: ACJfBosgyHgBpHhx+VVu29QSTYDyo22AnPibDIJO9S8HgaMEGiGyWLmcjcB/85Vbft1H45DGroMj X-Received: by 10.223.187.76 with SMTP id x12mr39974784wrg.110.1514755645444; Sun, 31 Dec 2017 13:27:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1514755645; cv=none; d=google.com; s=arc-20160816; b=Fb7OolJaUHMZBEGtANxHiV8WKNl4OTtYQOtAJwgb43FzMTjDsKffeU4ivEJ3Ah9z9x nfannwMwAefAUaFRv4p8wiYXzHvb3PC3A/JQF+Qgp13R98SNgiWdc/bIRsFJGxvu2LXz SwLdXS286SVQQkBLqvkrxhsOVhGXvvdKoCoAw5tv1QRhGAPKU/Jq5oE0W2pF9TG3hxjG MXhoKEc+xa3pMfByiX4Xkivoq3FmNVo5AuQ8m+EfhONHnnkN1+r0/BgLnP4MINsXVSs0 tJQhW0uI4G3KYinDUXfoN7uhqOQkMkpGCuxYhWemR8quJ1ZEFIaW1gxOo0CC4iz7ArOH 2kLA== 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=09JvEQ/w9aq9eUt1YB4eXUyKbBO37TUDNuNFRCqtCsQ=; b=v11Pjt+PUJz8LWeyHUE5i7+H+sZvpY0wRxca0gmQ7Gg6DSDmofEv/9LGuNT0APhbQd 7ntWgfsE1aPM6dFLGDnE/fH/r4Jzh30GBa9z6EiIJxxoNGGhqo0OEnl8w9VCixrqBMbf Yo5S2p1+fet4s07b6ZqeQyFVlGnjvLuYQsrHD6QY9EGq0YaDgqCcDAqk8n3h8HViD2nW rJPgvwKnYprVwodpuyJDcHx49a97Olcyfj/mAysO9aV1yR/w5dzjTo4+8qQ+eIpT1BiV jyTihiuBuDcMKvV9DmVZgYyZC3GFqLCHZdfhP3XWnzPfr9yz/3fFneBJZZl/r2nZr+7L QEDg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=eC/P+iQX; 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 l3si14424554wrf.469.2017.12.31.13.27.24; Sun, 31 Dec 2017 13:27:25 -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=eC/P+iQX; 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 1B81668075B; Sun, 31 Dec 2017 23:27:08 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-io0-f173.google.com (mail-io0-f173.google.com [209.85.223.173]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0AA34680114 for ; Sun, 31 Dec 2017 23:27:02 +0200 (EET) Received: by mail-io0-f173.google.com with SMTP id f6so9346838ioh.8 for ; Sun, 31 Dec 2017 13:27:17 -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=r17eMO7fYSdjRmCTXXMDnGE6YOhCFjdaZ2Kt7RRrf1M=; b=eC/P+iQXaU7hcjXiFVoWIuZVbAinWu8KQpLwFaK/HVI5KpeEuP0/PUCxs5jIhhGKIK rO2BHE6CFqCUdizlXvhqpJYUJSYr/xYbRVqCrjLLQite7Ypigp5kTHYSyP++9MP6c3vi Md96E3J5vXXioPlXcXW6q3tMypOsesTxZo5j8UakXBrpr6R0F5frXkXmoMN6tWrIm5mY CRNHI/f7S2O94UsgAk14uAtgKVykhXa6p7ZV9PwLarH5v165abgAvxnUdOVPFXkvlCrf L9D0eW+ccWKTxers0MYdP6F1IbTlMTJCKC1uqAlAZDLqsJXAesvlPPeuZUvElZPFsh/w JsLA== 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=r17eMO7fYSdjRmCTXXMDnGE6YOhCFjdaZ2Kt7RRrf1M=; b=MpyVBMrOY1xhFVf6ivGHQgSgyIE/acvYlgZclytfIkWWuzpQsXAKs/Zzkh0Snii33H 07fHcE0WKSzdisyYoFS8tE3YbjuEWOuCPXELc4aaiC2Djbf0CHnncnl5iUhtnmybiUXj jMuYDCpYLorQ/DPIb5LUw3XQssdFJVe/ARDfaNWXmkQwfHLJ7JzdQ+OJZU8Db8uJ3JhY DqDNMe1iDaDmDfV10S/Qlu0dZnU5xEtY44F1/E2pmbIeefRScryFdRZIMbHpVtTIVSKu EOrR6Hpng95SWhHH/NFfG6naKtN77ZOaWFx8E4uBdOmQl8Cc98/qx5SctezRbE0DIg+2 WNXA== X-Gm-Message-State: AKGB3mJoX5b5khm8G16Ofbap4hgezeU7xyhzHPB5WvTCedKxnshxp9FY Y08Tu3YMzqd9p2hPG1DMe2tR1dqZO+xCP6dH+hw= X-Received: by 10.107.35.208 with SMTP id j199mr5424445ioj.65.1514755635896; Sun, 31 Dec 2017 13:27:15 -0800 (PST) MIME-Version: 1.0 Received: by 10.2.119.211 with HTTP; Sun, 31 Dec 2017 13:26:55 -0800 (PST) In-Reply-To: <20171231211702.GQ4926@michaelspb> References: <20171128203239.GC4636@nb4> <20171231211702.GQ4926@michaelspb> From: Carl Eugen Hoyos Date: Sun, 31 Dec 2017 22:26:55 +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: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. Thank you, Carl Eugen From 42fc4aabccb5b5da0db2fd312187d90b303e96f7 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos Date: Sun, 31 Dec 2017 22:25:29 +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 | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 2064473..ee86c65 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -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 alloc_size = 0, 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