[{"id":37381,"web_url":"https://patchwork.libcamera.org/comment/37381/","msgid":"<6acc889a-6a4b-465a-8c53-f17cb4616630@ideasonboard.com>","date":"2025-12-15T12:21:17","subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 02/12/2025 14:49, Jacopo Mondi wrote:\n> The list of controls part of a Request is initialized with the\n> ControlInfoMap of the Camera the Request is created from.\n> \n> Applications can re-assign the controls list in a Request\n> which could cause issues during serialization.\n> \n> Validate that the ControlList in a Request is valid when\n> the Request is queued to the Camera by inspecting the\n> ControlInfoMap pointer.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/camera.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 2e1e146a25b13b94f3a0df5935c0861f78c949ed..93cd4d5f334f982133a12cbc4caec5cdf7bb52c6 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1345,6 +1345,12 @@ int Camera::queueRequest(Request *request)\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\t/* Make sure the Request has a valid control list. */\n> +\tif (request->controls().infoMap() != &controls()) {\n> +\t\tLOG(Camera, Error) << \"Invalid control list in the Request\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n\nSo this is a pointer comparison rather than a _contents_ comparison right? Could we not prevent the \npointer from being overwritten?\n\nThanks\nDan\n\n> \t/*\n> \t * The camera state may change until the end of the function. No locking\n> \t * is however needed as PipelineHandler::queueRequest() will handle\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 3DC53BD7D8\n\tfor ;\n\tMon, 15 Dec 2025 12:21:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 831E261980;\n\tMon, 15 Dec 2025 13:21:22 +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 BC588615B2\n\tfor ;\n\tMon, 15 Dec 2025 13:21:20 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9FEF5465;\n\tMon, 15 Dec 2025 13:21:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DdEzWgt9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765801275;\n\tbh=9y+FoImPuRWk5tramH62f5r6ez3QN5w3VKcc/6hLkgg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=DdEzWgt9cyLXRIwa/gZjKtN1TxoxNdBQ5gSiczSsOMb+mIx3Rmil8otDttgjIOEEt\n\tJf+0znKZ0RSd4yL8DvFJWbyST2Hda4wuTvKhtVwlWJBdO41/BmHkt9rOqA2SNZvJwm\n\tD4RImamGidjDjmdMYfTmctOqIKwXha2yD+iMMsgY=","Message-ID":"<6acc889a-6a4b-465a-8c53-f17cb4616630@ideasonboard.com>","Date":"Mon, 15 Dec 2025 12:21:17 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251202-cam-control-override-v3-0-eacab052798d@ideasonboard.com>\n\t<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","In-Reply-To":"<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37382,"web_url":"https://patchwork.libcamera.org/comment/37382/","msgid":"<3ff7rovplwemnluzpfx2krppn5hzpqmo6aihcyeqlchusspku4@dzccvxxpwuss>","date":"2025-12-15T12:30:21","subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Mon, Dec 15, 2025 at 12:21:17PM +0000, Dan Scally wrote:\n> Hi Jacopo\n>\n> On 02/12/2025 14:49, Jacopo Mondi wrote:\n> > The list of controls part of a Request is initialized with the\n> > ControlInfoMap of the Camera the Request is created from.\n> >\n> > Applications can re-assign the controls list in a Request\n> > which could cause issues during serialization.\n> >\n> > Validate that the ControlList in a Request is valid when\n> > the Request is queued to the Camera by inspecting the\n> > ControlInfoMap pointer.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/camera.cpp | 6 ++++++\n> > 1 file changed, 6 insertions(+)\n> >\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 2e1e146a25b13b94f3a0df5935c0861f78c949ed..93cd4d5f334f982133a12cbc4caec5cdf7bb52c6 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -1345,6 +1345,12 @@ int Camera::queueRequest(Request *request)\n> > \t\treturn -EINVAL;\n> > \t}\n> > +\t/* Make sure the Request has a valid control list. */\n> > +\tif (request->controls().infoMap() != &controls()) {\n> > +\t\tLOG(Camera, Error) << \"Invalid control list in the Request\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n>\n> So this is a pointer comparison rather than a _contents_ comparison right?\n\nYes\n\n> Could we not prevent the pointer from being overwritten?\n\nI tried, by\n-\tControlList &controls() { return *controls_; }\n+\tconst ControlList &controls() const { return *controls_; }\n\nhttps://patchwork.libcamera.org/patch/25212/\n\n\n>\n> Thanks\n> Dan\n>\n> > \t/*\n> > \t * The camera state may change until the end of the function. No locking\n> > \t * is however needed as PipelineHandler::queueRequest() will handle\n> >\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 28003C3257\n\tfor ;\n\tMon, 15 Dec 2025 12:30:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18C1961985;\n\tMon, 15 Dec 2025 13:30:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B4CB615B2\n\tfor ;\n\tMon, 15 Dec 2025 13:30:25 +0100 (CET)","from ideasonboard.com (mob-5-90-49-39.net.vodafone.it [5.90.49.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B39B6465;\n\tMon, 15 Dec 2025 13:30:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CL9lyWF7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765801820;\n\tbh=WF9jpbgehKyZkmh3UAotS6NL7SXaI6K8Q8NdQwqnDqw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CL9lyWF7R4scn42j1tKaMQgO+yBvET+ZfpAF0ZkeP3FIhBZyiYz5C1Dv5fcj8Frqi\n\t7hS61jSQYnkVnMezpPDC/HRPQst6AH3Y0qWend4T9p0jtJ/6bVggW9l/sVDkzMPPky\n\t8A18rFCS8/G0NMi2BPgbVQaBK44oVQuu+2jB2FaI=","Date":"Mon, 15 Dec 2025 13:30:21 +0100","From":"Jacopo Mondi ","To":"Dan Scally ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","Message-ID":"<3ff7rovplwemnluzpfx2krppn5hzpqmo6aihcyeqlchusspku4@dzccvxxpwuss>","References":"<20251202-cam-control-override-v3-0-eacab052798d@ideasonboard.com>\n\t<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>\n\t<6acc889a-6a4b-465a-8c53-f17cb4616630@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6acc889a-6a4b-465a-8c53-f17cb4616630@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37384,"web_url":"https://patchwork.libcamera.org/comment/37384/","msgid":"<67b2d2d6-5bb6-48a0-a8fc-94a28dd14c6e@ideasonboard.com>","date":"2025-12-15T14:08:10","subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 15/12/2025 12:30, Jacopo Mondi wrote:\n> Hi Dan\n> \n> On Mon, Dec 15, 2025 at 12:21:17PM +0000, Dan Scally wrote:\n>> Hi Jacopo\n>>\n>> On 02/12/2025 14:49, Jacopo Mondi wrote:\n>>> The list of controls part of a Request is initialized with the\n>>> ControlInfoMap of the Camera the Request is created from.\n>>>\n>>> Applications can re-assign the controls list in a Request\n>>> which could cause issues during serialization.\n>>>\n>>> Validate that the ControlList in a Request is valid when\n>>> the Request is queued to the Camera by inspecting the\n>>> ControlInfoMap pointer.\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/libcamera/camera.cpp | 6 ++++++\n>>> 1 file changed, 6 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>> index 2e1e146a25b13b94f3a0df5935c0861f78c949ed..93cd4d5f334f982133a12cbc4caec5cdf7bb52c6 100644\n>>> --- a/src/libcamera/camera.cpp\n>>> +++ b/src/libcamera/camera.cpp\n>>> @@ -1345,6 +1345,12 @@ int Camera::queueRequest(Request *request)\n>>> \t\treturn -EINVAL;\n>>> \t}\n>>> +\t/* Make sure the Request has a valid control list. */\n>>> +\tif (request->controls().infoMap() != &controls()) {\n>>> +\t\tLOG(Camera, Error) << \"Invalid control list in the Request\";\n>>> +\t\treturn -EINVAL;\n>>> +\t}\n>>> +\n>>\n>> So this is a pointer comparison rather than a _contents_ comparison right?\n> \n> Yes\n> \n>> Could we not prevent the pointer from being overwritten?\n> \n> I tried, by\n> -\tControlList &controls() { return *controls_; }\n> +\tconst ControlList &controls() const { return *controls_; }\n> \n> https://patchwork.libcamera.org/patch/25212/\n\nAh-ha...ok. Hmm, I feel like this is the same change by proxy, really. If the goal is to make sure \nthat the Request is only queued with controls that are valid for the Camera, what about something like:\n\n\t/* Make sure the Request has a valid control list. */\n\tfor (auto control : request->controls()) {\n\t\tif (!d->validator()->validate(control.first)) {\n\t\t\tLOG(Camera, Error) << \"Invalid control in Request\";\n\t\t\treturn -EINVAL;\n\t\t}\n\t}\n\nThe Request's ControlList is created with the Camera's validator by default, so unless the \napplication has replaced the pointer validation should work here too.\n\n\n\n> \n> \n>>\n>> Thanks\n>> Dan\n>>\n>>> \t/*\n>>> \t * The camera state may change until the end of the function. No locking\n>>> \t * is however needed as PipelineHandler::queueRequest() will handle\n>>>\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 4E6EBBD7D8\n\tfor ;\n\tMon, 15 Dec 2025 14:08:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 31203619C6;\n\tMon, 15 Dec 2025 15:08:15 +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 7CBAB615B2\n\tfor ;\n\tMon, 15 Dec 2025 15:08:13 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6C7FBC66;\n\tMon, 15 Dec 2025 15:08:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hu43M6Dz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765807688;\n\tbh=6wp9h1plcF8J8VeSz3gC66DI98wzYRCzLkei95VhsZk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=hu43M6DzrGkS3/p6DpIzcLIYRwM5/3FSZzNqebKlYNP+BKvJDtXlhNUw7hjOZy/di\n\tiFHhyONd8g7GmC0kJwEvINCPmzAFYleKHGy+eZ3Scf7HOMlAcYDLfvaign0Vf7gGd3\n\tv1knN4KAT4avq0waNSLWnrVASpi5eqOkwfpDWOWQ=","Message-ID":"<67b2d2d6-5bb6-48a0-a8fc-94a28dd14c6e@ideasonboard.com>","Date":"Mon, 15 Dec 2025 14:08:10 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251202-cam-control-override-v3-0-eacab052798d@ideasonboard.com>\n\t<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>\n\t<6acc889a-6a4b-465a-8c53-f17cb4616630@ideasonboard.com>\n\t<3ff7rovplwemnluzpfx2krppn5hzpqmo6aihcyeqlchusspku4@dzccvxxpwuss>","Content-Language":"en-US","From":"Dan Scally ","In-Reply-To":"<3ff7rovplwemnluzpfx2krppn5hzpqmo6aihcyeqlchusspku4@dzccvxxpwuss>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37387,"web_url":"https://patchwork.libcamera.org/comment/37387/","msgid":"","date":"2025-12-15T15:49:54","subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Mon, Dec 15, 2025 at 02:08:10PM +0000, Dan Scally wrote:\n> Hi Jacopo\n>\n> On 15/12/2025 12:30, Jacopo Mondi wrote:\n> > Hi Dan\n> >\n> > On Mon, Dec 15, 2025 at 12:21:17PM +0000, Dan Scally wrote:\n> > > Hi Jacopo\n> > >\n> > > On 02/12/2025 14:49, Jacopo Mondi wrote:\n> > > > The list of controls part of a Request is initialized with the\n> > > > ControlInfoMap of the Camera the Request is created from.\n> > > >\n> > > > Applications can re-assign the controls list in a Request\n> > > > which could cause issues during serialization.\n> > > >\n> > > > Validate that the ControlList in a Request is valid when\n> > > > the Request is queued to the Camera by inspecting the\n> > > > ControlInfoMap pointer.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/libcamera/camera.cpp | 6 ++++++\n> > > > 1 file changed, 6 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > index 2e1e146a25b13b94f3a0df5935c0861f78c949ed..93cd4d5f334f982133a12cbc4caec5cdf7bb52c6 100644\n> > > > --- a/src/libcamera/camera.cpp\n> > > > +++ b/src/libcamera/camera.cpp\n> > > > @@ -1345,6 +1345,12 @@ int Camera::queueRequest(Request *request)\n> > > > \t\treturn -EINVAL;\n> > > > \t}\n> > > > +\t/* Make sure the Request has a valid control list. */\n> > > > +\tif (request->controls().infoMap() != &controls()) {\n> > > > +\t\tLOG(Camera, Error) << \"Invalid control list in the Request\";\n> > > > +\t\treturn -EINVAL;\n> > > > +\t}\n> > > > +\n> > >\n> > > So this is a pointer comparison rather than a _contents_ comparison right?\n> >\n> > Yes\n> >\n> > > Could we not prevent the pointer from being overwritten?\n> >\n> > I tried, by\n> > -\tControlList &controls() { return *controls_; }\n> > +\tconst ControlList &controls() const { return *controls_; }\n> >\n> > https://patchwork.libcamera.org/patch/25212/\n>\n> Ah-ha...ok. Hmm, I feel like this is the same change by proxy, really. If\n> the goal is to make sure that the Request is only queued with controls that\n> are valid for the Camera, what about something like:\n\nThe purpose is for application not to override the control list in a\nrequest, as they can currently do\n\n request->controls() = { anything }\n\nand that's what 'cam' was doing and breaking serialization as a\nconsequence.\n\n>\n> \t/* Make sure the Request has a valid control list. */\n> \tfor (auto control : request->controls()) {\n> \t\tif (!d->validator()->validate(control.first)) {\n> \t\t\tLOG(Camera, Error) << \"Invalid control in Request\";\n> \t\t\treturn -EINVAL;\n> \t\t}\n> \t}\n\nA request control list is already created with the camera control\nvalidator\n\n\tcontrols_ = new ControlList(controls::controls,\n\t\t\t\t camera->_d()->validator());\n\nand validator->validate() is already called everytime a control is\nlooked up\n\nControlValue *ControlList::find(unsigned int id)\n{\n\tif (validator_ && !validator_->validate(id)) {\n\t\tLOG(Controls, Error)\n\t\t\t<< \"Control \" << utils::hex(id)\n\t\t\t<< \" is not valid for \" << validator_->name();\n\t\treturn nullptr;\n\t}\n\n>\n> The Request's ControlList is created with the Camera's validator by default,\n> so unless the application has replaced the pointer validation should work\n> here too.\n\nExactly, applications can overwrite the request->controls() list.\n\nIf they populate the new list with valid controls, the above check\nwill pass, but the control list could not have a valid control info map.\n\nI agree what's implemented here is a minimal protection, I would have\nliked to go stricter and remove rw access to Request::controls() and\ngo through an API to set controls in a request, but as that wasn't\napreciated I think that's the minimum we should do.\n\n>\n>\n>\n> >\n> >\n> > >\n> > > Thanks\n> > > Dan\n> > >\n> > > > \t/*\n> > > > \t * The camera state may change until the end of the function. No locking\n> > > > \t * is however needed as PipelineHandler::queueRequest() will handle\n> > > >\n> > >\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 44360BD7D8\n\tfor ;\n\tMon, 15 Dec 2025 15:50:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 340B3619E9;\n\tMon, 15 Dec 2025 16:50:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D40A2615B2\n\tfor ;\n\tMon, 15 Dec 2025 16:49:58 +0100 (CET)","from ideasonboard.com (mob-5-90-49-39.net.vodafone.it [5.90.49.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 74BE1C66;\n\tMon, 15 Dec 2025 16:49:53 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RW8bZkdW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765813793;\n\tbh=9rym3JZWkxXy+zBwqz8h0hnqzS1rI401V6Hr+JHr6tI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RW8bZkdWgzB2sftSUWJtB0ftjfxw9pPlr/dzTZQFmjZzp4yQpVhwzY8oU2hLEmbq+\n\te6WokKLhTJQQUq502wDazKTwOU0UjT1Y8qSN9b1IKtrJYHk3bCtf2XLtxTdVbc07eI\n\tKjOHUMqpkGV0of8Mv8UK4u/p9/S44fXYeMgqTlMw=","Date":"Mon, 15 Dec 2025 16:49:54 +0100","From":"Jacopo Mondi ","To":"Dan Scally ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","Message-ID":"","References":"<20251202-cam-control-override-v3-0-eacab052798d@ideasonboard.com>\n\t<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>\n\t<6acc889a-6a4b-465a-8c53-f17cb4616630@ideasonboard.com>\n\t<3ff7rovplwemnluzpfx2krppn5hzpqmo6aihcyeqlchusspku4@dzccvxxpwuss>\n\t<67b2d2d6-5bb6-48a0-a8fc-94a28dd14c6e@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<67b2d2d6-5bb6-48a0-a8fc-94a28dd14c6e@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37402,"web_url":"https://patchwork.libcamera.org/comment/37402/","msgid":"<4291a9cf-7b3b-40a6-953d-d896e1c7142a@ideasonboard.com>","date":"2025-12-15T23:37:12","subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 15/12/2025 15:49, Jacopo Mondi wrote:\n> Hi Dan\n> \n> On Mon, Dec 15, 2025 at 02:08:10PM +0000, Dan Scally wrote:\n>> Hi Jacopo\n>>\n>> On 15/12/2025 12:30, Jacopo Mondi wrote:\n>>> Hi Dan\n>>>\n>>> On Mon, Dec 15, 2025 at 12:21:17PM +0000, Dan Scally wrote:\n>>>> Hi Jacopo\n>>>>\n>>>> On 02/12/2025 14:49, Jacopo Mondi wrote:\n>>>>> The list of controls part of a Request is initialized with the\n>>>>> ControlInfoMap of the Camera the Request is created from.\n>>>>>\n>>>>> Applications can re-assign the controls list in a Request\n>>>>> which could cause issues during serialization.\n>>>>>\n>>>>> Validate that the ControlList in a Request is valid when\n>>>>> the Request is queued to the Camera by inspecting the\n>>>>> ControlInfoMap pointer.\n>>>>>\n>>>>> Signed-off-by: Jacopo Mondi \n>>>>> ---\n>>>>> src/libcamera/camera.cpp | 6 ++++++\n>>>>> 1 file changed, 6 insertions(+)\n>>>>>\n>>>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>>>> index 2e1e146a25b13b94f3a0df5935c0861f78c949ed..93cd4d5f334f982133a12cbc4caec5cdf7bb52c6 100644\n>>>>> --- a/src/libcamera/camera.cpp\n>>>>> +++ b/src/libcamera/camera.cpp\n>>>>> @@ -1345,6 +1345,12 @@ int Camera::queueRequest(Request *request)\n>>>>> \t\treturn -EINVAL;\n>>>>> \t}\n>>>>> +\t/* Make sure the Request has a valid control list. */\n>>>>> +\tif (request->controls().infoMap() != &controls()) {\n>>>>> +\t\tLOG(Camera, Error) << \"Invalid control list in the Request\";\n>>>>> +\t\treturn -EINVAL;\n>>>>> +\t}\n>>>>> +\n>>>>\n>>>> So this is a pointer comparison rather than a _contents_ comparison right?\n>>>\n>>> Yes\n>>>\n>>>> Could we not prevent the pointer from being overwritten?\n>>>\n>>> I tried, by\n>>> -\tControlList &controls() { return *controls_; }\n>>> +\tconst ControlList &controls() const { return *controls_; }\n>>>\n>>> https://patchwork.libcamera.org/patch/25212/\n>>\n>> Ah-ha...ok. Hmm, I feel like this is the same change by proxy, really. If\n>> the goal is to make sure that the Request is only queued with controls that\n>> are valid for the Camera, what about something like:\n> \n> The purpose is for application not to override the control list in a\n> request, as they can currently do\n> \n> request->controls() = { anything }\n> \n> and that's what 'cam' was doing and breaking serialization as a\n> consequence.\n> \n>>\n>> \t/* Make sure the Request has a valid control list. */\n>> \tfor (auto control : request->controls()) {\n>> \t\tif (!d->validator()->validate(control.first)) {\n>> \t\t\tLOG(Camera, Error) << \"Invalid control in Request\";\n>> \t\t\treturn -EINVAL;\n>> \t\t}\n>> \t}\n> \n> A request control list is already created with the camera control\n> validator\n> \n> \tcontrols_ = new ControlList(controls::controls,\n> \t\t\t\t camera->_d()->validator());\n> \n> and validator->validate() is already called everytime a control is\n> looked up\n> \n> ControlValue *ControlList::find(unsigned int id)\n> {\n> \tif (validator_ && !validator_->validate(id)) {\n> \t\tLOG(Controls, Error)\n> \t\t\t<< \"Control \" << utils::hex(id)\n> \t\t\t<< \" is not valid for \" << validator_->name();\n> \t\treturn nullptr;\n> \t}\n> \n>>\n>> The Request's ControlList is created with the Camera's validator by default,\n>> so unless the application has replaced the pointer validation should work\n>> here too.\n> \n> Exactly, applications can overwrite the request->controls() list.\n> \n> If they populate the new list with valid controls, the above check\n> will pass, but the control list could not have a valid control info map.\n> \n> I agree what's implemented here is a minimal protection, I would have\n> liked to go stricter and remove rw access to Request::controls() and\n> go through an API to set controls in a request, but as that wasn't\n> apreciated I think that's the minimum we should do.\n\nAh - sorry; I should have read the issue from the cover letter and the thread you linked in full \nbefore replying. I'm sold now:\n\nReviewed-by: Daniel Scally \n\n> \n>>\n>>\n>>\n>>>\n>>>\n>>>>\n>>>> Thanks\n>>>> Dan\n>>>>\n>>>>> \t/*\n>>>>> \t * The camera state may change until the end of the function. No locking\n>>>>> \t * is however needed as PipelineHandler::queueRequest() will handle\n>>>>>\n>>>>\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 4BA58C3257\n\tfor ;\n\tMon, 15 Dec 2025 23:37:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E64E619F9;\n\tTue, 16 Dec 2025 00:37:18 +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 239EC61613\n\tfor ;\n\tTue, 16 Dec 2025 00:37:16 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8F7E96AC;\n\tTue, 16 Dec 2025 00:37:10 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"InYF29NF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765841830;\n\tbh=mmNAUWaucQVilNS1p5NDS3QbozggMovH8ce68mup488=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=InYF29NF2cddutXhk7kIvznTa4mnRMnNfUjoLYoQOmxtAEUrBAHZAF0D7cd1rzJn2\n\tiHd6SZM4aQxF8WsN/y2acCuApjUE+WCmFMncXEI1VayofKgYlM4BRZflVOyn1OB1ap\n\tInMRY0NHfrCdox1VSXQ+Rb6SD3YTZ32KrornMgz8=","Message-ID":"<4291a9cf-7b3b-40a6-953d-d896e1c7142a@ideasonboard.com>","Date":"Mon, 15 Dec 2025 23:37:12 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251202-cam-control-override-v3-0-eacab052798d@ideasonboard.com>\n\t<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>\n\t<6acc889a-6a4b-465a-8c53-f17cb4616630@ideasonboard.com>\n\t<3ff7rovplwemnluzpfx2krppn5hzpqmo6aihcyeqlchusspku4@dzccvxxpwuss>\n\t<67b2d2d6-5bb6-48a0-a8fc-94a28dd14c6e@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Dan Scally ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37405,"web_url":"https://patchwork.libcamera.org/comment/37405/","msgid":"<0025e65e-9174-43f3-abf4-346822d5f023@ideasonboard.com>","date":"2025-12-16T09:07:05","subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 12. 02. 15:49 keltezéssel, Jacopo Mondi írta:\n> The list of controls part of a Request is initialized with the\n> ControlInfoMap of the Camera the Request is created from.\n> \n> Applications can re-assign the controls list in a Request\n> which could cause issues during serialization.\n> \n> Validate that the ControlList in a Request is valid when\n> the Request is queued to the Camera by inspecting the\n> ControlInfoMap pointer.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/camera.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 2e1e146a25b13b94f3a0df5935c0861f78c949ed..93cd4d5f334f982133a12cbc4caec5cdf7bb52c6 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1345,6 +1345,12 @@ int Camera::queueRequest(Request *request)\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\t/* Make sure the Request has a valid control list. */\n> +\tif (request->controls().infoMap() != &controls()) {\n> +\t\tLOG(Camera, Error) << \"Invalid control list in the Request\";\n\nI feel a bit like this message could be more helpful:\n\n Request::controls() seems to have been overwritten. This is not allowed.\n\nor something like that. Otherwise in my opinion it is somewhat\ndifficult to determine the cause for an app developer.\n\n\nReviewed-by: Barnabás Pőcze \n\n\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \t/*\n> \t * The camera state may change until the end of the function. No locking\n> \t * is however needed as PipelineHandler::queueRequest() will handle\n> \n> --\n> 2.51.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 E1AA5C3257\n\tfor ;\n\tTue, 16 Dec 2025 09:07:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DFF7C61A03;\n\tTue, 16 Dec 2025 10:07:10 +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 09CDA61603\n\tfor ;\n\tTue, 16 Dec 2025 10:07:09 +0100 (CET)","from [192.168.33.23] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 396317CE;\n\tTue, 16 Dec 2025 10:07:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lW+U7FNB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765876023;\n\tbh=EwS0FVPbRM152fEi5AsbaElZTuc0BErfv/trm60D2o4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=lW+U7FNB1x9OxBWPTPO5q0PR5kXlmJM/GmW9dcb3MGCa3rMji+hkdh7jWimn11J8K\n\tvYntXewxRO/uiVYq4g7aUlEGQcqAwXbpJ8BP0QnoCNgLgzp9dDQE8bEDhh52Z25l8X\n\tUNX5ssb9ZXlRujt2d4qcpxK95mIMKcPGZKCxb3uk=","Message-ID":"<0025e65e-9174-43f3-abf4-346822d5f023@ideasonboard.com>","Date":"Tue, 16 Dec 2025 10:07:05 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 5/5] libcamera: camera: Ensure a request's controls\n\tare valid","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251202-cam-control-override-v3-0-eacab052798d@ideasonboard.com>\n\t\n\t<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251202-cam-control-override-v3-5-eacab052798d@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]