[{"id":26383,"web_url":"https://patchwork.libcamera.org/comment/26383/","msgid":"<20230130193847.z7bffmgzzbwcdevy@uno.localdomain>","date":"2023-01-30T19:38:47","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, Jan 30, 2023 at 06:02:44PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Request objects are created and owned by the application, but are\n> utilised widely throughout the internals of libcamera.\n>\n\nI would have sworn me moved Request in and out from the Camera..\n\n unique_ptr Camera::createRequest();\n int Camera::queueRequest(unique_ptr r);\n\nBut we indeed pass it by referece...\n\n int Camera::queueRequest(Request *r);\n\nI wonder\n\n- Once a Request is queued and before it is signaled as complete,\n should applications be able to access it ?\n\n- Request are signaled as complete through a signal, I guess a signal\n cannot transport a unique_ptr<> back ?\n\n> If the application free's the requests while they are still active\n> within libcamera a use after free will occur. While this can be trapped\n> by tools such as valgrind, given the importance of this object and the\n> relationship of external ownership, it may have some value to provide\n> extended assertions on the condition of these objects.\n>\n> Make sure the request fails an assertion immediately if used after free.\n>\n> Further more, this allows us to immediately reject invalid Request\n> objects directly from the Camera queueRequest API.\n>\n> Signed-off-by: Kieran Bingham \n>\n> ---\n>\n> v2?\n> - Added canary to both the public and private request objects.\n> - Added validation to the Camera queueRequest().\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/internal/request.h | 2 ++\n> include/libcamera/request.h | 4 +++\n> src/libcamera/camera.cpp | 5 +++\n> src/libcamera/request.cpp | 54 ++++++++++++++++++++++++++--\n> 4 files changed, 63 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 8c92a27a95e5..981ad184fffa 100644\n> --- a/include/libcamera/internal/request.h\n> +++ b/include/libcamera/internal/request.h\n> @@ -59,6 +59,8 @@ private:\n> \tstd::unordered_set pending_;\n> \tstd::map> notifiers_;\n> \tstd::unique_ptr timer_;\n> +\n> +\tuint32_t canary_;\n> };\n>\n> } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index dffde1536cad..8e377de14b12 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -65,6 +65,8 @@ public:\n>\n> \tstd::string toString() const;\n>\n> +\tbool canary() const;\n> +\n> private:\n> \tLIBCAMERA_DISABLE_COPY(Request)\n>\n> @@ -74,6 +76,8 @@ private:\n>\n> \tconst uint64_t cookie_;\n> \tStatus status_;\n> +\n> +\tint32_t canary_;\n> };\n>\n> std::ostream &operator<<(std::ostream &out, const Request &r);\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 48bf19b28979..3b72098cce59 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1116,6 +1116,11 @@ int Camera::queueRequest(Request *request)\n> {\n> \tPrivate *const d = _d();\n>\n> +\tif (request->canary()) {\n> +\t\tLOG(Camera, Error) << \"Invalid request\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tint ret = d->isAccessAllowed(Private::CameraRunning);\n> \tif (ret < 0)\n> \t\treturn ret;\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 949c556fa437..cfe732765f86 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -23,6 +23,8 @@\n> #include \"libcamera/internal/framebuffer.h\"\n> #include \"libcamera/internal/tracepoints.h\"\n>\n> +#define REQUEST_CANARY 0x1F2E3D4C\n> +\n> /**\n> * \\file libcamera/request.h\n> * \\brief Describes a frame capture request to be processed by a camera\n> @@ -48,13 +50,14 @@ LOG_DEFINE_CATEGORY(Request)\n> * \\param camera The Camera that creates the request\n> */\n> Request::Private::Private(Camera *camera)\n> -\t: camera_(camera), cancelled_(false)\n> +\t: camera_(camera), cancelled_(false), canary_(REQUEST_CANARY)\n> {\n> }\n>\n> Request::Private::~Private()\n> {\n> \tdoCancelRequest();\n> +\tcanary_ = 0;\n> }\n>\n> /**\n> @@ -114,6 +117,7 @@ void Request::Private::complete()\n> {\n> \tRequest *request = _o();\n>\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> \tASSERT(request->status() == RequestPending);\n> \tASSERT(!hasPendingBuffers());\n>\n> @@ -128,6 +132,8 @@ void Request::Private::doCancelRequest()\n> {\n> \tRequest *request = _o();\n>\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> +\n> \tfor (FrameBuffer *buffer : pending_) {\n> \t\tbuffer->_d()->cancel();\n> \t\tcamera_->bufferCompleted.emit(request, buffer);\n> @@ -149,6 +155,8 @@ void Request::Private::doCancelRequest()\n> */\n> void Request::Private::cancel()\n> {\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> +\n> \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n>\n> \tRequest *request = _o();\n> @@ -346,7 +354,7 @@ void Request::Private::timeout()\n> */\n> Request::Request(Camera *camera, uint64_t cookie)\n> \t: Extensible(std::make_unique(camera)),\n> -\t cookie_(cookie), status_(RequestPending)\n> +\t cookie_(cookie), status_(RequestPending), canary_(REQUEST_CANARY)\n> {\n> \tcontrols_ = new ControlList(controls::controls,\n> \t\t\t\t camera->_d()->validator());\n> @@ -367,6 +375,8 @@ Request::~Request()\n>\n> \tdelete metadata_;\n> \tdelete controls_;\n> +\n> +\tcanary_ = 0;\n> }\n>\n> /**\n> @@ -381,6 +391,11 @@ Request::~Request()\n> */\n> void Request::reuse(ReuseFlag flags)\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> +\t\treturn;\n> +\t}\n> +\n> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n>\n> \t_d()->reset();\n> @@ -462,6 +477,11 @@ void Request::reuse(ReuseFlag flags)\n> int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\t std::unique_ptr fence)\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Attempt to add buffer to invalid request\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tif (!stream) {\n> \t\tLOG(Request, Error) << \"Invalid stream reference\";\n> \t\treturn -EINVAL;\n> @@ -509,6 +529,11 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> */\n> FrameBuffer *Request::findBuffer(const Stream *stream) const\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> \tconst auto it = bufferMap_.find(stream);\n> \tif (it == bufferMap_.end())\n> \t\treturn nullptr;\n> @@ -571,6 +596,7 @@ uint32_t Request::sequence() const\n> */\n> bool Request::hasPendingBuffers() const\n> {\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> \treturn !_d()->pending_.empty();\n> }\n>\n> @@ -590,6 +616,25 @@ std::string Request::toString() const\n> \treturn ss.str();\n> }\n>\n> +/**\n> + * \\brief Identify if the Request object is valid and alive\n> + *\n> + * This provides a means of checking if the request is a valid request object.\n> + * While Requests are constructed by libcamera, they are owned and may be freed\n> + * by applications. It can be all to easy too release a Request object while it\n> + * is still in use by libcamera - or attempt to requeue invalid or deleted\n> + * requests.\n> + *\n> + * The canary provides an insight that the object is not valid and shall be\n> + * rejected by libcamera API calls.\n> + *\n> + * \\return True if the canary has died, and the object shall not be trusted\n> + */\n> +bool Request::canary() const\n> +{\n> +\treturn canary_ != REQUEST_CANARY;\n> +}\n> +\n> /**\n> * \\brief Insert a text representation of a Request into an output stream\n> * \\param[in] out The output stream\n> @@ -601,6 +646,11 @@ std::ostream &operator<<(std::ostream &out, const Request &r)\n> \t/* Pending, Completed, Cancelled(X). */\n> \tstatic const char *statuses = \"PCX\";\n>\n> +\tif (r.canary()) {\n> +\t\tout << \"Request(Invalid Canary)\";\n> +\t\treturn out;\n> +\t}\n> +\n> \t/* Example Output: Request(55:P:1/2:6523524) */\n> \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> \t << r._d()->pending_.size() << \"/\" << r.buffers().size() << \":\"\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E33E5BDC71\n\tfor ;\n\tMon, 30 Jan 2023 19:38:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D3F3625D8;\n\tMon, 30 Jan 2023 20:38:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DC1E960482\n\tfor ;\n\tMon, 30 Jan 2023 20:38:50 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5E2AF8B8;\n\tMon, 30 Jan 2023 20:38:50 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675107532;\n\tbh=ZNA1Kgx0p/AEAFHw0HN0rdgXN0XSwn0NP1VjcoBbhh4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fjdIVl/r71kxv8ouXNWlEUwl10hdF38iz3ZiSUauPfI6q802PMow+hzPVRjQGGGze\n\toiRElE5rCDwVXVNyjekPGaRZ1yIo/hAI9IVx+8gjvxLIMIsmyC0E2hzdRl9lwUJheC\n\thzAuWgxR+lcPzxS41zQh7G6EAECgoByzlNKijcfrCLO8hmQkL2+c6pm5ApWrbQ+igs\n\tI4JxfEG3IEUcqNEH0E5zJWBfmz6URNQOKQ93T5+JU4hE8sbIo+EqQc/+V8IQaZgZdQ\n\t8ZFygHvQExf7mMp6ozeTo/PykRXdTGikOCd5YLY0ZS2ZX0PVrMXelFatYwQExeFfVa\n\tZdpuCo4KvISyg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675107530;\n\tbh=ZNA1Kgx0p/AEAFHw0HN0rdgXN0XSwn0NP1VjcoBbhh4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CXed/LYEZEuZJ0R/vLWofJAuYga1vjwVHxqZCGs4JY9Q/pYlJFz2ecnXKTTHvPZ0j\n\tO+y7uYHxEVOxDVLAVH/7zIZf/ijNuE53iAEY7bCB5LXuJ7R9cQu7PNc6kK/qvfC+QG\n\tuGjw3rfXKeooGvd6YWMO+EeOH0gbwQiGTvhqu3Fw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CXed/LYE\"; dkim-atps=neutral","Date":"Mon, 30 Jan 2023 20:38:47 +0100","To":"Kieran Bingham ","Message-ID":"<20230130193847.z7bffmgzzbwcdevy@uno.localdomain>","References":"<20230130180244.2150205-1-kieran.bingham@ideasonboard.com>\n\t<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26419,"web_url":"https://patchwork.libcamera.org/comment/26419/","msgid":"","date":"2023-02-07T10:11:30","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Mon, Jan 30, 2023 at 06:02:44PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Request objects are created and owned by the application, but are\n> utilised widely throughout the internals of libcamera.\n> \n> If the application free's the requests while they are still active\n> within libcamera a use after free will occur. While this can be trapped\n> by tools such as valgrind, given the importance of this object and the\n> relationship of external ownership, it may have some value to provide\n> extended assertions on the condition of these objects.\n> \n> Make sure the request fails an assertion immediately if used after free.\n> \n> Further more, this allows us to immediately reject invalid Request\n> objects directly from the Camera queueRequest API.\n> \n> Signed-off-by: Kieran Bingham \n> \n> ---\n> \n> v2?\n> - Added canary to both the public and private request objects.\n> - Added validation to the Camera queueRequest().\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/internal/request.h | 2 ++\n> include/libcamera/request.h | 4 +++\n> src/libcamera/camera.cpp | 5 +++\n> src/libcamera/request.cpp | 54 ++++++++++++++++++++++++++--\n> 4 files changed, 63 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 8c92a27a95e5..981ad184fffa 100644\n> --- a/include/libcamera/internal/request.h\n> +++ b/include/libcamera/internal/request.h\n> @@ -59,6 +59,8 @@ private:\n> \tstd::unordered_set pending_;\n> \tstd::map> notifiers_;\n> \tstd::unique_ptr timer_;\n> +\n> +\tuint32_t canary_;\n\nIt's unsigned here...\n\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index dffde1536cad..8e377de14b12 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -65,6 +65,8 @@ public:\n> \n> \tstd::string toString() const;\n> \n> +\tbool canary() const;\n> +\n> private:\n> \tLIBCAMERA_DISABLE_COPY(Request)\n> \n> @@ -74,6 +76,8 @@ private:\n> \n> \tconst uint64_t cookie_;\n> \tStatus status_;\n> +\n> +\tint32_t canary_;\n\n...while it's signed here :D\n\nOther than that, I think the canary is a good thing to have.\n\nReviewed-by: Paul Elder \n\n> };\n> \n> std::ostream &operator<<(std::ostream &out, const Request &r);\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 48bf19b28979..3b72098cce59 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1116,6 +1116,11 @@ int Camera::queueRequest(Request *request)\n> {\n> \tPrivate *const d = _d();\n> \n> +\tif (request->canary()) {\n> +\t\tLOG(Camera, Error) << \"Invalid request\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tint ret = d->isAccessAllowed(Private::CameraRunning);\n> \tif (ret < 0)\n> \t\treturn ret;\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 949c556fa437..cfe732765f86 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -23,6 +23,8 @@\n> #include \"libcamera/internal/framebuffer.h\"\n> #include \"libcamera/internal/tracepoints.h\"\n> \n> +#define REQUEST_CANARY 0x1F2E3D4C\n> +\n> /**\n> * \\file libcamera/request.h\n> * \\brief Describes a frame capture request to be processed by a camera\n> @@ -48,13 +50,14 @@ LOG_DEFINE_CATEGORY(Request)\n> * \\param camera The Camera that creates the request\n> */\n> Request::Private::Private(Camera *camera)\n> -\t: camera_(camera), cancelled_(false)\n> +\t: camera_(camera), cancelled_(false), canary_(REQUEST_CANARY)\n> {\n> }\n> \n> Request::Private::~Private()\n> {\n> \tdoCancelRequest();\n> +\tcanary_ = 0;\n> }\n> \n> /**\n> @@ -114,6 +117,7 @@ void Request::Private::complete()\n> {\n> \tRequest *request = _o();\n> \n> +\tASSERT(canary_ == REQUEST_CANARY);\n> \tASSERT(request->status() == RequestPending);\n> \tASSERT(!hasPendingBuffers());\n> \n> @@ -128,6 +132,8 @@ void Request::Private::doCancelRequest()\n> {\n> \tRequest *request = _o();\n> \n> +\tASSERT(canary_ == REQUEST_CANARY);\n> +\n> \tfor (FrameBuffer *buffer : pending_) {\n> \t\tbuffer->_d()->cancel();\n> \t\tcamera_->bufferCompleted.emit(request, buffer);\n> @@ -149,6 +155,8 @@ void Request::Private::doCancelRequest()\n> */\n> void Request::Private::cancel()\n> {\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> +\n> \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n> \n> \tRequest *request = _o();\n> @@ -346,7 +354,7 @@ void Request::Private::timeout()\n> */\n> Request::Request(Camera *camera, uint64_t cookie)\n> \t: Extensible(std::make_unique(camera)),\n> -\t cookie_(cookie), status_(RequestPending)\n> +\t cookie_(cookie), status_(RequestPending), canary_(REQUEST_CANARY)\n> {\n> \tcontrols_ = new ControlList(controls::controls,\n> \t\t\t\t camera->_d()->validator());\n> @@ -367,6 +375,8 @@ Request::~Request()\n> \n> \tdelete metadata_;\n> \tdelete controls_;\n> +\n> +\tcanary_ = 0;\n> }\n> \n> /**\n> @@ -381,6 +391,11 @@ Request::~Request()\n> */\n> void Request::reuse(ReuseFlag flags)\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> +\t\treturn;\n> +\t}\n> +\n> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> \n> \t_d()->reset();\n> @@ -462,6 +477,11 @@ void Request::reuse(ReuseFlag flags)\n> int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\t std::unique_ptr fence)\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Attempt to add buffer to invalid request\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tif (!stream) {\n> \t\tLOG(Request, Error) << \"Invalid stream reference\";\n> \t\treturn -EINVAL;\n> @@ -509,6 +529,11 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> */\n> FrameBuffer *Request::findBuffer(const Stream *stream) const\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> \tconst auto it = bufferMap_.find(stream);\n> \tif (it == bufferMap_.end())\n> \t\treturn nullptr;\n> @@ -571,6 +596,7 @@ uint32_t Request::sequence() const\n> */\n> bool Request::hasPendingBuffers() const\n> {\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> \treturn !_d()->pending_.empty();\n> }\n> \n> @@ -590,6 +616,25 @@ std::string Request::toString() const\n> \treturn ss.str();\n> }\n> \n> +/**\n> + * \\brief Identify if the Request object is valid and alive\n> + *\n> + * This provides a means of checking if the request is a valid request object.\n> + * While Requests are constructed by libcamera, they are owned and may be freed\n> + * by applications. It can be all to easy too release a Request object while it\n> + * is still in use by libcamera - or attempt to requeue invalid or deleted\n> + * requests.\n> + *\n> + * The canary provides an insight that the object is not valid and shall be\n> + * rejected by libcamera API calls.\n> + *\n> + * \\return True if the canary has died, and the object shall not be trusted\n> + */\n> +bool Request::canary() const\n> +{\n> +\treturn canary_ != REQUEST_CANARY;\n> +}\n> +\n> /**\n> * \\brief Insert a text representation of a Request into an output stream\n> * \\param[in] out The output stream\n> @@ -601,6 +646,11 @@ std::ostream &operator<<(std::ostream &out, const Request &r)\n> \t/* Pending, Completed, Cancelled(X). */\n> \tstatic const char *statuses = \"PCX\";\n> \n> +\tif (r.canary()) {\n> +\t\tout << \"Request(Invalid Canary)\";\n> +\t\treturn out;\n> +\t}\n> +\n> \t/* Example Output: Request(55:P:1/2:6523524) */\n> \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> \t << r._d()->pending_.size() << \"/\" << r.buffers().size() << \":\"\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8EC17BEFBE\n\tfor ;\n\tTue, 7 Feb 2023 10:11:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF8C6625F3;\n\tTue, 7 Feb 2023 11:11:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 518BC603BD\n\tfor ;\n\tTue, 7 Feb 2023 11:11:37 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E47C9589;\n\tTue, 7 Feb 2023 11:11:35 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675764698;\n\tbh=S7sC1pkvWi/oSAKZpSnNnMFP9noncm78wDaxXvhOcwU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=dQGmmsRhADK5bLGaiiZwLDOZluHIzgIm69pfkPKifKzfNe0rM1tw7B03XdLoqVull\n\tIffWkB5b9fAs7t+6OfUGaFgWZATWVNOYBE4W6UEX0c7xHdGU5z7GN/ERUgNFeyChiC\n\tkuxpqyS5PJ+fToRU1NN/6CJcuxMXTyhXcgBltVwKMk8YBdJBaA9zsNa/eufOcgyEqe\n\tc+hNGkXGErMr41yyqCb3qDHwLfxnq0+gdusjtRKVTPk5sNY+YNsg2VSfQcfVsoK75t\n\t53WeaInnpSpoV5KaKk7rC4X1MtFZy63xCtgfOPkfWfoQ8flQvFsQMQbm6yvOrVKUTJ\n\tIDjKP/I9EE3aQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675764696;\n\tbh=S7sC1pkvWi/oSAKZpSnNnMFP9noncm78wDaxXvhOcwU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ub/4gkeIWIezneStQCJd9bvyKo5eAfEzuFyyXGQcBzrnK5uypwpRRONNHkXpDZsmb\n\t9dH9MXxe3uIplNnNCmCYry/raEa8UXphwrJ8QRbvUOJ32Kfa3E3jA32cU8YCJKbtRM\n\t0DBOJoaPSN0Pvj4IsVtIpvOoGyqBtW3SI6z+FrOg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ub/4gkeI\"; dkim-atps=neutral","Date":"Tue, 7 Feb 2023 19:11:30 +0900","To":"Kieran Bingham ","Message-ID":"","References":"<20230130180244.2150205-1-kieran.bingham@ideasonboard.com>\n\t<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26423,"web_url":"https://patchwork.libcamera.org/comment/26423/","msgid":"","date":"2023-02-07T16:05:54","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Jan 30, 2023 at 08:38:47PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> On Mon, Jan 30, 2023 at 06:02:44PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > Request objects are created and owned by the application, but are\n> > utilised widely throughout the internals of libcamera.\n> \n> I would have sworn me moved Request in and out from the Camera..\n> \n> unique_ptr Camera::createRequest();\n> int Camera::queueRequest(unique_ptr r);\n> \n> But we indeed pass it by referece...\n> \n> int Camera::queueRequest(Request *r);\n> \n> I wonder\n> \n> - Once a Request is queued and before it is signaled as complete,\n> should applications be able to access it ?\n\nPossibly in read-only mode, but even that will likely be racy, and I\ndon't see any compelling use case right now.\n\n> - Request are signaled as complete through a signal, I guess a signal\n> cannot transport a unique_ptr<> back ?\n\nCorrect. This is due to the fact that you can connect a signal to\nmultiple slots, which would break the ownership transfer requirements of\nstd::unique_ptr<>.\n\nI'm more and more tempted to experiment with passing requests by value\n(although we'll have a similar problem with framebuffers). It feels like\nwe're working around a badly designed API, we should fix the API\ninstead.\n\n> > If the application free's the requests while they are still active\n\ns/free's/fress/\n\n> > within libcamera a use after free will occur. While this can be trapped\n> > by tools such as valgrind, given the importance of this object and the\n> > relationship of external ownership, it may have some value to provide\n> > extended assertions on the condition of these objects.\n> >\n> > Make sure the request fails an assertion immediately if used after free.\n> >\n> > Further more, this allows us to immediately reject invalid Request\n\ns/Further more/Furthermore/\n\n> > objects directly from the Camera queueRequest API.\n> >\n> > Signed-off-by: Kieran Bingham \n> >\n> > ---\n> >\n> > v2?\n> > - Added canary to both the public and private request objects.\n> > - Added validation to the Camera queueRequest().\n> >\n> > Signed-off-by: Kieran Bingham \n> > ---\n> > include/libcamera/internal/request.h | 2 ++\n> > include/libcamera/request.h | 4 +++\n> > src/libcamera/camera.cpp | 5 +++\n> > src/libcamera/request.cpp | 54 ++++++++++++++++++++++++++--\n> > 4 files changed, 63 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> > index 8c92a27a95e5..981ad184fffa 100644\n> > --- a/include/libcamera/internal/request.h\n> > +++ b/include/libcamera/internal/request.h\n> > @@ -59,6 +59,8 @@ private:\n> > \tstd::unordered_set pending_;\n> > \tstd::map> notifiers_;\n> > \tstd::unique_ptr timer_;\n> > +\n> > +\tuint32_t canary_;\n> > };\n> >\n> > } /* namespace libcamera */\n> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index dffde1536cad..8e377de14b12 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -65,6 +65,8 @@ public:\n> >\n> > \tstd::string toString() const;\n> >\n> > +\tbool canary() const;\n> > +\n> > private:\n> > \tLIBCAMERA_DISABLE_COPY(Request)\n> >\n> > @@ -74,6 +76,8 @@ private:\n> >\n> > \tconst uint64_t cookie_;\n> > \tStatus status_;\n> > +\n> > +\tint32_t canary_;\n\nWe're trying to move away from storing private data in the public class\n:-S I really dislike exposing this to the public API, including the\npublic canary() function.\n\n> > };\n> >\n> > std::ostream &operator<<(std::ostream &out, const Request &r);\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 48bf19b28979..3b72098cce59 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -1116,6 +1116,11 @@ int Camera::queueRequest(Request *request)\n> > {\n> > \tPrivate *const d = _d();\n> >\n> > +\tif (request->canary()) {\n> > +\t\tLOG(Camera, Error) << \"Invalid request\";\n> > +\t\treturn -EINVAL;\n\nI'd make everything fatal, using ASSERT(). If the request passed to this\nfunction has been freed, it's game over anyway. Same below.\n\n> > +\t}\n> > +\n> > \tint ret = d->isAccessAllowed(Private::CameraRunning);\n> > \tif (ret < 0)\n> > \t\treturn ret;\n> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > index 949c556fa437..cfe732765f86 100644\n> > --- a/src/libcamera/request.cpp\n> > +++ b/src/libcamera/request.cpp\n> > @@ -23,6 +23,8 @@\n> > #include \"libcamera/internal/framebuffer.h\"\n> > #include \"libcamera/internal/tracepoints.h\"\n> >\n> > +#define REQUEST_CANARY 0x1F2E3D4C\n\nLower-case hex constants.\n\n> > +\n> > /**\n> > * \\file libcamera/request.h\n> > * \\brief Describes a frame capture request to be processed by a camera\n> > @@ -48,13 +50,14 @@ LOG_DEFINE_CATEGORY(Request)\n> > * \\param camera The Camera that creates the request\n> > */\n> > Request::Private::Private(Camera *camera)\n> > -\t: camera_(camera), cancelled_(false)\n> > +\t: camera_(camera), cancelled_(false), canary_(REQUEST_CANARY)\n> > {\n> > }\n> >\n> > Request::Private::~Private()\n> > {\n> > \tdoCancelRequest();\n> > +\tcanary_ = 0;\n> > }\n> >\n> > /**\n> > @@ -114,6 +117,7 @@ void Request::Private::complete()\n> > {\n> > \tRequest *request = _o();\n> >\n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > \tASSERT(request->status() == RequestPending);\n> > \tASSERT(!hasPendingBuffers());\n> >\n> > @@ -128,6 +132,8 @@ void Request::Private::doCancelRequest()\n> > {\n> > \tRequest *request = _o();\n> >\n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > +\n> > \tfor (FrameBuffer *buffer : pending_) {\n> > \t\tbuffer->_d()->cancel();\n> > \t\tcamera_->bufferCompleted.emit(request, buffer);\n> > @@ -149,6 +155,8 @@ void Request::Private::doCancelRequest()\n> > */\n> > void Request::Private::cancel()\n> > {\n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > +\n> > \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n> >\n> > \tRequest *request = _o();\n> > @@ -346,7 +354,7 @@ void Request::Private::timeout()\n> > */\n> > Request::Request(Camera *camera, uint64_t cookie)\n> > \t: Extensible(std::make_unique(camera)),\n> > -\t cookie_(cookie), status_(RequestPending)\n> > +\t cookie_(cookie), status_(RequestPending), canary_(REQUEST_CANARY)\n> > {\n> > \tcontrols_ = new ControlList(controls::controls,\n> > \t\t\t\t camera->_d()->validator());\n> > @@ -367,6 +375,8 @@ Request::~Request()\n> >\n> > \tdelete metadata_;\n> > \tdelete controls_;\n> > +\n> > +\tcanary_ = 0;\n> > }\n> >\n> > /**\n> > @@ -381,6 +391,11 @@ Request::~Request()\n> > */\n> > void Request::reuse(ReuseFlag flags)\n> > {\n> > +\tif (canary_ != REQUEST_CANARY) {\n> > +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> > +\t\treturn;\n> > +\t}\n> > +\n> > \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> >\n> > \t_d()->reset();\n> > @@ -462,6 +477,11 @@ void Request::reuse(ReuseFlag flags)\n> > int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > \t\t std::unique_ptr fence)\n> > {\n> > +\tif (canary_ != REQUEST_CANARY) {\n> > +\t\tLOG(Request, Error) << \"Attempt to add buffer to invalid request\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > \tif (!stream) {\n> > \t\tLOG(Request, Error) << \"Invalid stream reference\";\n> > \t\treturn -EINVAL;\n> > @@ -509,6 +529,11 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > */\n> > FrameBuffer *Request::findBuffer(const Stream *stream) const\n> > {\n> > +\tif (canary_ != REQUEST_CANARY) {\n> > +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > \tconst auto it = bufferMap_.find(stream);\n> > \tif (it == bufferMap_.end())\n> > \t\treturn nullptr;\n> > @@ -571,6 +596,7 @@ uint32_t Request::sequence() const\n> > */\n> > bool Request::hasPendingBuffers() const\n> > {\n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > \treturn !_d()->pending_.empty();\n> > }\n> >\n> > @@ -590,6 +616,25 @@ std::string Request::toString() const\n> > \treturn ss.str();\n> > }\n> >\n> > +/**\n> > + * \\brief Identify if the Request object is valid and alive\n> > + *\n> > + * This provides a means of checking if the request is a valid request object.\n> > + * While Requests are constructed by libcamera, they are owned and may be freed\n> > + * by applications. It can be all to easy too release a Request object while it\n> > + * is still in use by libcamera - or attempt to requeue invalid or deleted\n> > + * requests.\n> > + *\n> > + * The canary provides an insight that the object is not valid and shall be\n> > + * rejected by libcamera API calls.\n> > + *\n> > + * \\return True if the canary has died, and the object shall not be trusted\n> > + */\n> > +bool Request::canary() const\n> > +{\n> > +\treturn canary_ != REQUEST_CANARY;\n> > +}\n> > +\n> > /**\n> > * \\brief Insert a text representation of a Request into an output stream\n> > * \\param[in] out The output stream\n> > @@ -601,6 +646,11 @@ std::ostream &operator<<(std::ostream &out, const Request &r)\n> > \t/* Pending, Completed, Cancelled(X). */\n> > \tstatic const char *statuses = \"PCX\";\n> >\n> > +\tif (r.canary()) {\n> > +\t\tout << \"Request(Invalid Canary)\";\n> > +\t\treturn out;\n> > +\t}\n> > +\n> > \t/* Example Output: Request(55:P:1/2:6523524) */\n> > \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> > \t << r._d()->pending_.size() << \"/\" << r.buffers().size() << \":\"","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5FCAFBEFBE\n\tfor ;\n\tTue, 7 Feb 2023 16:06:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8A0C8625F4;\n\tTue, 7 Feb 2023 17:05:59 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5CED4625DF\n\tfor ;\n\tTue, 7 Feb 2023 17:05:57 +0100 (CET)","from pendragon.ideasonboard.com (unknown [109.136.43.56])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C33D34AF;\n\tTue, 7 Feb 2023 17:05:56 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675785959;\n\tbh=wbDBNk+w+o3jhuQBfRBjlhesM3bgIah/QwxYF4RLzY0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=KoFpRqtHwNzu2JbZMI5LABACbpAl8bxYxfYD8JXmbUksOCH0Bxaxf+rg0J3Uyfdoy\n\twgWS2eT/+XoEA44Em44y+vFBc9MBOX6/iszwgZlq9+INmX4atf3CscQAA68mw+mPps\n\tW+VefWe6wujGeaMbb31RNJleb3l/PAJjXI1vnv8ji8eDKW7FxybAlfF8/K+38IvpQZ\n\t2Xh9Xxc2wQ1XQU+9BBcvgSfub4YlnAKUhKwt5x0JLuA1MQvImXUxGqrSIyI7QuVo2t\n\tlfBbxydhsIS3V8vLbkMp7tzp9zAvVMmh8l09gHs1FYPoZZiiBj9FAb9Ty/eyoOe2D6\n\tREw6ckCwWYMTQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675785956;\n\tbh=wbDBNk+w+o3jhuQBfRBjlhesM3bgIah/QwxYF4RLzY0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=l5j3Pmc8blIa0FCfwOnMLV7Sy16IOssjmonCx4xeD899VyAkltbjU66t0uINm2fLo\n\tpPMq4ndtkuFWUTfldOjysSKK/UKC2y4nZKhIlaVEYX5jufotT2Jx1Ly5j2K390edY5\n\tkgd4VFumTYXREr2AIjU6DuEDoSlregUx7oeABu0c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"l5j3Pmc8\"; dkim-atps=neutral","Date":"Tue, 7 Feb 2023 18:05:54 +0200","To":"Jacopo Mondi ","Message-ID":"","References":"<20230130180244.2150205-1-kieran.bingham@ideasonboard.com>\n\t<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>\n\t<20230130193847.z7bffmgzzbwcdevy@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230130193847.z7bffmgzzbwcdevy@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26654,"web_url":"https://patchwork.libcamera.org/comment/26654/","msgid":"<20230313184317.608bdab4.dorota.czaplejewicz@puri.sm>","date":"2023-03-13T17:43:17","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","submitter":{"id":96,"url":"https://patchwork.libcamera.org/api/people/96/","name":"Dorota Czaplejewicz","email":"dorota.czaplejewicz@puri.sm"},"content":"On Mon, 30 Jan 2023 18:02:44 +0000\nKieran Bingham wrote:\n\n> Request objects are created and owned by the application, but are\n> utilised widely throughout the internals of libcamera.\n> \n> If the application free's the requests while they are still active\n> within libcamera a use after free will occur. While this can be trapped\n> by tools such as valgrind, given the importance of this object and the\n> relationship of external ownership, it may have some value to provide\n> extended assertions on the condition of these objects.\n> \n> Make sure the request fails an assertion immediately if used after free.\n> \n> Further more, this allows us to immediately reject invalid Request\n> objects directly from the Camera queueRequest API.\n> \n> Signed-off-by: Kieran Bingham \n> Reviewed-by: Paul Elder \n> ---\n> \n> v2?\n> - Added canary to both the public and private request objects.\n> - Added validation to the Camera queueRequest().\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/internal/request.h | 2 ++\n> include/libcamera/request.h | 4 +++\n> src/libcamera/camera.cpp | 5 +++\n> src/libcamera/request.cpp | 54 ++++++++++++++++++++++++++--\n> 4 files changed, 63 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 8c92a27a95e5..981ad184fffa 100644\n> --- a/include/libcamera/internal/request.h\n> +++ b/include/libcamera/internal/request.h\n> @@ -59,6 +59,8 @@ private:\n> \tstd::unordered_set pending_;\n> \tstd::map> notifiers_;\n> \tstd::unique_ptr timer_;\n> +\n> +\tuint32_t canary_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index dffde1536cad..8e377de14b12 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -65,6 +65,8 @@ public:\n> \n> \tstd::string toString() const;\n> \n> +\tbool canary() const;\n> +\n> private:\n> \tLIBCAMERA_DISABLE_COPY(Request)\n> \n> @@ -74,6 +76,8 @@ private:\n> \n> \tconst uint64_t cookie_;\n> \tStatus status_;\n> +\n> +\tint32_t canary_;\n> };\n> \n> std::ostream &operator<<(std::ostream &out, const Request &r);\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 48bf19b28979..3b72098cce59 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1116,6 +1116,11 @@ int Camera::queueRequest(Request *request)\n> {\n> \tPrivate *const d = _d();\n> \n> +\tif (request->canary()) {\n> +\t\tLOG(Camera, Error) << \"Invalid request\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tint ret = d->isAccessAllowed(Private::CameraRunning);\n> \tif (ret < 0)\n> \t\treturn ret;\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 949c556fa437..cfe732765f86 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -23,6 +23,8 @@\n> #include \"libcamera/internal/framebuffer.h\"\n> #include \"libcamera/internal/tracepoints.h\"\n> \n> +#define REQUEST_CANARY 0x1F2E3D4C\n> +\n> /**\n> * \\file libcamera/request.h\n> * \\brief Describes a frame capture request to be processed by a camera\n> @@ -48,13 +50,14 @@ LOG_DEFINE_CATEGORY(Request)\n> * \\param camera The Camera that creates the request\n> */\n> Request::Private::Private(Camera *camera)\n> -\t: camera_(camera), cancelled_(false)\n> +\t: camera_(camera), cancelled_(false), canary_(REQUEST_CANARY)\n> {\n> }\n> \n> Request::Private::~Private()\n> {\n> \tdoCancelRequest();\n> +\tcanary_ = 0;\n> }\n> \n> /**\n> @@ -114,6 +117,7 @@ void Request::Private::complete()\n> {\n> \tRequest *request = _o();\n> \n> +\tASSERT(canary_ == REQUEST_CANARY);\n> \tASSERT(request->status() == RequestPending);\n> \tASSERT(!hasPendingBuffers());\n> \n> @@ -128,6 +132,8 @@ void Request::Private::doCancelRequest()\n> {\n> \tRequest *request = _o();\n> \n> +\tASSERT(canary_ == REQUEST_CANARY);\n> +\n> \tfor (FrameBuffer *buffer : pending_) {\n> \t\tbuffer->_d()->cancel();\n> \t\tcamera_->bufferCompleted.emit(request, buffer);\n> @@ -149,6 +155,8 @@ void Request::Private::doCancelRequest()\n> */\n> void Request::Private::cancel()\n> {\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> +\n> \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n> \n> \tRequest *request = _o();\n> @@ -346,7 +354,7 @@ void Request::Private::timeout()\n> */\n> Request::Request(Camera *camera, uint64_t cookie)\n> \t: Extensible(std::make_unique(camera)),\n> -\t cookie_(cookie), status_(RequestPending)\n> +\t cookie_(cookie), status_(RequestPending), canary_(REQUEST_CANARY)\n> {\n> \tcontrols_ = new ControlList(controls::controls,\n> \t\t\t\t camera->_d()->validator());\n> @@ -367,6 +375,8 @@ Request::~Request()\n> \n> \tdelete metadata_;\n> \tdelete controls_;\n> +\n> +\tcanary_ = 0;\n> }\n> \n> /**\n> @@ -381,6 +391,11 @@ Request::~Request()\n> */\n> void Request::reuse(ReuseFlag flags)\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> +\t\treturn;\n> +\t}\n> +\n> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> \n> \t_d()->reset();\n> @@ -462,6 +477,11 @@ void Request::reuse(ReuseFlag flags)\n> int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> \t\t std::unique_ptr fence)\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Attempt to add buffer to invalid request\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tif (!stream) {\n> \t\tLOG(Request, Error) << \"Invalid stream reference\";\n> \t\treturn -EINVAL;\n> @@ -509,6 +529,11 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> */\n> FrameBuffer *Request::findBuffer(const Stream *stream) const\n> {\n> +\tif (canary_ != REQUEST_CANARY) {\n> +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> \tconst auto it = bufferMap_.find(stream);\n> \tif (it == bufferMap_.end())\n> \t\treturn nullptr;\n> @@ -571,6 +596,7 @@ uint32_t Request::sequence() const\n> */\n> bool Request::hasPendingBuffers() const\n> {\n> +\tASSERT(canary_ == REQUEST_CANARY);\n> \treturn !_d()->pending_.empty();\n> }\n> \n> @@ -590,6 +616,25 @@ std::string Request::toString() const\n> \treturn ss.str();\n> }\n> \n> +/**\n> + * \\brief Identify if the Request object is valid and alive\n> + *\n> + * This provides a means of checking if the request is a valid request object.\n> + * While Requests are constructed by libcamera, they are owned and may be freed\n> + * by applications. It can be all to easy too release a Request object while it\n> + * is still in use by libcamera - or attempt to requeue invalid or deleted\n> + * requests.\n> + *\n> + * The canary provides an insight that the object is not valid and shall be\n> + * rejected by libcamera API calls.\n> + *\n> + * \\return True if the canary has died, and the object shall not be trusted\n> + */\n> +bool Request::canary() const\n> +{\n> +\treturn canary_ != REQUEST_CANARY;\n> +}\n> +\n> /**\n> * \\brief Insert a text representation of a Request into an output stream\n> * \\param[in] out The output stream\n> @@ -601,6 +646,11 @@ std::ostream &operator<<(std::ostream &out, const Request &r)\n> \t/* Pending, Completed, Cancelled(X). */\n> \tstatic const char *statuses = \"PCX\";\n> \n> +\tif (r.canary()) {\n> +\t\tout << \"Request(Invalid Canary)\";\n> +\t\treturn out;\n> +\t}\n> +\n> \t/* Example Output: Request(55:P:1/2:6523524) */\n> \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> \t << r._d()->pending_.size() << \"/\" << r.buffers().size() << \":\"\n\n\nHi,\n\nI ran into a version of this problem where some code refactored using a few smaller code stopped being correct when taken as a whole. This kind of a canary would have probably prevented that. A small reproducer is here:\n\nhttps://source.puri.sm/dorota.czaplejewicz/simplecam/-/commits/bug_time\n\nIn this case, the problem is accessing the local reference to the request after the (same) owner just released it. While I prefer to steer people to not make mistakes in the first place (like I mentioned in https://bugs.libcamera.org/show_bug.cgi?id=186 ), I'm not familiar enough with C++ to know if such a use-after-free can be caught at compile time.\n\nCheers,\nDorota","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 49F53BE080\n\tfor ;\n\tMon, 13 Mar 2023 17:43:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 668FE62709;\n\tMon, 13 Mar 2023 18:43:37 +0100 (CET)","from comms.puri.sm (comms.puri.sm [159.203.221.185])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78CFE62674\n\tfor ;\n\tMon, 13 Mar 2023 18:43:35 +0100 (CET)","from localhost (localhost [127.0.0.1])\n\tby comms.puri.sm (Postfix) with ESMTP id 2B45CE2042;\n\tMon, 13 Mar 2023 10:43:34 -0700 (PDT)","from comms.puri.sm ([127.0.0.1])\n\tby localhost (comms.puri.sm [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id Z98GAN3NGL_n; Mon, 13 Mar 2023 10:43:32 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678729417;\n\tbh=hk021Le5LPN3TkvYqdiVGLAdYyuq2kvH55fiZ7GyatU=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=3Cmxi3R+ns+K6sIjLkB0fR5i7LQHgs9ofnGn8uY5f/6Sux6j68BZqhXJhRSuzB8l6\n\tYkWJdtBhdkwKmFDbxlg0vhHV/lBWSqLSG7Kt2VsBnwE7huZc/nE/402HJwrwQXm8eB\n\t91yxlUEeMbsAS62HtyDWOEicb2Mcvyo1N8ZVv4mV71f7KH54uMOizbhweuc66UflsJ\n\tZ/yqwGW9PdbtYDdkPfR8WzAzr3e6Brm0gHQ+Tg2626O+XNXHWyLrK4086FIm3YFCCI\n\tb5gm7UiK2C1NL64mmezVJdwI7U7cv7SaxW8gYupnWL0RxWTx0lnw4jZvYbqZw/Cf8O\n\toHYFgua2r8Ctg==","v=1; a=rsa-sha256; c=relaxed/simple; d=puri.sm; s=comms;\n\tt=1678729412; bh=hk021Le5LPN3TkvYqdiVGLAdYyuq2kvH55fiZ7GyatU=;\n\th=Date:From:To:Cc:Subject:In-Reply-To:References:From;\n\tb=q84narTeor20kB2/esbn5eQy82gXq3613OyNUMtlIA41ZTQCkmvUjVgkOpMuQLNNS\n\tB6m4rq32c4BJmSkABfyi2/xhjI2BQFW0pPV9i2Tc2dRjktnABkryBIysTVBh9eZfic\n\tpGRvh98EmE5xjFp9J7l4xtj5Y9r+xJzZ54TfYYkmD+Il0n+4wT4nZiETC2vcSybufZ\n\txshDlde+uQw/zbDvykYrjn9ANvwt4Gt7NhUfGFzXKGj0j80tUSuIU0zVNiAWckRD8M\n\tPnLjdChZ+CISURUgq5zKU2gUGF6dCdmnnWhh//CyoC/hk3769XmcYHPWlv0L+4nI9a\n\tGfIBdGcsLXnqg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=puri.sm header.i=@puri.sm\n\theader.b=\"q84narTe\"; dkim-atps=neutral","Date":"Mon, 13 Mar 2023 18:43:17 +0100","To":"Kieran Bingham ","Message-ID":"<20230313184317.608bdab4.dorota.czaplejewicz@puri.sm>","In-Reply-To":"<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>","References":"<20230130180244.2150205-1-kieran.bingham@ideasonboard.com>\n\t<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>","Organization":"Purism","MIME-Version":"1.0","Content-Type":"multipart/signed; boundary=\"Sig_/VGAD=2GDcPLd8X.vhNKa__B\";\n\tprotocol=\"application/pgp-signature\"; micalg=pgp-sha256","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Dorota Czaplejewicz via libcamera-devel\n\t","Reply-To":"Dorota Czaplejewicz ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26656,"web_url":"https://patchwork.libcamera.org/comment/26656/","msgid":"<20230314110118.4ffd4b25.dorota.czaplejewicz@puri.sm>","date":"2023-03-14T10:01:18","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","submitter":{"id":96,"url":"https://patchwork.libcamera.org/api/people/96/","name":"Dorota Czaplejewicz","email":"dorota.czaplejewicz@puri.sm"},"content":"On Mon, 13 Mar 2023 18:43:17 +0100\nDorota Czaplejewicz wrote:\n\n> On Mon, 30 Jan 2023 18:02:44 +0000\n> Kieran Bingham wrote:\n> \n> > Request objects are created and owned by the application, but are\n> > utilised widely throughout the internals of libcamera.\n> > \n> > If the application free's the requests while they are still active\n> > within libcamera a use after free will occur. While this can be trapped\n> > by tools such as valgrind, given the importance of this object and the\n> > relationship of external ownership, it may have some value to provide\n> > extended assertions on the condition of these objects.\n> > \n> > Make sure the request fails an assertion immediately if used after free.\n> > \n> > Further more, this allows us to immediately reject invalid Request\n> > objects directly from the Camera queueRequest API.\n> > \n> > Signed-off-by: Kieran Bingham \n> > Reviewed-by: Paul Elder \n> > ---\n> > \n> > v2?\n> > - Added canary to both the public and private request objects.\n> > - Added validation to the Camera queueRequest().\n> > \n> > Signed-off-by: Kieran Bingham \n> > ---\n> > include/libcamera/internal/request.h | 2 ++\n> > include/libcamera/request.h | 4 +++\n> > src/libcamera/camera.cpp | 5 +++\n> > src/libcamera/request.cpp | 54 ++++++++++++++++++++++++++--\n> > 4 files changed, 63 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> > index 8c92a27a95e5..981ad184fffa 100644\n> > --- a/include/libcamera/internal/request.h\n> > +++ b/include/libcamera/internal/request.h\n> > @@ -59,6 +59,8 @@ private:\n> > \tstd::unordered_set pending_;\n> > \tstd::map> notifiers_;\n> > \tstd::unique_ptr timer_;\n> > +\n> > +\tuint32_t canary_;\n> > };\n> > \n> > } /* namespace libcamera */\n> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index dffde1536cad..8e377de14b12 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -65,6 +65,8 @@ public:\n> > \n> > \tstd::string toString() const;\n> > \n> > +\tbool canary() const;\n> > +\n> > private:\n> > \tLIBCAMERA_DISABLE_COPY(Request)\n> > \n> > @@ -74,6 +76,8 @@ private:\n> > \n> > \tconst uint64_t cookie_;\n> > \tStatus status_;\n> > +\n> > +\tint32_t canary_;\n> > };\n> > \n> > std::ostream &operator<<(std::ostream &out, const Request &r);\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 48bf19b28979..3b72098cce59 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -1116,6 +1116,11 @@ int Camera::queueRequest(Request *request)\n> > {\n> > \tPrivate *const d = _d();\n> > \n> > +\tif (request->canary()) {\n> > +\t\tLOG(Camera, Error) << \"Invalid request\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > \tint ret = d->isAccessAllowed(Private::CameraRunning);\n> > \tif (ret < 0)\n> > \t\treturn ret;\n> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > index 949c556fa437..cfe732765f86 100644\n> > --- a/src/libcamera/request.cpp\n> > +++ b/src/libcamera/request.cpp\n> > @@ -23,6 +23,8 @@\n> > #include \"libcamera/internal/framebuffer.h\"\n> > #include \"libcamera/internal/tracepoints.h\"\n> > \n> > +#define REQUEST_CANARY 0x1F2E3D4C\n> > +\n> > /**\n> > * \\file libcamera/request.h\n> > * \\brief Describes a frame capture request to be processed by a camera\n> > @@ -48,13 +50,14 @@ LOG_DEFINE_CATEGORY(Request)\n> > * \\param camera The Camera that creates the request\n> > */\n> > Request::Private::Private(Camera *camera)\n> > -\t: camera_(camera), cancelled_(false)\n> > +\t: camera_(camera), cancelled_(false), canary_(REQUEST_CANARY)\n> > {\n> > }\n> > \n> > Request::Private::~Private()\n> > {\n> > \tdoCancelRequest();\n> > +\tcanary_ = 0;\n> > }\n> > \n> > /**\n> > @@ -114,6 +117,7 @@ void Request::Private::complete()\n> > {\n> > \tRequest *request = _o();\n> > \n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > \tASSERT(request->status() == RequestPending);\n> > \tASSERT(!hasPendingBuffers());\n> > \n> > @@ -128,6 +132,8 @@ void Request::Private::doCancelRequest()\n> > {\n> > \tRequest *request = _o();\n> > \n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > +\n> > \tfor (FrameBuffer *buffer : pending_) {\n> > \t\tbuffer->_d()->cancel();\n> > \t\tcamera_->bufferCompleted.emit(request, buffer);\n> > @@ -149,6 +155,8 @@ void Request::Private::doCancelRequest()\n> > */\n> > void Request::Private::cancel()\n> > {\n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > +\n> > \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n> > \n> > \tRequest *request = _o();\n> > @@ -346,7 +354,7 @@ void Request::Private::timeout()\n> > */\n> > Request::Request(Camera *camera, uint64_t cookie)\n> > \t: Extensible(std::make_unique(camera)),\n> > -\t cookie_(cookie), status_(RequestPending)\n> > +\t cookie_(cookie), status_(RequestPending), canary_(REQUEST_CANARY)\n> > {\n> > \tcontrols_ = new ControlList(controls::controls,\n> > \t\t\t\t camera->_d()->validator());\n> > @@ -367,6 +375,8 @@ Request::~Request()\n> > \n> > \tdelete metadata_;\n> > \tdelete controls_;\n> > +\n> > +\tcanary_ = 0;\n> > }\n> > \n> > /**\n> > @@ -381,6 +391,11 @@ Request::~Request()\n> > */\n> > void Request::reuse(ReuseFlag flags)\n> > {\n> > +\tif (canary_ != REQUEST_CANARY) {\n> > +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> > +\t\treturn;\n> > +\t}\n> > +\n> > \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> > \n> > \t_d()->reset();\n> > @@ -462,6 +477,11 @@ void Request::reuse(ReuseFlag flags)\n> > int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > \t\t std::unique_ptr fence)\n> > {\n> > +\tif (canary_ != REQUEST_CANARY) {\n> > +\t\tLOG(Request, Error) << \"Attempt to add buffer to invalid request\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > \tif (!stream) {\n> > \t\tLOG(Request, Error) << \"Invalid stream reference\";\n> > \t\treturn -EINVAL;\n> > @@ -509,6 +529,11 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > */\n> > FrameBuffer *Request::findBuffer(const Stream *stream) const\n> > {\n> > +\tif (canary_ != REQUEST_CANARY) {\n> > +\t\tLOG(Request, Error) << \"Invalid Request object\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > \tconst auto it = bufferMap_.find(stream);\n> > \tif (it == bufferMap_.end())\n> > \t\treturn nullptr;\n> > @@ -571,6 +596,7 @@ uint32_t Request::sequence() const\n> > */\n> > bool Request::hasPendingBuffers() const\n> > {\n> > +\tASSERT(canary_ == REQUEST_CANARY);\n> > \treturn !_d()->pending_.empty();\n> > }\n> > \n> > @@ -590,6 +616,25 @@ std::string Request::toString() const\n> > \treturn ss.str();\n> > }\n> > \n> > +/**\n> > + * \\brief Identify if the Request object is valid and alive\n> > + *\n> > + * This provides a means of checking if the request is a valid request object.\n> > + * While Requests are constructed by libcamera, they are owned and may be freed\n> > + * by applications. It can be all to easy too release a Request object while it\n> > + * is still in use by libcamera - or attempt to requeue invalid or deleted\n> > + * requests.\n> > + *\n> > + * The canary provides an insight that the object is not valid and shall be\n> > + * rejected by libcamera API calls.\n> > + *\n> > + * \\return True if the canary has died, and the object shall not be trusted\n> > + */\n> > +bool Request::canary() const\n> > +{\n> > +\treturn canary_ != REQUEST_CANARY;\n> > +}\n> > +\n> > /**\n> > * \\brief Insert a text representation of a Request into an output stream\n> > * \\param[in] out The output stream\n> > @@ -601,6 +646,11 @@ std::ostream &operator<<(std::ostream &out, const Request &r)\n> > \t/* Pending, Completed, Cancelled(X). */\n> > \tstatic const char *statuses = \"PCX\";\n> > \n> > +\tif (r.canary()) {\n> > +\t\tout << \"Request(Invalid Canary)\";\n> > +\t\treturn out;\n> > +\t}\n> > +\n> > \t/* Example Output: Request(55:P:1/2:6523524) */\n> > \tout << \"Request(\" << r.sequence() << \":\" << statuses[r.status()] << \":\"\n> > \t << r._d()->pending_.size() << \"/\" << r.buffers().size() << \":\" \n> \n> \n> Hi,\n> \n> I ran into a version of this problem where some code refactored using a few smaller code stopped being correct when taken as a whole. This kind of a canary would have probably prevented that. A small reproducer is here:\n> \n> https://source.puri.sm/dorota.czaplejewicz/simplecam/-/commits/bug_time\n> \n> In this case, the problem is accessing the local reference to the request after the (same) owner just released it. While I prefer to steer people to not make mistakes in the first place (like I mentioned in https://bugs.libcamera.org/show_bug.cgi?id=186 ), I'm not familiar enough with C++ to know if such a use-after-free can be caught at compile time.\n\nThinking about this a bit harder, the problem can be avoided by breaking up the connection between the local reference, and the freed reference. It was already proposed to pass the request by value. The local value would be standalone, so it would be harder to free it.\n\nAnother way to solve it would be to notice that the same request object comes from two sources to the buggy handler: on one side it's accessible due to the vector which actually owns the object, and then again, libcamera passes its own reference to the handler. A solution would make it impossible to free the object unilaterally from the handler as long as the reference exists. This can be done by wrapping the owned Request in a shared_ptr, and keeping another shared reference in libcamera for as long as it's needed for signals. (Once that's done, the \"owning\" vector will probably be unnecessary, and this will end up looking very similar to the pass-by-value case).\n\nNeither would make misuse impossible, but they would make it hard to do by accident.\n\n--Dorota\n> \n> Cheers,\n> Dorota","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7038DBE080\n\tfor ;\n\tTue, 14 Mar 2023 10:01:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ADFED62705;\n\tTue, 14 Mar 2023 11:01:38 +0100 (CET)","from comms.puri.sm (comms.puri.sm [159.203.221.185])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BD02F626D2\n\tfor ;\n\tTue, 14 Mar 2023 11:01:35 +0100 (CET)","from localhost (localhost [127.0.0.1])\n\tby comms.puri.sm (Postfix) with ESMTP id 5789CE20AB;\n\tTue, 14 Mar 2023 03:01:34 -0700 (PDT)","from comms.puri.sm ([127.0.0.1])\n\tby localhost (comms.puri.sm [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id R7Ztrxv0SJJV; Tue, 14 Mar 2023 03:01:32 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678788098;\n\tbh=epsKezJCsUBuzhcuGseYL0oCMGL5QpW+lhQbloVD1lM=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=wJNTuFcWLl2CqSCLAS8XOWq8qxHxEfcPJzjaj3vUxLD5yyiAGubgqXgPpqWF+WYvb\n\t1t6FHru7LPQUsWXtPEU+ebN897saA+NsfemtdRoE3qImM0yyza/eCZ/ypKNT4mpNhF\n\tCM71bPEmzA2BtvaLowbqLEFHIJ8fV0dTOd6E0le/+FEOZnzNT6NY0NoON+R4MP3qx0\n\t6BK7gFXHOKIa9NNsgFCOAuIDV3uRLWQbrJVfk2qaW9ptJywFdIAEBh/axRcyVKgAri\n\tZqmy31isDGYOXzH+ibnpnnLJK07SqrGuxRuEw2w5MM5dJmTh+1hW85npElWym6UXs7\n\trIyb120mt5oEg==","v=1; a=rsa-sha256; c=relaxed/simple; d=puri.sm; s=comms;\n\tt=1678788092; bh=epsKezJCsUBuzhcuGseYL0oCMGL5QpW+lhQbloVD1lM=;\n\th=Date:From:To:Cc:Subject:In-Reply-To:References:From;\n\tb=j5D4gGacUUotr6BxelcHt6UipUodVlduNZDK9xO/jP1QevjpIPwQscz27rg6sPdah\n\tVY+8RrILa+DA/9fHcczAj14z4H24AQ9Ywyd5pj8CZZljhvNiGLXOf35PzvrAoickAV\n\trsjxYHjkS9f6hRcKoo+iO/jZvR46G+V15M7kzRlgBC6Seu4QMZMP3E1xVYrB8C079l\n\tI0/chVaxfCMBojk7k8VDgJ1QwyW4IsxPZbt+z503wXIFJvfKl6WGQjn723M/O3OpcA\n\t73j93dTci09UpLEwqjl8oXJAEC17GdX4WfgTObZ8z5Qp6G+0eOgPPWLEaItOSaXhr3\n\tTqU6WGWn+lt2A=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=puri.sm header.i=@puri.sm\n\theader.b=\"j5D4gGac\"; dkim-atps=neutral","Date":"Tue, 14 Mar 2023 11:01:18 +0100","To":"Kieran Bingham ","Message-ID":"<20230314110118.4ffd4b25.dorota.czaplejewicz@puri.sm>","In-Reply-To":"<20230313184317.608bdab4.dorota.czaplejewicz@puri.sm>","References":"<20230130180244.2150205-1-kieran.bingham@ideasonboard.com>\n\t<20230130180244.2150205-3-kieran.bingham@ideasonboard.com>\n\t<20230313184317.608bdab4.dorota.czaplejewicz@puri.sm>","Organization":"Purism","MIME-Version":"1.0","Content-Type":"multipart/signed; boundary=\"Sig_/DPfxm+DGmplyXKtu3GiMJoE\";\n\tprotocol=\"application/pgp-signature\"; micalg=pgp-sha256","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: request: A request\n\tcanary","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Dorota Czaplejewicz via libcamera-devel\n\t","Reply-To":"Dorota Czaplejewicz ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]