From patchwork Sat Mar 20 07:08:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 26500 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 8FAFB44B872 for ; Sat, 20 Mar 2021 09:08:19 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 50CE768A82B; Sat, 20 Mar 2021 09:08:19 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BA2D8687FA1 for ; Sat, 20 Mar 2021 09:08:12 +0200 (EET) Received: by mail-wm1-f53.google.com with SMTP id u5-20020a7bcb050000b029010e9316b9d5so6330438wmj.2 for ; Sat, 20 Mar 2021 00:08:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:reply-to:mime-version :content-transfer-encoding; bh=Te5NviRx3g5wX6qPVoRISi5zWaTftueBFxdsAvnMkpA=; b=IrNLEFY60ZVHh4yZL3sBuqCMGkwPokxV9Q3KN1pRtvLay4v/oJP3+ZL9J42iU0d2l4 OUNfKxDGSdBg8iAt3SNtcdMFNmpwlvYJEoVxUYgHGewP7LkisdkkWqgq3jQP3dkK4E6u yXb7y19MuSZ3YDz8RAJKjXd90u1D9I/BTZg+jFfcJCFF5LkH1WbhTTWjwBJ4WNGDezQV pgEKmue3/FC1g8SfO9tOTIPwQngcfM08Zq+i1kepClocLl8JCOiuXfPa43D5m6Qplh7z xEaEaa4vhQ6w9OE+/U7k1UE6SsiuILYiKgFGxlvKJwmQx3oZkEiHGLY7non6zzJvNxYD anmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :mime-version:content-transfer-encoding; bh=Te5NviRx3g5wX6qPVoRISi5zWaTftueBFxdsAvnMkpA=; b=UsTDueSNEz6hTiVDerK96hgzXI9hyc+Y8f8x095RAOxKEd2o36GLe4pYHgXkVmz12r /QZ2t1hnrm42vY3mZdXC5KgK8NXNNHWGtzCfwHo2Uf4Q5htKHUkIPTytrE4mQrvrW1jm dPUnTLPOGCu8bY0U89j2RnsSXqrrZpZiJJXtO6R+VjMDP2XVeOSs6RY/VJsIcQYEg60h dJ8ALIOnnDMbIeSkC06Lv/wQur8vTFrCJn0bIfineSxInxbhuBUBYreBSSmRrv1KOQFS F0mtXauW3tJ05ersDjnEloMsqUdZmsQ+mtdHBskiypIDT7j5SKHbJCOTrEphxTD+s+Ow xOUQ== X-Gm-Message-State: AOAM530c6UrYnYtrTJTtPaUePpZYfPLTJ8vZEK/V5t2lGpjwqvHdwmcV rNQ6vnz+CD8CKdlAd2BBB7Dg1Px2V+sZZg== X-Google-Smtp-Source: ABdhPJzUo7KUaNDlFPN5qFPM/kvHTy+GizD8AMS9ZBbJZSAhH9WO+zUer+PgRRuVdLqU/X15jd5rWw== X-Received: by 2002:a05:600c:9:: with SMTP id g9mr6888147wmc.134.1616224091715; Sat, 20 Mar 2021 00:08:11 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08960.dynamic.kabel-deutschland.de. [188.192.137.96]) by smtp.gmail.com with ESMTPSA id v18sm11816022wrf.41.2021.03.20.00.08.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 20 Mar 2021 00:08:11 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 20 Mar 2021 08:08:01 +0100 Message-Id: <20210320070801.94890-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avformat/pp_bnk: Fix memleaks when reading non-stereo tracks 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 Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Commit 6973df112275c8ea4af0bf3cb1338baecc1d06b3 added support for music tracks by outputting its two containing tracks together in one packet. But the actual data is not contiguous in the file and therefore one can't simply use av_get_packet() (which has been used before) for it. Therefore the packet was now allocated via av_new_packet() and read via avio_read(); and this is also for non-music files. This causes problems because one can now longer rely on things done automatically by av_get_packet(): It automatically freed the packet in case of errors; this lead to memleaks in several FATE-tests covering this demuxer. Furthermore, in case the data read is less than the data desired, the returned packet was not zero-allocated (the packet's padding was uninitialized); for music files the actual data could even be uninitialized. The former problems are fixed by using av_get_packet() for non-music files; the latter problem is handled by erroring out unless both tracks could be fully read. Signed-off-by: Andreas Rheinhardt --- libavformat/pp_bnk.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/libavformat/pp_bnk.c b/libavformat/pp_bnk.c index 4aa77c1c9d..5c89295d27 100644 --- a/libavformat/pp_bnk.c +++ b/libavformat/pp_bnk.c @@ -265,28 +265,24 @@ static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt) size = FFMIN(trk->data_size - trk->bytes_read, PP_BNK_MAX_READ_SIZE); - if (!ctx->is_music) - ret = av_new_packet(pkt, size); - else if (ctx->current_track == 0) - ret = av_new_packet(pkt, size * 2); - else - ret = 0; - - if (ret < 0) - return ret; - - if (ctx->is_music) + if (!ctx->is_music) { + ret = av_get_packet(s->pb, pkt, size); + if (ret == AVERROR_EOF) { + /* If we've hit EOF, don't attempt this track again. */ + trk->data_size = trk->bytes_read; + continue; + } + } else { + if (!pkt->data && (ret = av_new_packet(pkt, size * 2)) < 0) + return ret; ret = avio_read(s->pb, pkt->data + size * ctx->current_track, size); - else - ret = avio_read(s->pb, pkt->data, size); - - if (ret == AVERROR_EOF) { - /* If we've hit EOF, don't attempt this track again. */ - trk->data_size = trk->bytes_read; - continue; - } else if (ret < 0) { - return ret; + if (ret >= 0 && ret != size) { + /* Only return stereo packets if both tracks could be read. */ + ret = AVERROR_EOF; + } } + if (ret < 0) + return ret; trk->bytes_read += ret; pkt->flags &= ~AV_PKT_FLAG_CORRUPT; @@ -298,8 +294,6 @@ static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt) continue; pkt->stream_index = 0; - } else { - pkt->size = ret; } ctx->current_track++;