From patchwork Sat May 9 17:35:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andriy Gelman X-Patchwork-Id: 19581 X-Patchwork-Delegate: andriy.gelman@gmail.com 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 2B1AE44B519 for ; Sat, 9 May 2020 20:36:32 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0A3986881C6; Sat, 9 May 2020 20:36:32 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qv1-f68.google.com (mail-qv1-f68.google.com [209.85.219.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 02ABC687F43 for ; Sat, 9 May 2020 20:36:24 +0300 (EEST) Received: by mail-qv1-f68.google.com with SMTP id fb4so2328353qvb.7 for ; Sat, 09 May 2020 10:36:24 -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=OexGvVFkaybsJJ23sF/Q2VNZ3CrjBaXPA+qmqPFFyuI=; b=smzTPIBaepZpgXBiPsTJ/aBqrrIpF4zGOxE8UKhVsWCE2dQAOd+C23EbMMaYSisKiq +KFeBDZDcuABIqsVFPanJPDDJ9WKYb7FX1v/AeskXoL64d9hfEgLPFYRbtLd3qNeymx9 CoPlWWML8KTqTFnmbQxZdqKk60gTKACaHwOpnVGNzboxXasjaAU17E9HFZV/O/oJYbWV QWF29fn3RZskl3dyn9kwJv5khLPI8SEVxfdVh6ZxxvsFQ4hedgfGZ2+jJbOOdngKcuaI 71xXVmjxplQ6sUEwWz4kxFGDTgh8ImbM5YVziOrw8B8jeIZiqhIkbN6rFzorEMCsVnmx DoxQ== 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=OexGvVFkaybsJJ23sF/Q2VNZ3CrjBaXPA+qmqPFFyuI=; b=dQu9fZriU//Qp+wk5uMg/reQh/6f1pBHOyKOxHTsUlHfY7udfc9xHEo6Auw/SX0SDN c9bD5IIiamVDOGcc6fFnq/+x9QvXweqfOO1bxN/SRDbNO0qEW2ZoFmIQGzBVN9E7rmn2 Gm79OmK5ytP4usbhIV0qRRJsFmxz3QX9CMEAkxwlgP/8uu76X6fGk29TiNmuKoxI0FyB +s08hGdramfsSXK79l3smNhGJeGBrFiWhQuNGOoh6hmXP1y4fPhUeogYOagO1VGlyMvj JfMiK+rmt+dDFBAyNLYkVdL1RDOnakQZvGw+JEhhchyHkXJ0d0P5Ysleji2oIpNzhWzg k41w== X-Gm-Message-State: AGi0PuZXgZ98jrEO7wCI8+obusEr8Ak9XqhAb5CwO8Zx79HtvhN/vm30 sSq7qyBniZvMZmmDNQ3JVpFqYQm9 X-Google-Smtp-Source: APiQypJw8MyzphrnEKMxN4Mwrz/3RllUukPd7x6ZWfaMw/VE2FaoNAkthCM4ZZtTiwOb+k//NsZNrg== X-Received: by 2002:ad4:58c4:: with SMTP id dh4mr804965qvb.157.1589045783272; Sat, 09 May 2020 10:36:23 -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 n9sm4040420qkn.10.2020.05.09.10.36.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 May 2020 10:36:22 -0700 (PDT) From: Andriy Gelman X-Google-Original-From: Andriy Gelman To: ffmpeg-devel@ffmpeg.org Date: Sat, 9 May 2020 13:35:41 -0400 Message-Id: <20200509173541.7667-1-andriy.gelman@gmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v3] 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 --- Sorry, forgot to squash the commit from v1 so v2 didn't apply. This is correct version. Supersedes: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200505055454.28683-1-andriy.gelman@gmail.com/ libavcodec/v4l2_m2m_dec.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c index 3e17e0fcac..b038efed9c 100644 --- a/libavcodec/v4l2_m2m_dec.c +++ b/libavcodec/v4l2_m2m_dec.c @@ -138,14 +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) { - avpkt = s->buf_pkt; - memset(&s->buf_pkt, 0, sizeof(AVPacket)); - } 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; } @@ -153,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)