From 80bebfb64ec9874762e26b6e779549ea587936a8 Mon Sep 17 00:00:00 2001 From: Marian Cichy Date: Thu, 11 Mar 2021 10:33:25 +0100 Subject: libcamera: gst: Fix double-free when acquire_buffer fails 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 Signed-off-by: Laurent Pinchart Reviewed-by: Nicolas Dufresne --- src/gstreamer/gstlibcamerasrc.cpp | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) (limited to 'src/gstreamer/gstlibcamerasrc.cpp') diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 636c14df..7b967120 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -52,19 +52,18 @@ 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 +73,6 @@ RequestWrap::~RequestWrap() if (item.second) gst_buffer_unref(item.second); } - - delete request_; } void RequestWrap::attachBuffer(GstBuffer *buffer) @@ -164,7 +161,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"); @@ -269,7 +266,18 @@ gst_libcamera_src_task_run(gpointer user_data) GstLibcameraSrcState *state = self->state; std::unique_ptr request = state->cam_->createRequest(); - auto wrap = std::make_unique(request.get()); + if (!request) { + GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT, + ("Failed to allocate request for camera '%s'.", + state->cam_->id().c_str()), + ("libcamera::Camera::createRequest() failed")); + gst_task_stop(self->task); + return; + } + + std::unique_ptr wrap = + std::make_unique(std::move(request)); + for (GstPad *srcpad : state->srcpads_) { GstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad); GstBuffer *buffer; @@ -279,24 +287,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; -- cgit v1.2.1