summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarian Cichy <m.cichy@pengutronix.de>2021-03-11 10:33:25 +0100
committerLaurent Pinchart <laurent.pinchart@ideasonboard.com>2021-03-12 22:38:15 +0200
commit80bebfb64ec9874762e26b6e779549ea587936a8 (patch)
tree0a8978e44df0a421a8f5cf3dddbc8044be53ffa6
parentf5d3fa1254420efb882bbea30db1ff34b8999567 (diff)
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 <m.cichy@pengutronix.de> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
-rw-r--r--src/gstreamer/gstlibcamerasrc.cpp39
1 files changed, 23 insertions, 16 deletions
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> request);
~RequestWrap();
void attachBuffer(GstBuffer *buffer);
GstBuffer *detachBuffer(Stream *stream);
- /* For ptr comparison only. */
- Request *request_;
+ std::unique_ptr<Request> request_;
std::map<Stream *, GstBuffer *> buffers_;
};
-RequestWrap::RequestWrap(Request *request)
- : request_(request)
+RequestWrap::RequestWrap(std::unique_ptr<Request> 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<RequestWrap> 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> request = state->cam_->createRequest();
- auto wrap = std::make_unique<RequestWrap>(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<RequestWrap> wrap =
+ std::make_unique<RequestWrap>(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;