From patchwork Tue Jul 9 09:34:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Niklas Haas X-Patchwork-Id: 50433 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:612c:fad:b0:482:c625:d099 with SMTP id kf13csp245648vqb; Tue, 9 Jul 2024 02:34:53 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCUvwSBMrnPW2BlKjd9jubf3rs2S1gOC7RYZKEwW+pybhUUqpyWtnhm+qEHi5mngXenl5uXW3AWvxK6X5WXX8ROitAFdHEdc+cHfQw== X-Google-Smtp-Source: AGHT+IEygWQf1fu+myEUVdtHJALAuK5cuipmzQhgCmGL1L+h6UUQZh0UHx0f1h0CqCtSe8EGzXrz X-Received: by 2002:a2e:80cd:0:b0:2ee:974c:596f with SMTP id 38308e7fff4ca-2eeb30feba0mr12692841fa.28.1720517693292; Tue, 09 Jul 2024 02:34:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1720517693; cv=none; d=google.com; s=arc-20160816; b=RL/T2djhUT+hmGQsIrPHRS6pIkHzO/n4pCV2TH6+8RO40edN7YaatwxOOqcQr7pEaA HMsITD7WU3SJxDLh/602paIlWqwFkV/XaGrmqWJpC1+f66AdEYLTYThS4oT0c3D+H2sM PL//7QgeG+m54pDxI+7sAJzwcWgsWNrgj4sQzp7KX2OnR+ijszS8m0TcjMfdLZ6zkCbc rasloYfCwaUXVFcwUB9t5YFSzyGIruzKyeuZZp/zCfiSUIKxn11fc/fUNOdJ9fSTfw4h obyxE+6BjCpIoHFEdltgv/0jak2og4RL4W1WaFNz3skBSRXGr17WdWjxCDJJ4JrhOBeo a0Mw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:mime-version:message-id:date:to:from :dkim-signature:delivered-to; bh=+clzqp1IC6SDe+QJyyR6038dLZKFBYqCj2C3GlgVLJM=; fh=xmAeKtysnShNOmkhiJmYkS30uw4Fu2hvBJ7qlIwukxQ=; b=a3UIHbm8gWpa0chLFNj9oVyhVCpsA5A4fJFM+83FN8QT2+wR+4uxd5QOigPqf+ZmI8 SgZkXCP+kHBf367h8hxTq5kQD3DmcI2QUcSMkylb7os7pGSlB7Dev7TcIlXhdhb0qnF0 z5B9jsCPQzqxoDp2QvzNCzeGBGwwTVztSgkn1i5DVECCP24uw0aF3+XZWCp1U74WkXvh 7CN+mQ6c/wc0S5FU8FqTj9JYG5R9DGP48vf8geYC7Dd153kNH5sX6/AyFbepBOXUVuzk tkC0DQwNIas4RnmmcTNF7b/DL9tYik+ZGF+PUEAy/aovnjlMr8eALxg8B0zMouflgT8L bNJg==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@haasn.xyz header.s=mail header.b=KmUObW1+; 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 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id 38308e7fff4ca-2eeb348c2a4si3861521fa.452.2024.07.09.02.34.52; Tue, 09 Jul 2024 02:34:53 -0700 (PDT) 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=@haasn.xyz header.s=mail header.b=KmUObW1+; 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 Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5142468DC2C; Tue, 9 Jul 2024 12:34:49 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7CB8968DB42 for ; Tue, 9 Jul 2024 12:34:41 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1720517680; bh=T7HZH33Jpc7+c/8oU1ZOHidN1I3yj4sMXovPAHbhH2w=; h=From:To:Cc:Subject:Date:From; b=KmUObW1+iwWGdQVAjraqckxTj5QbPrznQWlA4TtGcTYe6HuTmWRfVOmCyraoWJiyj TY2O3p8FqQAUfjqX46WTV2gzl0R5oqq055U5jtyfqVJCWBUIelLfuBbj6lzv4Fi4Zb kY1TjIV3d718kbfhHinuilwWgIvpJahW1M7sBgiU= Received: from localhost (unknown [217.111.52.58]) by haasn.dev (Postfix) with ESMTPSA id 6394C406C7; Tue, 9 Jul 2024 11:34:40 +0200 (CEST) From: Niklas Haas To: ffmpeg-devel@ffmpeg.org Date: Tue, 9 Jul 2024 11:34:36 +0200 Message-ID: <20240709093437.16563-1-ffmpeg@haasn.xyz> X-Mailer: git-send-email 2.44.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_scale: fix frame lifetimes X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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: Niklas Haas Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: /bpheXMgqj1G From: Niklas Haas scale_frame() inconsistently handled the lifetime of `in`. Fixes a possible double free and a possible memory leak. The new code always has `scale_frame` take over ownership of the input frame. I first tried writing this code in a way where the calling code retains ownership, but this is nontrivial due to the presence of the no-op short-circuit condition in which the input frame is directly returned. (As an alternative, we could use av_frame_clone() instead, but I wanted to avoid touching the original behavior in this commit) --- libavfilter/vf_scale.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index 841075193e..a1a322ed9e 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -869,17 +869,20 @@ static int scale_field(ScaleContext *scale, AVFrame *dst, AVFrame *src, return 0; } -static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) +/* Takes over ownership of *frame_in, passes ownership of *frame_out to caller */ +static int scale_frame(AVFilterLink *link, AVFrame **frame_in, + AVFrame **frame_out) { AVFilterContext *ctx = link->dst; ScaleContext *scale = ctx->priv; AVFilterLink *outlink = ctx->outputs[0]; - AVFrame *out; + AVFrame *out, *in = *frame_in; const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format); char buf[32]; int ret; int frame_changed; + *frame_in = NULL; *frame_out = NULL; if (in->colorspace == AVCOL_SPC_YCGCO) av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n"); @@ -922,11 +925,11 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) ret = scale_parse_expr(ctx, NULL, &scale->w_pexpr, "width", scale->w_expr); if (ret < 0) - return ret; + goto err; ret = scale_parse_expr(ctx, NULL, &scale->h_pexpr, "height", scale->h_expr); if (ret < 0) - return ret; + goto err; } if (ctx->filter == &ff_vf_scale2ref) { @@ -957,7 +960,7 @@ FF_ENABLE_DEPRECATION_WARNINGS link->dst->inputs[0]->sample_aspect_ratio.num = in->sample_aspect_ratio.num; if ((ret = config_props(outlink)) < 0) - return ret; + goto err; } scale: @@ -971,10 +974,9 @@ scale: out = ff_get_video_buffer(outlink, outlink->w, outlink->h); if (!out) { - av_frame_free(&in); - return AVERROR(ENOMEM); + ret = AVERROR(ENOMEM); + goto err; } - *frame_out = out; av_frame_copy_props(out, in); out->width = outlink->w; @@ -999,9 +1001,12 @@ scale: ret = sws_scale_frame(scale->sws, out, in); } - av_frame_free(&in); if (ret < 0) - av_frame_free(frame_out); + av_frame_free(&out); + *frame_out = out; + +err: + av_frame_free(&in); return ret; } @@ -1058,7 +1063,7 @@ FF_ENABLE_DEPRECATION_WARNINGS } } - ret = scale_frame(ctx->inputs[0], in, &out); + ret = scale_frame(ctx->inputs[0], &in, &out); if (out) { out->pts = av_rescale_q(fs->pts, fs->time_base, outlink->time_base); return ff_filter_frame(outlink, out); @@ -1077,7 +1082,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) AVFrame *out; int ret; - ret = scale_frame(link, in, &out); + ret = scale_frame(link, &in, &out); if (out) return ff_filter_frame(outlink, out);