From patchwork Tue Feb 14 02:04:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aman Karmani X-Patchwork-Id: 2544 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp1210959vsb; Mon, 13 Feb 2017 18:10:35 -0800 (PST) X-Received: by 10.223.149.39 with SMTP id 36mr22215905wrs.125.1487038234975; Mon, 13 Feb 2017 18:10:34 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id w51si16052362wrb.76.2017.02.13.18.10.34; Mon, 13 Feb 2017 18:10:34 -0800 (PST) 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=@tmm1-net.20150623.gappssmtp.com; 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 6C91F68974E; Tue, 14 Feb 2017 04:10:26 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ot0-f194.google.com (mail-ot0-f194.google.com [74.125.82.194]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 771D6687ED3 for ; Tue, 14 Feb 2017 04:10:20 +0200 (EET) Received: by mail-ot0-f194.google.com with SMTP id 65so14502204otq.2 for ; Mon, 13 Feb 2017 18:10:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tmm1-net.20150623.gappssmtp.com; s=20150623; h=sender:from:to:cc:subject:date:message-id; bh=xUHURL0d7yo88KenFHLi7QdOkxnOAwyxqzrY3jBEEQ0=; b=ujSPFbLfOaRazsJCqG+zbmAHlQenEktMm9pJOwr0Ufm00Yb0ga3CkmVx/xwRtkWcHR neLBMTnPMygawD/KpOdmIvd3mTxTfPGohk0cQi0p6Qh2fiK3+O63zhp0ycDPOQB0xqyR Kn4YzAULbD0xwa/dAIz8xFKTgBRaNrn6wAlxedq4nYDYlTwMXczRqmE0mYCEyu4IDj1R i2SEw7MqPoYAirrt9gTadim6Uaem4xVXoUK9qcf+V6xjIBIXLvxg8H7sKHyyRA9zbeU5 /9KCsOYPIR5GNuNVKp0OD1Q6+Al+yQzLgzQwefMOPI3MZbG+LD0pTY5/2GjyY4Tk5ajX jYOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id; bh=xUHURL0d7yo88KenFHLi7QdOkxnOAwyxqzrY3jBEEQ0=; b=ljK1l7bcFCWjMhNbNzt4NOi2VFuuN7DmZGHmjA4W9tfuEdyPei+opWLEg/Pzu+il6y /rqoT+dUFG5srSLnXTEq4IZHXH4Xv8swJCCD4FUrjuGZPwk57XYVmv7tZPo9Oty0Bvrm yqI0Z0OxzGgydli63ETHmnhfwx5PcTH86Z59G8GjhA8KejrAIYWjQEoxlX23X84Bc5PU 48507tJBz39FbKVXsVL9tb+zCpfD2SV94d0239FFjkppecsr4ulztdgNEQeq7sVd7bc1 yBV+ZQVBI9R53kAQ2c/E3t0C2SU1xlPSENj2CcyAvKtjou2A9cTTrujyxjPqeIX+Ii/u cnVg== X-Gm-Message-State: AMke39n1iiSn6yCwXzVyA/EgOpH+rETmPKKmXjGXIo0A/qj/dj4r3zePUtCjKz7JkgGbhg== X-Received: by 10.98.70.12 with SMTP id t12mr29515223pfa.47.1487037855473; Mon, 13 Feb 2017 18:04:15 -0800 (PST) Received: from tmm1-macbook.local.net (c-73-252-174-83.hsd1.ca.comcast.net. [73.252.174.83]) by smtp.gmail.com with ESMTPSA id d68sm23105493pfj.92.2017.02.13.18.04.14 (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 13 Feb 2017 18:04:14 -0800 (PST) From: Aman Gupta To: ffmpeg-devel@ffmpeg.org Date: Mon, 13 Feb 2017 18:04:10 -0800 Message-Id: <20170214020410.6054-1-ffmpeg@tmm1.net> X-Mailer: git-send-email 2.10.1 (Apple Git-78) Subject: [FFmpeg-devel] [PATCH] avcodec/h264, videotoolbox: fix use-after-free of AVFrame buffer when VT decoder fails 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: Aman Gupta MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" From: Aman Gupta The videotoolbox hwaccel returns a dummy frame with a 1-byte buffer from alloc_frame(). In end_frame(), this buffer is replaced with the actual decoded data from VTDecompressionSessionDecodeFrame(). This is hacky, but works well enough, as long as the VT decoder returns a valid frame on every end_frame() call. In the case of errors, things get more interesting. Before 9747219958060d8c4f697df62e7f172c2a77e6c7, the dummy 1-byte frame would accidentally be returned all the way up to the user. After that commit, the dummy frame was properly unref'd so the user received an error. However, since that commit, VT hwaccel failures started causing random segfaults in the h264 decoder. This happened more often on iOS where the VT implementation is more likely to throw errors on bitstream anomolies. A recent report of this issue can be see in http://ffmpeg.org/pipermail/libav-user/2016-November/009831.html The root cause here is that between the calls to alloc_frame() and end_frame(), the h264 decoder will reference the dummy 1-byte frame in its ref_list. When the end_frame() call fails, the dummy frame is unref'd but still referenced in the h264 decoder. This eventually causes a NULL deference and segmentation fault. This patch fixes the issue by properly discarding references to the dummy frame in the H264Context after the frame has been unref'd. --- libavcodec/h264_picture.c | 3 +++ libavcodec/h264_refs.c | 20 ++++++++++++++++++-- libavcodec/h264dec.h | 1 + 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c index f634d2a..702ca12 100644 --- a/libavcodec/h264_picture.c +++ b/libavcodec/h264_picture.c @@ -177,6 +177,9 @@ int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup) if (err < 0) av_log(avctx, AV_LOG_ERROR, "hardware accelerator failed to decode picture\n"); + + if (err < 0 && avctx->hwaccel->pix_fmt == AV_PIX_FMT_VIDEOTOOLBOX) + ff_h264_remove_cur_pic_ref(h); } #if FF_API_CAP_VDPAU diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c index 97bf588..9c77bc7 100644 --- a/libavcodec/h264_refs.c +++ b/libavcodec/h264_refs.c @@ -560,6 +560,23 @@ static H264Picture *remove_long(H264Context *h, int i, int ref_mask) return pic; } +void ff_h264_remove_cur_pic_ref(H264Context *h) +{ + int j; + + if (h->short_ref[0] == h->cur_pic_ptr) { + remove_short_at_index(h, 0); + } + + if (h->cur_pic_ptr->long_ref) { + for (j = 0; j < FF_ARRAY_ELEMS(h->long_ref); j++) { + if (h->long_ref[j] == h->cur_pic_ptr) { + remove_long(h, j, 0); + } + } + } +} + void ff_h264_remove_all_refs(H264Context *h) { int i; @@ -571,8 +588,7 @@ void ff_h264_remove_all_refs(H264Context *h) if (h->short_ref_count && !h->last_pic_for_ec.f->data[0]) { ff_h264_unref_picture(h, &h->last_pic_for_ec); - if (h->short_ref[0]->f->buf[0]) - ff_h264_ref_picture(h, &h->last_pic_for_ec, h->short_ref[0]); + ff_h264_ref_picture(h, &h->last_pic_for_ec, h->short_ref[0]); } for (i = 0; i < h->short_ref_count; i++) { diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h index 5f868b7..063afed 100644 --- a/libavcodec/h264dec.h +++ b/libavcodec/h264dec.h @@ -569,6 +569,7 @@ int ff_h264_alloc_tables(H264Context *h); int ff_h264_decode_ref_pic_list_reordering(H264SliceContext *sl, void *logctx); int ff_h264_build_ref_list(H264Context *h, H264SliceContext *sl); void ff_h264_remove_all_refs(H264Context *h); +void ff_h264_remove_cur_pic_ref(H264Context *h); /** * Execute the reference picture marking (memory management control operations).