cam: kms_sink: Make lifetime management of DRM request safer

The drmRequest is KMSSink::processRequest() is created as a naked
pointer, passed to the constructor of the KMSSink::Request object that
stores it in a std::unique_ptr<>, and used later in the function. The
current implementation is safe, but could be prone to both memory leaks
and use-after-free bugs if modified. Improve it by replacing the naked
pointer with a std::unique_ptr<>.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Eric Curtin <ecurtin@redhat.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

src/cam/kms_sink.cpp

@@ -301,7 +301,8 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
 	DRM::FrameBuffer *drmBuffer = iter->second.get();
 
 	unsigned int flags = DRM::AtomicRequest::FlagAsync;
-	DRM::AtomicRequest *drmRequest = new DRM::AtomicRequest(&dev_);
+	std::unique_ptr<DRM::AtomicRequest> drmRequest =
+		std::make_unique<DRM::AtomicRequest>(&dev_);
 	drmRequest->addProperty(plane_, "FB_ID", drmBuffer->id());
 
 	if (!active_ && !queued_) {
@@ -324,12 +325,12 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
 		flags |= DRM::AtomicRequest::FlagAllowModeset;
 	}
 
-	pending_ = std::make_unique<Request>(drmRequest, camRequest);
+	pending_ = std::make_unique<Request>(std::move(drmRequest), camRequest);
 
 	std::lock_guard<std::mutex> lock(lock_);
 
 	if (!queued_) {
-		int ret = drmRequest->commit(flags);
+		int ret = pending_->drmRequest_->commit(flags);
 		if (ret < 0) {
 			std::cerr
 				<< "Failed to commit atomic request: "

src/cam/kms_sink.h

@@ -38,8 +38,9 @@ private:
 	class Request
 	{
 	public:
-		Request(DRM::AtomicRequest *drmRequest, libcamera::Request *camRequest)
-			: drmRequest_(drmRequest), camRequest_(camRequest)
+		Request(std::unique_ptr<DRM::AtomicRequest> drmRequest,
+			libcamera::Request *camRequest)
+			: drmRequest_(std::move(drmRequest)), camRequest_(camRequest)
 		{
 		}