From patchwork Mon Feb 22 09:32:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 25889 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 AAFF544B4B8 for ; Mon, 22 Feb 2021 11:32:49 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5F1C268A328; Mon, 22 Feb 2021 11:32:49 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7B9B36806AF for ; Mon, 22 Feb 2021 11:32:43 +0200 (EET) Received: by mail-wr1-f50.google.com with SMTP id n8so18240722wrm.10 for ; Mon, 22 Feb 2021 01:32:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=ie7URM46hKDb4k/6Sp/yptj7x+nCFJfvaj7i5H3IOTg=; b=J3j55lFpwj36XaFZQ9is0m7IsW96AT2Rk29lKVkmW65L08DyMrNwq8pj79Vv4Hgrkd vUKnlm4sreeuIX0J2NLJ9kvt6kyTwh9AhnEVz+8E9gRIUgDA9Wf/c8w1WmNEjtRUKXNN m/6YNTfEMbce5wZajRZnDcHi2CUoyOiKx3WUmd4XRRunEpyWt4A5BeVUkcU8+KqdMkTZ L1RXAwI+PNPBp5Gde3k8r5cnzkGNzqmquz6qCcuDyVCZacGk/u2i56XQlatPbNkf2MpW u/b6sYoGdVSIjwiHD6LF6G+gSKPSIJ3QcOW2rhXwjpW8PzLvCfOGVfjZgpZkTulwqiod gvYA== 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:mime-version :content-transfer-encoding; bh=ie7URM46hKDb4k/6Sp/yptj7x+nCFJfvaj7i5H3IOTg=; b=QmSJ7SvEwRVQDMwAXcWlaOJa4EDPcobcfeBH7bBUO6Mc7vz9mzeYpv8CLSr8RkFmeU woN0iCrV10nq0aXajk1TnWzVK/EhAnOn6vyPd/84FUsLdLi9oJIAo92A4Hr+qqAI6xMU +gepDUbrhoHRqtYEZiuA6m9WlQrakc+SKJQmnVFkPRZFKwhclPJ0lQQOPZSKucmbRDmK Z/huNa7L9l/lIxzlZMgIlpj4IWQzrQig0yayd4qcTiolKzlmvRrVbiY8Dgm6qCCwOb4e cP1Qz0pNiR1jQPSkHRxFQAaq6PDm7+9bzWegNSK6RZuN2gG6TDhgaJ6kCpM+vj3M2vXh yp0A== X-Gm-Message-State: AOAM533l7M6LkGggsjcY7ljDf0SpWM+eoLvZQ0a4kG07B7fdMRMFjCMl X4mD2fHBPvkU5uGwv1/M8jd8FavL1uA= X-Google-Smtp-Source: ABdhPJx1WawAlN7ZIx4QvRoBCea0e0Bs7h3qhiUJPNjPNqQeL/U/INKPaNNV1PivvHhJP9RFRi88zw== X-Received: by 2002:adf:ea01:: with SMTP id q1mr7979489wrm.14.1613986362783; Mon, 22 Feb 2021 01:32:42 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id d16sm5284924wrx.79.2021.02.22.01.32.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 22 Feb 2021 01:32:42 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 22 Feb 2021 10:32:32 +0100 Message-Id: <20210222093234.212078-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/3] avformat/wavenc: Fix leak and segfault on reallocation error 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" Up until now, the wav muxer used a reallocation of the form ptr = av_realloc(ptr, size); that leaks upon error. Furthermore, if a failed reallocation happened when writing the trailer, a segfault would occur due to avio_write(NULL, size) because the muxer only prints an error message upon allocation error, but does not return the error. Moreover setting the pointer to the buffer to NULL on error seems to be done on purpose in order to record that an error has occured so that outputting the peak values is no longer attempted. This behaviour has been retained by simply disabling whether peak data should be written if an error occurs. Finally, the reallocation is now done once per peak block and not once per peak block per channel; it is also done with av_fast_realloc and not with a linear size increase. Signed-off-by: Andreas Rheinhardt --- libavformat/wavenc.c | 53 ++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/libavformat/wavenc.c b/libavformat/wavenc.c index 1027f107ee..9ef72f6e1c 100644 --- a/libavformat/wavenc.c +++ b/libavformat/wavenc.c @@ -50,8 +50,6 @@ #define RF64_NEVER 0 #define RF64_ALWAYS 1 -#define PEAK_BUFFER_SIZE 1024 - typedef enum { PEAK_OFF = 0, PEAK_ON, @@ -72,8 +70,9 @@ typedef struct WAVMuxContext { int64_t maxpts; int16_t *peak_maxpos, *peak_maxneg; uint32_t peak_num_frames; - uint32_t peak_outbuf_size; + unsigned peak_outbuf_size; uint32_t peak_outbuf_bytes; + unsigned size_increment; uint8_t *peak_output; int last_duration; int write_bext; @@ -172,15 +171,15 @@ static av_cold int peak_init_writer(AVFormatContext *s) "Writing 16 bit peak for 8 bit audio does not make sense\n"); return AVERROR(EINVAL); } + if (par->channels > INT_MAX / (wav->peak_bps * wav->peak_ppv)) + return AVERROR(ERANGE); + wav->size_increment = par->channels * wav->peak_bps * wav->peak_ppv; wav->peak_maxpos = av_mallocz_array(par->channels, sizeof(*wav->peak_maxpos)); wav->peak_maxneg = av_mallocz_array(par->channels, sizeof(*wav->peak_maxneg)); - wav->peak_output = av_malloc(PEAK_BUFFER_SIZE); - if (!wav->peak_maxpos || !wav->peak_maxneg || !wav->peak_output) + if (!wav->peak_maxpos || !wav->peak_maxneg) goto nomem; - wav->peak_outbuf_size = PEAK_BUFFER_SIZE; - return 0; nomem: @@ -188,14 +187,24 @@ nomem: return AVERROR(ENOMEM); } -static void peak_write_frame(AVFormatContext *s) +static int peak_write_frame(AVFormatContext *s) { WAVMuxContext *wav = s->priv_data; AVCodecParameters *par = s->streams[0]->codecpar; + unsigned new_size = wav->peak_outbuf_bytes + wav->size_increment; + uint8_t *tmp; int c; - if (!wav->peak_output) - return; + if (new_size > INT_MAX) { + wav->write_peak = PEAK_OFF; + return AVERROR(ERANGE); + } + tmp = av_fast_realloc(wav->peak_output, &wav->peak_outbuf_size, new_size); + if (!tmp) { + wav->write_peak = PEAK_OFF; + return AVERROR(ENOMEM); + } + wav->peak_output = tmp; for (c = 0; c < par->channels; c++) { wav->peak_maxneg[c] = -wav->peak_maxneg[c]; @@ -209,17 +218,6 @@ static void peak_write_frame(AVFormatContext *s) wav->peak_maxpos[c] = FFMAX(wav->peak_maxpos[c], wav->peak_maxneg[c]); - if (wav->peak_outbuf_size - wav->peak_outbuf_bytes < - wav->peak_format * wav->peak_ppv) { - wav->peak_outbuf_size += PEAK_BUFFER_SIZE; - wav->peak_output = av_realloc(wav->peak_output, - wav->peak_outbuf_size); - if (!wav->peak_output) { - av_log(s, AV_LOG_ERROR, "No memory for peak data\n"); - return; - } - } - if (wav->peak_format == PEAK_FORMAT_UINT8) { wav->peak_output[wav->peak_outbuf_bytes++] = wav->peak_maxpos[c]; @@ -241,6 +239,8 @@ static void peak_write_frame(AVFormatContext *s) wav->peak_maxneg[c] = 0; } wav->peak_num_frames++; + + return 0; } static int peak_write_chunk(AVFormatContext *s) @@ -254,8 +254,11 @@ static int peak_write_chunk(AVFormatContext *s) char timestamp[28]; /* Peak frame of incomplete block at end */ - if (wav->peak_block_pos) - peak_write_frame(s); + if (wav->peak_block_pos) { + int ret = peak_write_frame(s); + if (ret < 0) + return ret; + } memset(timestamp, 0, sizeof(timestamp)); if (!(s->flags & AVFMT_FLAG_BITEXACT)) { @@ -386,7 +389,9 @@ static int wav_write_packet(AVFormatContext *s, AVPacket *pkt) if (++c == s->streams[0]->codecpar->channels) { c = 0; if (++wav->peak_block_pos == wav->peak_block_size) { - peak_write_frame(s); + int ret = peak_write_frame(s); + if (ret < 0) + return ret; wav->peak_block_pos = 0; } }