From patchwork Thu Mar 11 09:33:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marian Cichy X-Patchwork-Id: 11547 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B4FC4BD1F1 for ; Thu, 11 Mar 2021 09:33:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E1E9568C69; Thu, 11 Mar 2021 10:33:38 +0100 (CET) Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 063C868AA3 for ; Thu, 11 Mar 2021 10:33:37 +0100 (CET) Received: from dude02.hi.pengutronix.de ([2001:67c:670:100:1d::28]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lKHhI-00067z-Bm; Thu, 11 Mar 2021 10:33:36 +0100 Received: from mci by dude02.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1lKHhH-0002Ll-Ug; Thu, 11 Mar 2021 10:33:35 +0100 From: Marian Cichy To: libcamera-devel@lists.libcamera.org Date: Thu, 11 Mar 2021 10:33:25 +0100 Message-Id: <20210311093325.8933-1-m.cichy@pengutronix.de> X-Mailer: git-send-email 2.29.2 MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::28 X-SA-Exim-Mail-From: mci@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH v2] libcamera: gst: Fix double-free when acquire_buffer fails X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: graphics@pengutronix.de, Marian Cichy Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" If gst_buffer_pool_acquire_buffer in gst_libcamera_task_run fails, the unique_ptr to the request-object gets reset and hence, its destructor is called. However, the wrap-object points to the same object and is still alive at this moment. When the task_run-function is finished, the destructor of the wrap-object is called, which in return calls the destructor of the request-object again. Instead of taking care of both, the request and the wrap-object, we can move the request to the wrap which will then effectively take care of the request object automatically. Signed-off-by: Marian Cichy Suggested-by: Laurent Pinchart Reviewed-by: Nicolas Dufresne --- src/gstreamer/gstlibcamerasrc.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 636c14df..e86c3d7f 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -52,19 +52,19 @@ GST_DEBUG_CATEGORY_STATIC(source_debug); #define GST_CAT_DEFAULT source_debug struct RequestWrap { - RequestWrap(Request *request); + RequestWrap(std::unique_ptr request); ~RequestWrap(); void attachBuffer(GstBuffer *buffer); GstBuffer *detachBuffer(Stream *stream); /* For ptr comparison only. */ - Request *request_; + std::unique_ptr request_; std::map buffers_; }; -RequestWrap::RequestWrap(Request *request) - : request_(request) +RequestWrap::RequestWrap(std::unique_ptr request) + : request_(std::move(request)) { } @@ -74,8 +74,6 @@ RequestWrap::~RequestWrap() if (item.second) gst_buffer_unref(item.second); } - - delete request_; } void RequestWrap::attachBuffer(GstBuffer *buffer) @@ -164,7 +162,7 @@ GstLibcameraSrcState::requestCompleted(Request *request) std::unique_ptr wrap = std::move(requests_.front()); requests_.pop(); - g_return_if_fail(wrap->request_ == request); + g_return_if_fail(wrap->request_.get() == request); if ((request->status() == Request::RequestCancelled)) { GST_DEBUG_OBJECT(src_, "Request was cancelled"); @@ -268,8 +266,7 @@ gst_libcamera_src_task_run(gpointer user_data) GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data); GstLibcameraSrcState *state = self->state; - std::unique_ptr request = state->cam_->createRequest(); - auto wrap = std::make_unique(request.get()); + auto wrap = std::make_unique(state->cam_->createRequest()); for (GstPad *srcpad : state->srcpads_) { GstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad); GstBuffer *buffer; @@ -279,24 +276,23 @@ gst_libcamera_src_task_run(gpointer user_data) &buffer, nullptr); if (ret != GST_FLOW_OK) { /* - * RequestWrap does not take ownership, and we won't be - * queueing this one due to lack of buffers. + * RequestWrap has ownership of the rquest, and we + * won't be queueing this one due to lack of buffers. */ - request.reset(); + wrap.release(); break; } wrap->attachBuffer(buffer); } - if (request) { + if (wrap) { GLibLocker lock(GST_OBJECT(self)); GST_TRACE_OBJECT(self, "Requesting buffers"); - state->cam_->queueRequest(request.get()); + state->cam_->queueRequest(wrap->request_.get()); state->requests_.push(std::move(wrap)); - /* The request will be deleted in the completion handler. */ - request.release(); + /* The RequestWrap will be deleted in the completion handler. */ } GstFlowReturn ret = GST_FLOW_OK;