From patchwork Sat May 9 16:50:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andriy Gelman X-Patchwork-Id: 19580 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 5E47744A831 for ; Sat, 9 May 2020 20:16:10 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 29B44688291; Sat, 9 May 2020 20:16:10 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-pf1-f194.google.com (mail-pf1-f194.google.com [209.85.210.194]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0F74E6809FF for ; Sat, 9 May 2020 20:16:03 +0300 (EEST) Received: by mail-pf1-f194.google.com with SMTP id x77so2623897pfc.0 for ; Sat, 09 May 2020 10:16:02 -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:mime-version :content-transfer-encoding; bh=Ss7cnqiEjH5xFQdj6uEt1GkMxMt+x3Z+iNf4dr7Sz30=; b=Qp6xNgMTA5Bb6tEf9Qn+mDQsNlz1XJFrFjTAUAeAjoyc3gfI9eVMVLdobhiLb7JM+j Jp+UVTXwn8QvaaBO6xi7S7byP4610IPp6rIHRAzd4kldlEzwAfF9SQXDd68Wsp4mQPMB x/e+wFcZbHKVoVU9bSDF2/sEdcopb7Yo+JJbRgvnB7ptxmUYjNgZMhI/9pMM3Dk82uoF a/TevT7K1BOszoC+5Fo90rNSgWs7zFr3BEeXYvEMKBFt4AE6aCQGcqok3o+/rau/VFRH lwEdyWUScciCUwygV7Ne5kOYXmEQimWKPQXxlKeGSnVyCUxFov8kUJX52nC/cKqMZl/U Yzvw== 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=Ss7cnqiEjH5xFQdj6uEt1GkMxMt+x3Z+iNf4dr7Sz30=; b=hAf5AnXn7X03LcxI9Z2kmIZjlvcrJMC0xn8GLXev/PlijD0q6gH/EwNOEB1Hm/C6S+ UEWKcWeWNT+QQAbVrZH1eJBOdeTQMF/VlscVMRcgDx77AXE/CFzurq4z/saeskGjQykq 1S6/Mu1p7tuFLU4PIJgg2QnNvXKb7RgvUQE4DdYxYn/xzXL/3hVjK0OxxCo74vZHzIix UbzH/haN9qrYICzFdMWOAJ1E0dzkJhampM/HzzjVR1OBtdqPGdU1AWrrMEfi1dEZnZbJ LdZGmPpqZpBeiBnEv0XMf5v/oQd/4fhv+Kw7K+GctvxILRdi7ViHA3Gs423+yJiddrAD JKIg== X-Gm-Message-State: AGi0Puakh/IgHAJ2SOd4UH9HcNhlQQ4R8t9tEzE94tSHI1Mu9KVc6VAF ME40Vd4nk9nSInKSqFdT3oj6IXHM X-Google-Smtp-Source: APiQypJaGjgX1KJl+rLNF1xezqw0+vxynNnMAuRnT37zAsX7YLgUFg5Vu3rx4H2jO5rDoeKrGm/IuQ== X-Received: by 2002:a05:6214:17ca:: with SMTP id cu10mr7994973qvb.166.1589043046115; Sat, 09 May 2020 09:50:46 -0700 (PDT) Received: from localhost.localdomain (c-71-232-27-28.hsd1.ma.comcast.net. [71.232.27.28]) by smtp.gmail.com with ESMTPSA id 10sm5002476qtp.4.2020.05.09.09.50.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 May 2020 09:50:45 -0700 (PDT) From: Andriy Gelman X-Google-Original-From: Andriy Gelman To: ffmpeg-devel@ffmpeg.org Date: Sat, 9 May 2020 12:50:36 -0400 Message-Id: <20200509165036.30029-1-andriy.gelman@gmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2] avcodec/v4l2_m2m_dec: Remove redundant packet and fix double free 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: Andriy Gelman Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" From: Andriy Gelman v4l2_receive_frame() uses two packets s->buf_pkt and avpkt. If avpkt cannot be enqueued, the packet is buffered in s->buf_pkt and enqueued in the next call. Currently the ownership transfer between the two packets is not properly handled. A double free occurs if ff_v4l2_context_enqueue_packet() returns EAGAIN and v4l2_try_start returns EINVAL. In fact, having two AVPackets is not needed and everything can be handled by s->buf_pkt. This commit removes the local avpkt from v4l2_receive_frame(), meaning that the ownership transfer doesn't need to be handled and the double free is fixed. Signed-off-by: Andriy Gelman --- Supersedes https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200505055454.28683-1-andriy.gelman@gmail.com/ libavcodec/v4l2_m2m_dec.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c index f1ad1aa2a2..b038efed9c 100644 --- a/libavcodec/v4l2_m2m_dec.c +++ b/libavcodec/v4l2_m2m_dec.c @@ -138,13 +138,10 @@ static int v4l2_receive_frame(AVCodecContext *avctx, AVFrame *frame) V4L2m2mContext *s = ((V4L2m2mPriv*)avctx->priv_data)->context; V4L2Context *const capture = &s->capture; V4L2Context *const output = &s->output; - AVPacket avpkt = {0}; int ret; - if (s->buf_pkt.size) { - av_packet_move_ref(&avpkt, &s->buf_pkt); - } else { - ret = ff_decode_get_packet(avctx, &avpkt); + if (!s->buf_pkt.size) { + ret = ff_decode_get_packet(avctx, &s->buf_pkt); if (ret < 0 && ret != AVERROR_EOF) return ret; } @@ -152,32 +149,29 @@ static int v4l2_receive_frame(AVCodecContext *avctx, AVFrame *frame) if (s->draining) goto dequeue; - ret = ff_v4l2_context_enqueue_packet(output, &avpkt); - if (ret < 0) { - if (ret != AVERROR(EAGAIN)) - return ret; + ret = ff_v4l2_context_enqueue_packet(output, &s->buf_pkt); + if (ret < 0 && ret != AVERROR(EAGAIN)) + goto fail; - s->buf_pkt = avpkt; - /* no input buffers available, continue dequeing */ - } + /* if EAGAIN don't unref packet and try to enqueue in the next iteration */ + if (ret != AVERROR(EAGAIN)) + av_packet_unref(&s->buf_pkt); - if (avpkt.size) { + if (!s->draining) { ret = v4l2_try_start(avctx); if (ret) { - av_packet_unref(&avpkt); - /* cant recover */ - if (ret == AVERROR(ENOMEM)) - return ret; - - return 0; + if (ret != AVERROR(ENOMEM)) + ret = 0; + goto fail; } } dequeue: - if (!s->buf_pkt.size) - av_packet_unref(&avpkt); return ff_v4l2_context_dequeue_frame(capture, frame, -1); +fail: + av_packet_unref(&s->buf_pkt); + return ret; } static av_cold int v4l2_decode_init(AVCodecContext *avctx)