[{"id":32828,"web_url":"https://patchwork.libcamera.org/comment/32828/","msgid":"<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>","date":"2024-12-17T16:23:27","subject":"Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace presence\n\tbefore dereference","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Antoine Bouyer (2024-12-17 14:02:39)\n> During configuration stage, kms_sink doesn't check optional colorSpace\n> parameter is valid while dereferencing it. Then it leads to a crash\n> with below signature if parameter is not defined:\n> \n>   /opt/fsl-imx-internal-xwayland/6.12-styhead/sysroots/armv8a-poky-linux/usr/include/c++/14.2.0/optional:48\n>   2: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = libcamera::ColorSp\n>   ace; _Dp = std::_Optional_base<libcamera::ColorSpace, true, true>]: Assertion 'this->_M_is_engaged()' fai\n>   led.\n>   Aborted (core dumped)\n> \n> This behavior is confirmed in the \"operator->\" page:\n>  (https://en.cppreference.com/w/cpp/utility/optional/operator*)\n> \n> \"This operator does not check whether the optional contains a value!\"\n> \n> This crash is observed with styhead's gcc compiler (version 14.2.0),\n> while it was not with previous scarthgap's gcc compiler (version 13.2.0).\n> \n> Use has_value() as a fix to make sure this property exists and prevent\n> crash.\n> \n> Fixes: 6d7ddace5278 (\"cam: Add KMS sink class\")\n> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\nThanks for the updates that I suggested, however I'm weary that perhaps\nLaurent was suggesting this /shouldn't/ be possible to happen and should\nbe fixed in the pipeline handler? (and perhaps enforced?, and maybe\nguaranteed to be set to RAW if unknown?)\n\nWe could perhaps have a common path in the configuration that sets RAW\nwhen the pipeline handler didn't set it ? Or maybe we enforce it must be\nset by lc-compliance or such...\n\n\n\n> ---\n>  src/apps/cam/kms_sink.cpp | 3 ++-\n>  1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp\n> index 672c985..8f3b867 100644\n> --- a/src/apps/cam/kms_sink.cpp\n> +++ b/src/apps/cam/kms_sink.cpp\n> @@ -153,7 +153,8 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n>         colorEncoding_ = std::nullopt;\n>         colorRange_ = std::nullopt;\n>  \n> -       if (cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n> +       if (!cfg.colorSpace.has_value() ||\n> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>                 return 0;\n>  \n>         /*\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 88178C32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 16:23:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3B5E367FE8;\n\tTue, 17 Dec 2024 17:23:32 +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 B8D0E67FDF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 17:23:30 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7560D4C7;\n\tTue, 17 Dec 2024 17:22: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=\"TA2njkib\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734452573;\n\tbh=KfQj3WEpmXNPhcwaU9vrjDbGDUvB+FPisBw/LZroiTQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=TA2njkibDdetvt4bGI2Rbyq+pQCiSfkc87mXkPvcvnN0fQWanKQ5E0MTKPUtgYsNl\n\tKfVAW21eMiis+R1L43TjNx819Js04m4Prm53JicuRhPhw4aUBKKSltwDkXwpgte8n5\n\toCGEAYM9MAZqi/Ov1W6F2ukV0AduboT4twKnyU6o=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241217140238.215572-2-antoine.bouyer@nxp.com>","References":"<20241217140238.215572-1-antoine.bouyer@nxp.com>\n\t<20241217140238.215572-2-antoine.bouyer@nxp.com>","Subject":"Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace presence\n\tbefore dereference","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"julien.vuillaumier@nxp.com, Antoine Bouyer <antoine.bouyer@nxp.com>","To":"Antoine Bouyer <antoine.bouyer@nxp.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 17 Dec 2024 16:23:27 +0000","Message-ID":"<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32835,"web_url":"https://patchwork.libcamera.org/comment/32835/","msgid":"<20241217164650.GH20432@pendragon.ideasonboard.com>","date":"2024-12-17T16:46:50","subject":"Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace presence\n\tbefore dereference","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Dec 17, 2024 at 04:23:27PM +0000, Kieran Bingham wrote:\n> Quoting Antoine Bouyer (2024-12-17 14:02:39)\n> > During configuration stage, kms_sink doesn't check optional colorSpace\n> > parameter is valid while dereferencing it. Then it leads to a crash\n> > with below signature if parameter is not defined:\n> > \n> >   /opt/fsl-imx-internal-xwayland/6.12-styhead/sysroots/armv8a-poky-linux/usr/include/c++/14.2.0/optional:48\n> >   2: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = libcamera::ColorSp\n> >   ace; _Dp = std::_Optional_base<libcamera::ColorSpace, true, true>]: Assertion 'this->_M_is_engaged()' fai\n> >   led.\n> >   Aborted (core dumped)\n> > \n> > This behavior is confirmed in the \"operator->\" page:\n> >  (https://en.cppreference.com/w/cpp/utility/optional/operator*)\n> > \n> > \"This operator does not check whether the optional contains a value!\"\n> > \n> > This crash is observed with styhead's gcc compiler (version 14.2.0),\n> > while it was not with previous scarthgap's gcc compiler (version 13.2.0).\n> > \n> > Use has_value() as a fix to make sure this property exists and prevent\n> > crash.\n> > \n> > Fixes: 6d7ddace5278 (\"cam: Add KMS sink class\")\n> > Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> Thanks for the updates that I suggested, however I'm weary that perhaps\n> Laurent was suggesting this /shouldn't/ be possible to happen and should\n> be fixed in the pipeline handler? (and perhaps enforced?, and maybe\n> guaranteed to be set to RAW if unknown?)\n> \n> We could perhaps have a common path in the configuration that sets RAW\n> when the pipeline handler didn't set it ? Or maybe we enforce it must be\n> set by lc-compliance or such...\n\nLet's see first, Antoine, which pipeline handler have you encountered\nthis issue with ?\n\n> > ---\n> >  src/apps/cam/kms_sink.cpp | 3 ++-\n> >  1 file changed, 2 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp\n> > index 672c985..8f3b867 100644\n> > --- a/src/apps/cam/kms_sink.cpp\n> > +++ b/src/apps/cam/kms_sink.cpp\n> > @@ -153,7 +153,8 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n> >         colorEncoding_ = std::nullopt;\n> >         colorRange_ = std::nullopt;\n> >  \n> > -       if (cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n> > +       if (!cfg.colorSpace.has_value() ||\n> > +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n> >                 return 0;\n> >  \n> >         /*","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DFC0EBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 16:46:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C994867FE9;\n\tTue, 17 Dec 2024 17:46:54 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5528767FD3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 17:46:52 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BC0AE4C7;\n\tTue, 17 Dec 2024 17:46:14 +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=\"mlHDLbkm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734453974;\n\tbh=i6ft+Qo2KY4X17CPpXvBDiNWbMjAinc4rnt5Cgqc5nc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mlHDLbkmZbfPxnXzDgJXdi2d+cWrVtCg793fRApqus83vSpTJwOskJj8VjylQzRO6\n\t1/yjEYSRDyj0hYxT5E6kiJRRGxitJMYvqJ52Fo6kCqY110xr271wKUh7WtT3jGFMYA\n\tUSDQ1dCUjZ7QFBJmjYgqb54ETC2mAs6SFmUrhp1g=","Date":"Tue, 17 Dec 2024 18:46:50 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Antoine Bouyer <antoine.bouyer@nxp.com>,\n\tlibcamera-devel@lists.libcamera.org, julien.vuillaumier@nxp.com","Subject":"Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace presence\n\tbefore dereference","Message-ID":"<20241217164650.GH20432@pendragon.ideasonboard.com>","References":"<20241217140238.215572-1-antoine.bouyer@nxp.com>\n\t<20241217140238.215572-2-antoine.bouyer@nxp.com>\n\t<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32839,"web_url":"https://patchwork.libcamera.org/comment/32839/","msgid":"<bde515ca-ec84-4630-b497-693828d29388@nxp.com>","date":"2024-12-17T16:59:06","subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","submitter":{"id":218,"url":"https://patchwork.libcamera.org/api/people/218/","name":"Antoine Bouyer","email":"antoine.bouyer@nxp.com"},"content":"On 17/12/2024 17:46, Laurent Pinchart wrote:\n\n> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n>\n>\n> On Tue, Dec 17, 2024 at 04:23:27PM +0000, Kieran Bingham wrote:\n>> Quoting Antoine Bouyer (2024-12-17 14:02:39)\n>>> During configuration stage, kms_sink doesn't check optional colorSpace\n>>> parameter is valid while dereferencing it. Then it leads to a crash\n>>> with below signature if parameter is not defined:\n>>>\n>>>    /opt/fsl-imx-internal-xwayland/6.12-styhead/sysroots/armv8a-poky-linux/usr/include/c++/14.2.0/optional:48\n>>>    2: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = libcamera::ColorSp\n>>>    ace; _Dp = std::_Optional_base<libcamera::ColorSpace, true, true>]: Assertion 'this->_M_is_engaged()' fai\n>>>    led.\n>>>    Aborted (core dumped)\n>>>\n>>> This behavior is confirmed in the \"operator->\" page:\n>>>   (https://en.cppreference.com/w/cpp/utility/optional/operator*)\n>>>\n>>> \"This operator does not check whether the optional contains a value!\"\n>>>\n>>> This crash is observed with styhead's gcc compiler (version 14.2.0),\n>>> while it was not with previous scarthgap's gcc compiler (version 13.2.0).\n>>>\n>>> Use has_value() as a fix to make sure this property exists and prevent\n>>> crash.\n>>>\n>>> Fixes: 6d7ddace5278 (\"cam: Add KMS sink class\")\n>>> Signed-off-by: Antoine Bouyer<antoine.bouyer@nxp.com>\n>>> Reviewed-by: Kieran Bingham<kieran.bingham@ideasonboard.com>\n>> Thanks for the updates that I suggested, however I'm weary that perhaps\n>> Laurent was suggesting this /shouldn't/ be possible to happen and should\n>> be fixed in the pipeline handler? (and perhaps enforced?, and maybe\n>> guaranteed to be set to RAW if unknown?)\n>>\n>> We could perhaps have a common path in the configuration that sets RAW\n>> when the pipeline handler didn't set it ? Or maybe we enforce it must be\n>> set by lc-compliance or such...\n> Let's see first, Antoine, which pipeline handler have you encountered\n> this issue with ?\n\nThis is with imx8-isi pipeline.\nActually I was thinking about such approach initially, then changed to has_value() fix\nbecause of \"optional\" flag.\n\n>\n>>> ---\n>>>   src/apps/cam/kms_sink.cpp | 3 ++-\n>>>   1 file changed, 2 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp\n>>> index 672c985..8f3b867 100644\n>>> --- a/src/apps/cam/kms_sink.cpp\n>>> +++ b/src/apps/cam/kms_sink.cpp\n>>> @@ -153,7 +153,8 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n>>>          colorEncoding_ = std::nullopt;\n>>>          colorRange_ = std::nullopt;\n>>>\n>>> -       if (cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>>> +       if (!cfg.colorSpace.has_value() ||\n>>> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>>>                  return 0;\n>>>\n>>>          /*\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nBest regards\n\nAntoine","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B704FBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 16:57:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 67C4B67FF8;\n\tTue, 17 Dec 2024 17:57:49 +0100 (CET)","from EUR03-AM7-obe.outbound.protection.outlook.com\n\t(mail-am7eur03on20601.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:260e::601])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C4A367FE2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 17:57:47 +0100 (CET)","from GVXPR04MB9831.eurprd04.prod.outlook.com (2603:10a6:150:11c::8)\n\tby DBAPR04MB7429.eurprd04.prod.outlook.com (2603:10a6:10:1a2::22)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8251.21;\n\tTue, 17 Dec 2024 16:57:46 +0000","from GVXPR04MB9831.eurprd04.prod.outlook.com\n\t([fe80::4634:3d9c:c4a:641a]) by\n\tGVXPR04MB9831.eurprd04.prod.outlook.com\n\t([fe80::4634:3d9c:c4a:641a%7]) with mapi id 15.20.8251.015;\n\tTue, 17 Dec 2024 16:57:45 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"E8TopidL\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=N3atrQMO3Vvf6rqFCNvvzGwgi+z9w3FOig2SydzRuzyVxUZlz9tXJDcsq5vNd4oZFKRAypI3y+681vzjdt31vbij8fmfamQ588Ofy/p7HnijXwKPoBwSAm4nb+G3ted88ygkSwjynMZ3pNZWgzNuh7b1Gkkx1kgqtJxHcimQu6IgIsbf5x9ymWC9Qfw9hGkewNMeTL94ZstZHXh5SsRQ20PyEMnXKSufSu2Mq2k0oaF+kLy9SpmAuwGdUd5clRn81tSosbRL/9SCFg0Qq9x+SxGMzoPzSSfvdTN2l7y3Zg2AHPSvyJeBb7U4eimh9l//TUB1T3Q/nOlxbbR+aegGkQ==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=9l39eteCgOYOT/UPGyg2Vg2G+Eip7KYdYE/HmAgVTX8=;\n\tb=trUcMYr/3g/4ZgY7as/2Ow2dR/9fl6TCibi5g+8jUYyHnOpudsmkFEloZKarZc03FDNpsT0Yy0CK3LP5MDLr+29LnvLVuKxdtC1gY9g5CV8NvZ/XRS3voxvLeTjvLopGGb9n7k0XQjeS4VdxFI++kWat9qU36bGIEHBP9WDJexGwZ0EiORXqOab0UPCBbFbEAcMgoUk+I+JSRmEJ1M+ZodrsO0viiko87dGGFvm3zyJsBA1mdYBURWGNxZnjfjxZVTOLWrc5S+tJLQstPXIUGQBr1vyD9B8Ien6nPT82l42CYp46ZaIh7eogvjpGaeIC/ErU0j86tdvEnVvZet+VXg==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=9l39eteCgOYOT/UPGyg2Vg2G+Eip7KYdYE/HmAgVTX8=;\n\tb=E8TopidLGuC5/XAHnl851r//ykz1HdYO87EK4WbydUoWt9kaxixkFbnPnTslFwFa9dZi8Cug61H9PXAGaOFu8OullA7LiX0RrI14rdQXmxYhaIdFBGxHwI2rU9+2YIxcUx3cs2LhPc83ZO7cp8yC/QQ2teGi4EKkZCDRdWmRXJQvLQJc8wkw27JobEZnjj1QaLXXrih0UW1Iboew2ijWr3Fgpwd7P7CtQ+bzlvsb6xTL6zW4LXGU6mBozGGYdtxgrKX/G8QDps4XHKjnv7HrYx0rAh28C1P6copetc1V02FZA74cHjaRc4nrn9eS8SbYnWVlVyNfiDoyhk17Y3T2uw==","Content-Type":"multipart/alternative;\n\tboundary=\"------------eqeDrwURxAri0qDlMvq3Qc0s\"","Message-ID":"<bde515ca-ec84-4630-b497-693828d29388@nxp.com>","Date":"Tue, 17 Dec 2024 17:59:06 +0100","User-Agent":"Mozilla Thunderbird","Subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, julien.vuillaumier@nxp.com","References":"<20241217140238.215572-1-antoine.bouyer@nxp.com>\n\t<20241217140238.215572-2-antoine.bouyer@nxp.com>\n\t<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>\n\t<20241217164650.GH20432@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Antoine Bouyer <antoine.bouyer@nxp.com>","In-Reply-To":"<20241217164650.GH20432@pendragon.ideasonboard.com>","X-ClientProxiedBy":"SJ2PR07CA0018.namprd07.prod.outlook.com\n\t(2603:10b6:a03:505::22) To GVXPR04MB9831.eurprd04.prod.outlook.com\n\t(2603:10a6:150:11c::8)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"GVXPR04MB9831:EE_|DBAPR04MB7429:EE_","X-MS-Office365-Filtering-Correlation-Id":"b161bd1f-328a-4256-a677-08dd1ebbedbd","X-LD-Processed":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635,ExtAddr","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0; ARA:13230040|366016|376014|1800799024|8096899003;","X-Microsoft-Antispam-Message-Info":"=?utf-8?q?1bi0g6+o9IF9dnCOQlHj+ikHuDFz?=\n\t=?utf-8?q?J4T2KRH7xrSVdhtNpdzXMu+5IoHA6T0bK8CZI9YA53jljElTJehZt71g?=\n\t=?utf-8?q?w81iizBSmDSnJtj3Yn7fAa5CbQLlEUd1dODxK343uOoi11h9Qj2Jtliz?=\n\t=?utf-8?q?gGmZdJIPVlc/KbV3r2KvYqbdM7vEIiGyIifR7W24wksDrvWuET6DH1Qx?=\n\t=?utf-8?q?dhzLAX6PMzJ9q5R5WBHHqQEWqxfBaA6X+fsiiA9TaOQqeJXt4KusApqr?=\n\t=?utf-8?q?M4OU7Ef34+8d46Tu01xcDRZF8p1eQkQhHIzefXKIoq8qbCmObuOMHKvu?=\n\t=?utf-8?q?HQ62dKaIH2TbklePdm+ZhBAkSj47yqWZ+74I5i0r8Mej0BJHK2nxtyzY?=\n\t=?utf-8?q?F/rJc/8D4ZoHe3ix5+9FupkXDxCMsuz+1BvDJ9O9GLTc50ZhT+niaO7n?=\n\t=?utf-8?q?DisSB/HCCzP6aYJeQAsPJbaxDdr20CXTwHuWKOkNTNuZ5JhsRe0ufxm/?=\n\t=?utf-8?q?SP8pMg3lMbkNaMmv9jTUa6TOeDMinU8VXynuJHC2v7EdVm7mMsV0JPZi?=\n\t=?utf-8?q?xy8yyCkh2R4Uey0306wTa+XEl0Iva9kXkccSBNkyZNmaeK9TA+TFc3NW?=\n\t=?utf-8?q?dRoYQ5qlhUYJ8K+oOgaoT+Gy1xTA5SbGmm/0aOpzEFkWvrZJvQytP9L1?=\n\t=?utf-8?q?sJQxCE+HkDyzpDUKjTkA2f04g+g5uZwn+V/9LmA/0lNWN1KioJ7IZRZO?=\n\t=?utf-8?q?qEOK04PikDno8sbo8hnlqKhg6SNRP/b3Uf1GoQWyfsY6Oh3kzq2fJBVG?=\n\t=?utf-8?q?uwyJ5fMPD2q1kXI1vdxA/T55MvyWA0udPPeyOtfPitcNMTSzzHPuEtFA?=\n\t=?utf-8?q?NXCrjwgLWTkX+SLsNDM3YSHBHZ/VUuI9U/b8KUfi/yQIZL+L3udVAptN?=\n\t=?utf-8?q?VZvyw9498oYzOsREUoIxb7iNX49e7aFblSYX1jngzZRXz6bp35p4L/Q3?=\n\t=?utf-8?q?vKqery1ueHhqeU6Pm6eUT2bexnoNrv7xzn/MwxyFs+tUElVHqC2mkzla?=\n\t=?utf-8?q?7Atd2Yr15KH2CbuJRPtB0cSZPekfEV5djwPMisgdp0r3fAVWQcYhXujy?=\n\t=?utf-8?q?0HZBQx3HUzYneKse26JZaOtkPuX0YUrl2GdRXwEIalFk8jjI5OuIRTk6?=\n\t=?utf-8?q?4jvoMuGvbJN2ew+nU+4hyjEBljWmEXfABkqcPtthr/LiV8mGAWvDxfIo?=\n\t=?utf-8?q?sXBmZJmC7IRyiN03h7TrJQdusSfX8YenEmYERDw8l9QiMsvooy9nwN2n?=\n\t=?utf-8?q?sDsbIPihv67HIVAaipv9SjpU12/HUMN7E0ojM6/fj+hojo8KaULpZXsH?=\n\t=?utf-8?q?UVOSkotaekKTHS20N2pZmwAqTa3Y+vaUt/a0lATcDrK3RWh/th/HP96k?=\n\t=?utf-8?q?EKM1W8x2/jjSSNcEc87OY8DntOXDFmD3Khuf/qXQgorjfk9u5Q=3D=3D?=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:GVXPR04MB9831.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230040)(366016)(376014)(1800799024)(8096899003); DIR:OUT;\n\tSFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?DeXZPpdVfhcYVpFuY3fAl1+pd?=\n\t=?utf-8?q?7g+kgZSsvCz5rYr8MptMojZT+3fnpI8FCAzT+nLNrj1+c1Dji28D0dcp?=\n\t=?utf-8?q?1qOFkrrh0ygoYNb9s4+TFoc9X36vL3Y+1MnZyxC9kE5dFTKX+Yu/VXVm?=\n\t=?utf-8?q?moa/9mMGYmV1lq0Ym5tE9y3xKbkkUFxMc8zuVahBT2e3F15l7RcNp/D8?=\n\t=?utf-8?q?P1l5z6M3PxuPBS5DTcAiNGxVgMxi8tz8QiKeSvUuZgqnmfJHTdHfhVMN?=\n\t=?utf-8?q?PIfaIgFISZ9ybQFuvdK9yz46qXmlXMrp8LKG6GL4z8wLG9p4i5uPwhkL?=\n\t=?utf-8?q?WZ7qpaOml/Xooe50fU28IZYGMBqrD2lt0v3VZpT1WaSi6qt3WZdvvOXc?=\n\t=?utf-8?q?MBNIdoEzsxIVcgaifAW/cx3P6g0WMf6JdfyW6FtrmRpTu45Y7fQZofoT?=\n\t=?utf-8?q?ZR1EKu02MtJxipx5d7shk+iacr5INwFsdq1ohFHV3A/P/wVi8G2UQsQ+?=\n\t=?utf-8?q?rvBG/WT5OMRWP0xeK0nt2+a/rbEd391jlynXvS9inW5c2BPv4sMnqjFO?=\n\t=?utf-8?q?y0MHo+Oh0/VzXEOFBHI5tFxz+AeeyfYnj+fAnF8OxR9qoTM9jWdhOoRB?=\n\t=?utf-8?q?QnF0hMKoftyw9YfJugp+KULpvQ/CJVKDrcNlU9VZ9/OLlOQDLFSFTJIv?=\n\t=?utf-8?q?JaW4Nhbq5g0SqE3WnKpuazMaVfWJy8J+iw7iOUkjK9ZtTtGkWoPOGfhb?=\n\t=?utf-8?q?6GjBJjkWdNTjn35bWwXZHs+AKivn2l/RDf78+Ko4EOKuweNzfW4FkVsU?=\n\t=?utf-8?q?6nAo4SqM5haUuo23ZTYPxQYuonD1PvSo3p7bp/Md9IIfbG/34hWQ8AX/?=\n\t=?utf-8?q?SnllRuUQ+g+jAK0haBXVIrdAU5SVxvdyjPKXY09k9SeTL+BVmIzRkP5O?=\n\t=?utf-8?q?8d0MsSe3fTZYERQvl3J6k4CpVcEDXjDiyl/mxEzW6CWfuXK5EHwrXNEa?=\n\t=?utf-8?q?ckuSvQexZN56xv3XcKaAnoP3eYlb/PRVrzovKx35m1HC0LWqBrK7n2vg?=\n\t=?utf-8?q?uNfAyZCfRwLt9XDsBxDZv02jlt0dxhRuye38difXViwoo59M7uRElunF?=\n\t=?utf-8?q?ZIK6W4XgWXKcHhCyztGORX/6sPufwisLCMDgtQWf/eQnZjiZtKixtdG6?=\n\t=?utf-8?q?Y08hfx+nap34p+LvuMVaPyAtpe0h2ruWF7DIzDpo+M3FsGhXY4owo5Ed?=\n\t=?utf-8?q?Kt2y/XdjO/taLYVGQm4Iz2iQQzNm0M7PvgVTJ8iUFQyoYJ7QrwZeleWj?=\n\t=?utf-8?q?3kZM2Cw009b9XZNyC9TntMHFj677j7QftSJ4rgFza67Q7QEUpQhmW8oW?=\n\t=?utf-8?q?SswjVWp8GpnSXMIwL8h5cM0RN6zlNVQJpL5ZqTh3EBZIYjgwRhdhdton?=\n\t=?utf-8?q?PNFC9goLyoxtIk0IZNH+TYUST5QoiTcjuzWNLlp+HrNLGCWrCsMKl8y8?=\n\t=?utf-8?q?CXEV2W2NHbKNIWoRwCCgqmwJYTWUow1Fpf2NyZfczZzTTqUHi406UH74?=\n\t=?utf-8?q?MJcP0e8vxfCuiPw4px1BF1nrr5hdrB0SiBsqxU24qyNejHBts4T7lPMm?=\n\t=?utf-8?q?XZkYyZG8eCpZ11r0mhv4ZoDK/q9mnOkYjisG4HDZXGixNTG44PoMsNOd?=\n\t=?utf-8?q?DllH5zTqtTiDujxOYAEl8RYwHiIOJIfoN+K0JvH+5MtzxQZVq98ccQfW?=\n\t=?utf-8?q?zOFi5SvIfzF15//jo+RZD5xxTlxag=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"b161bd1f-328a-4256-a677-08dd1ebbedbd","X-MS-Exchange-CrossTenant-AuthSource":"GVXPR04MB9831.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"17 Dec 2024 16:57:45.5239\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"cKZOCtgjILtb57DfUgefZBX1Z+fseHPK0/ZnkA4IvuwJTotPMnvNijCQnJuuP7lXQQ2e7EHO11+jHqi32RtRDQ==","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"DBAPR04MB7429","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34683,"web_url":"https://patchwork.libcamera.org/comment/34683/","msgid":"<175101302158.3281735.15509943566216575950@ping.linuxembedded.co.uk>","date":"2025-06-27T08:30:21","subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Antoine Bouyer (2024-12-17 16:59:06)\n> On 17/12/2024 17:46, Laurent Pinchart wrote:\n> \n> > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> >\n> >\n> > On Tue, Dec 17, 2024 at 04:23:27PM +0000, Kieran Bingham wrote:\n> >> Quoting Antoine Bouyer (2024-12-17 14:02:39)\n> >>> During configuration stage, kms_sink doesn't check optional colorSpace\n> >>> parameter is valid while dereferencing it. Then it leads to a crash\n> >>> with below signature if parameter is not defined:\n> >>>\n> >>>    /opt/fsl-imx-internal-xwayland/6.12-styhead/sysroots/armv8a-poky-linux/usr/include/c++/14.2.0/optional:48\n> >>>    2: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = libcamera::ColorSp\n> >>>    ace; _Dp = std::_Optional_base<libcamera::ColorSpace, true, true>]: Assertion 'this->_M_is_engaged()' fai\n> >>>    led.\n> >>>    Aborted (core dumped)\n> >>>\n> >>> This behavior is confirmed in the \"operator->\" page:\n> >>>   (https://en.cppreference.com/w/cpp/utility/optional/operator*)\n> >>>\n> >>> \"This operator does not check whether the optional contains a value!\"\n> >>>\n> >>> This crash is observed with styhead's gcc compiler (version 14.2.0),\n> >>> while it was not with previous scarthgap's gcc compiler (version 13.2.0).\n> >>>\n> >>> Use has_value() as a fix to make sure this property exists and prevent\n> >>> crash.\n> >>>\n> >>> Fixes: 6d7ddace5278 (\"cam: Add KMS sink class\")\n> >>> Signed-off-by: Antoine Bouyer<antoine.bouyer@nxp.com>\n> >>> Reviewed-by: Kieran Bingham<kieran.bingham@ideasonboard.com>\n> >> Thanks for the updates that I suggested, however I'm weary that perhaps\n> >> Laurent was suggesting this /shouldn't/ be possible to happen and should\n> >> be fixed in the pipeline handler? (and perhaps enforced?, and maybe\n> >> guaranteed to be set to RAW if unknown?)\n> >>\n> >> We could perhaps have a common path in the configuration that sets RAW\n> >> when the pipeline handler didn't set it ? Or maybe we enforce it must be\n> >> set by lc-compliance or such...\n> > Let's see first, Antoine, which pipeline handler have you encountered\n> > this issue with ?\n> \n> This is with imx8-isi pipeline.\n> Actually I was thinking about such approach initially, then changed to has_value() fix\n> because of \"optional\" flag.\n\nMoving my questions from v1 to here: (I hope I haven't now missed a\nv3..)\n\nAntoine, do you plan to provide a fix for the ISI pipeline handler to\nalways provide a colorSpace ? I don't see such a change currently in\ntree ?\n\n--\nKieran\n\n> \n> >\n> >>> ---\n> >>>   src/apps/cam/kms_sink.cpp | 3 ++-\n> >>>   1 file changed, 2 insertions(+), 1 deletion(-)\n> >>>\n> >>> diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp\n> >>> index 672c985..8f3b867 100644\n> >>> --- a/src/apps/cam/kms_sink.cpp\n> >>> +++ b/src/apps/cam/kms_sink.cpp\n> >>> @@ -153,7 +153,8 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n> >>>          colorEncoding_ = std::nullopt;\n> >>>          colorRange_ = std::nullopt;\n> >>>\n> >>> -       if (cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n> >>> +       if (!cfg.colorSpace.has_value() ||\n> >>> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n> >>>                  return 0;\n> >>>\n> >>>          /*\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> \n> Best regards\n> \n> Antoine","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 5A380C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Jun 2025 08:30:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C04E768DFE;\n\tFri, 27 Jun 2025 10:30:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 168F762C43\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Jun 2025 10:30:25 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 45CD0FDB;\n\tFri, 27 Jun 2025 10:30:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pIlqvBSg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751013005;\n\tbh=OUzLdhYFcCFYVp/IJmm4MJINnSl4TXGGHkRp64xrzdI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pIlqvBSgDka0jzyLxzfcoqZUVyfNabFIB8s7ALg5aLpZ35PDAUlSpD0k+hLFXuzMN\n\tXT0S/aeGVl4f/r8st0VMW41RRvjoVE8lTY7cOo9j9eIzC/+TXzCLEwWXLsjgWkt4KX\n\te8QHy59l6I+mPD1nm9ZSfwNlLs/6h0j7dH4N3168=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<bde515ca-ec84-4630-b497-693828d29388@nxp.com>","References":"<20241217140238.215572-1-antoine.bouyer@nxp.com>\n\t<20241217140238.215572-2-antoine.bouyer@nxp.com>\n\t<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>\n\t<20241217164650.GH20432@pendragon.ideasonboard.com>\n\t<bde515ca-ec84-4630-b497-693828d29388@nxp.com>","Subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, julien.vuillaumier@nxp.com","To":"Antoine Bouyer <antoine.bouyer@nxp.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Fri, 27 Jun 2025 09:30:21 +0100","Message-ID":"<175101302158.3281735.15509943566216575950@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34690,"web_url":"https://patchwork.libcamera.org/comment/34690/","msgid":"<813db32b-2580-442f-b11f-ae0bc2bdec10@nxp.com>","date":"2025-06-27T09:50:01","subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","submitter":{"id":218,"url":"https://patchwork.libcamera.org/api/people/218/","name":"Antoine Bouyer","email":"antoine.bouyer@nxp.com"},"content":"On 27/06/2025 10:30, Kieran Bingham wrote:\n> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> \n> \n> Quoting Antoine Bouyer (2024-12-17 16:59:06)\n>> On 17/12/2024 17:46, Laurent Pinchart wrote:\n>>\n>>> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n>>>\n>>>\n>>> On Tue, Dec 17, 2024 at 04:23:27PM +0000, Kieran Bingham wrote:\n>>>> Quoting Antoine Bouyer (2024-12-17 14:02:39)\n>>>>> During configuration stage, kms_sink doesn't check optional colorSpace\n>>>>> parameter is valid while dereferencing it. Then it leads to a crash\n>>>>> with below signature if parameter is not defined:\n>>>>>\n>>>>>     /opt/fsl-imx-internal-xwayland/6.12-styhead/sysroots/armv8a-poky-linux/usr/include/c++/14.2.0/optional:48\n>>>>>     2: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = libcamera::ColorSp\n>>>>>     ace; _Dp = std::_Optional_base<libcamera::ColorSpace, true, true>]: Assertion 'this->_M_is_engaged()' fai\n>>>>>     led.\n>>>>>     Aborted (core dumped)\n>>>>>\n>>>>> This behavior is confirmed in the \"operator->\" page:\n>>>>>    (https://en.cppreference.com/w/cpp/utility/optional/operator*)\n>>>>>\n>>>>> \"This operator does not check whether the optional contains a value!\"\n>>>>>\n>>>>> This crash is observed with styhead's gcc compiler (version 14.2.0),\n>>>>> while it was not with previous scarthgap's gcc compiler (version 13.2.0).\n>>>>>\n>>>>> Use has_value() as a fix to make sure this property exists and prevent\n>>>>> crash.\n>>>>>\n>>>>> Fixes: 6d7ddace5278 (\"cam: Add KMS sink class\")\n>>>>> Signed-off-by: Antoine Bouyer<antoine.bouyer@nxp.com>\n>>>>> Reviewed-by: Kieran Bingham<kieran.bingham@ideasonboard.com>\n>>>> Thanks for the updates that I suggested, however I'm weary that perhaps\n>>>> Laurent was suggesting this /shouldn't/ be possible to happen and should\n>>>> be fixed in the pipeline handler? (and perhaps enforced?, and maybe\n>>>> guaranteed to be set to RAW if unknown?)\n>>>>\n>>>> We could perhaps have a common path in the configuration that sets RAW\n>>>> when the pipeline handler didn't set it ? Or maybe we enforce it must be\n>>>> set by lc-compliance or such...\n>>> Let's see first, Antoine, which pipeline handler have you encountered\n>>> this issue with ?\n>>\n>> This is with imx8-isi pipeline.\n>> Actually I was thinking about such approach initially, then changed to has_value() fix\n>> because of \"optional\" flag.\n> \n> Moving my questions from v1 to here: (I hope I haven't now missed a\n> v3..)\n> \n> Antoine, do you plan to provide a fix for the ISI pipeline handler to\n> always provide a colorSpace ? I don't see such a change currently in\n> tree ?\n> \n> --\n> Kieran\n\nHi Kieran,\nGood to see old patchset are still alive ;) I did not push any V3 (yet).\n\nUnfortunately no, I did not plan to force colorSpace in imx8 pipeline \nhandler. I assume that if the parameter must be present, it becomes \n\"mandatory\" and not optional anymore. So optional property should be \nremoved as well I guess.\n\nI don't know what would be the impact on other pipelines then ... so I \ndid not explore that option on my side.\n\nBest regards\nAntoine\n\n> \n>>\n>>>\n>>>>> ---\n>>>>>    src/apps/cam/kms_sink.cpp | 3 ++-\n>>>>>    1 file changed, 2 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp\n>>>>> index 672c985..8f3b867 100644\n>>>>> --- a/src/apps/cam/kms_sink.cpp\n>>>>> +++ b/src/apps/cam/kms_sink.cpp\n>>>>> @@ -153,7 +153,8 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n>>>>>           colorEncoding_ = std::nullopt;\n>>>>>           colorRange_ = std::nullopt;\n>>>>>\n>>>>> -       if (cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>>>>> +       if (!cfg.colorSpace.has_value() ||\n>>>>> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>>>>>                   return 0;\n>>>>>\n>>>>>           /*\n>>> --\n>>> Regards,\n>>>\n>>> Laurent Pinchart\n>>\n>> Best regards\n>>\n>> Antoine","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 14C67BDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Jun 2025 09:48:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BCA3F68E00;\n\tFri, 27 Jun 2025 11:48:40 +0200 (CEST)","from DB3PR0202CU003.outbound.protection.outlook.com\n\t(mail-northeuropeazlp170100001.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:c200::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E84A568DF4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Jun 2025 11:48:38 +0200 (CEST)","from GVXPR04MB9831.eurprd04.prod.outlook.com (2603:10a6:150:11c::8)\n\tby VI1PR04MB9860.eurprd04.prod.outlook.com (2603:10a6:800:1d0::15)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8880.21;\n\tFri, 27 Jun 2025 09:48:35 +0000","from GVXPR04MB9831.eurprd04.prod.outlook.com\n\t([fe80::4634:3d9c:c4a:641a]) by\n\tGVXPR04MB9831.eurprd04.prod.outlook.com\n\t([fe80::4634:3d9c:c4a:641a%5]) with mapi id 15.20.8857.026;\n\tFri, 27 Jun 2025 09:48:35 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"Upux4gKK\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=PR8yTCZjmRInp4PEwb3dDOgcm8cjH38KHG7oxs2Kk6J4CrOLbpw4ZlikiydVmkkxUkHgeqRWtw1EkWRJCmHUd3sVNXZ8j6m0rGRgGeLYVNo9UaeNd8jnwgjsR9ElJPCPsI/5FnJGflzYPu91rK/K3E6g2yYKnp5e2pHRvmc9FVJiAQ84DQ7JBhUK+ykNV3GDe+AqcRHgYs3JLlUkCMv85s1wGGEomUKmbG/PdM71VgXGV7zec5LwaGzkIj2pZ2ChB2keRvEQ7/33sN+mxkFaJKwx/8MZ28aRclUZKvyo0jaAfEXgEI2yUUYe9mvHbXE5PUplwJhalYXS8rQZF5YTWQ==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=A2rgaPMhGcTo0KUF/TuNV2QmbY6J4dpPj73e5dXerhw=;\n\tb=iREtUXWksKBlo0iODInGkXlth9o1nKGKB7PceL5pCoiKZFYNZAYltfN6heNHQAtbNZXn29b9kX6Ua26R31cn/lfNOWQaskP8F2zn+uUb39Ag1ghIO36Pty/TWF90JCfFp6iFXVZ+alDxr5HXp4BIgsMFBDLiHb6CNI2W7TA/yBWpZjsplV97pZp5YwaAzxWf53MTmrRLH7pa5+3yuP8q5jrR8vRiHKi6/vSHPUqaDa0/1A0uqQiRpTJIJ9MVBVrxG661Ncmes1lKKfmzrrpjOVCxcIP4Ux6qeG/FcB+PT9Au+dDpf5kevySVP/JkgOl/AD7bGvcXaVAE9LmkyRz4sw==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=A2rgaPMhGcTo0KUF/TuNV2QmbY6J4dpPj73e5dXerhw=;\n\tb=Upux4gKKwIpcZDm336g5VWM0fGrqHK5cEZJUy2tTePnCRqlZ81jtupEFTe6ztCJ5VeT3j5GL+ne6NUtusy6+cNXguGQbhIbdJOJ9px+j88Ux49rlQQrD75iCsgvAiyywaLXIGo8gXN19+cjNDcRugMOowe2DcHOA9jkeCURaXVi4TZD2zn+Wlx2KpeEe+rTxwlPPnmKGLkEIF/+bulKEyfuhpl9wyGkGewZAi7C7QQcpu3ing4MlU6sVwMcwduytiimrQUmcglIfMFMnpVabMKrAL9cF51Iv28xi450mUnBZi8nqO1Bt7nmFIt1+j4jhH2O7GdtFMbE5uEJYay+UrQ==","Message-ID":"<813db32b-2580-442f-b11f-ae0bc2bdec10@nxp.com>","Date":"Fri, 27 Jun 2025 11:50:01 +0200","User-Agent":"Mozilla Thunderbird","Subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, julien.vuillaumier@nxp.com","References":"<20241217140238.215572-1-antoine.bouyer@nxp.com>\n\t<20241217140238.215572-2-antoine.bouyer@nxp.com>\n\t<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>\n\t<20241217164650.GH20432@pendragon.ideasonboard.com>\n\t<bde515ca-ec84-4630-b497-693828d29388@nxp.com>\n\t<175101302158.3281735.15509943566216575950@ping.linuxembedded.co.uk>","Content-Language":"en-US","From":"Antoine Bouyer <antoine.bouyer@nxp.com>","In-Reply-To":"<175101302158.3281735.15509943566216575950@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","X-ClientProxiedBy":"FR2P281CA0085.DEUP281.PROD.OUTLOOK.COM\n\t(2603:10a6:d10:9b::12) To GVXPR04MB9831.eurprd04.prod.outlook.com\n\t(2603:10a6:150:11c::8)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"GVXPR04MB9831:EE_|VI1PR04MB9860:EE_","X-MS-Office365-Filtering-Correlation-Id":"7e10b8a8-a54f-47e4-8185-08ddb55fc8cc","X-LD-Processed":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635,ExtAddr","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;ARA:13230040|1800799024|376014|366016;","X-Microsoft-Antispam-Message-Info":"=?utf-8?q?Xg8/qW/ZLRrlnOVG/F1oMZAZqhRK?=\n\t=?utf-8?q?1s95YuDlFmhf45PYcVP3PSuJ818kaHgkeqBWstVlfmEk6OjufkWfy22E?=\n\t=?utf-8?q?V0hKVyAQg3yvqDiKXRJWjNgIZt7LbfTyEKMmUygXlSTm/QQO8T2YIx/w?=\n\t=?utf-8?q?iOMpDisNIgjrrMdAMX0WXWpzyk1ihkxeH2msgufsqzBahUyc6tKDoRZm?=\n\t=?utf-8?q?l0J9sNUrQW6gELtIfn8DM1LSTM0PtailXHNmsObOshCWCdIMqX7IQTo5?=\n\t=?utf-8?q?l1/od4DijXY6KnqX45se3B/F2i1cNNVddKDWdTLK0/iODuFHbXcyFGMO?=\n\t=?utf-8?q?PtA8hfru/aian4JvYCbIVNOVS8rPpcjvEbNF7Lgiq/ps7X4sB8Ji4xGr?=\n\t=?utf-8?q?olsOTdc5tH5SQ3UgcYmqVF08LiA0YH/lwkFGIY+qC4KtyDU9VteZ59Pq?=\n\t=?utf-8?q?1/K6lXeoEjmw4e01wHKxh6AC+VrTjA5MhQveBIv4966s1eYMXp7KJNtg?=\n\t=?utf-8?q?Rp5fKeAQaeX+iR62cS5Cc6fImoMgRCBHMolOCK38H2h/ssnWjywsBZBH?=\n\t=?utf-8?q?+UbQDnySWM2sBSs3tN94AORXmPfADnmPo5NrmZi26lG2PtAx5isvHCY2?=\n\t=?utf-8?q?7Rfe1ghj0drw+odHPTSiEoAO4gmJDFKfllZsbV0fulonHs6fDDLaa3AB?=\n\t=?utf-8?q?87nxr1iQ36yORPvfMVEtxehNLjzJxUE2/PtZyX2ZLLHXxDFcevghscuI?=\n\t=?utf-8?q?xlciKpax9AztjRmM+5Yz2LHyh6tuf/wQPZasHMU3bY71ThOQ9zeeoz5B?=\n\t=?utf-8?q?LzU5WGWvRMcDeGLx9bSSHHK5i6F2/xkI3einZOy9LO9lDUSHrwy67ZMi?=\n\t=?utf-8?q?GQwe74LkadKX5iBG76FCDFO0F2aLd5yoZtPQIZlPJfsrLOAp8StZ0onP?=\n\t=?utf-8?q?twanGp6UPiQ7izZkVf150uVtS1Ew6w5hibT8WtWkipvBYej0uQNizQ6h?=\n\t=?utf-8?q?QmBxrZadiVjXNSfXVdhi22r7yW14/qledGb3VO8lLpqBuxBzyLWrpJRk?=\n\t=?utf-8?q?27+ww1bbdM3OnSlCcNl7vlzfePgczEGGWdQnQhHkBVuo4/G/XGhmNsCk?=\n\t=?utf-8?q?HsgfMB05rc/yJM8xq/DlsBEiMEj1GHMgJZvZZCStK3gE8jGDmLekJ9M8?=\n\t=?utf-8?q?JcjW0Ig0OZ/3aovfJya/9Kn7AeX2d+tjjLf7hSJyMJeMjDnC5cp06jMA?=\n\t=?utf-8?q?z5V8MlrOmK6b/4+ovoYpQgjsP8s2UF23lIcBm5OVud3YLjg75FHjWeZP?=\n\t=?utf-8?q?BfO1K9qH9VoacejgfjZcLnF5IGCb9M6Nmca9uZn94/qXRfeqCxXewLHh?=\n\t=?utf-8?q?+sfiv1NZM4HNq0zZINyIt7pvUHvO0D0TkJvKifr0/CZMGSHMyxvr+JRT?=\n\t=?utf-8?q?svYJEKFpD8g/E2hFDR+OtWOcGFT1hP214yXobw2iaJKh3J403rDGFDA4?=\n\t=?utf-8?q?unlr6xTNAcNT+ws=3D?=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:GVXPR04MB9831.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230040)(1800799024)(376014)(366016); DIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?9brDWMRGuY6plJ2Pj0U6szI1y?=\n\t=?utf-8?q?nmrPDSKd4JhWEIdkU5jn0zOvbG9zhzFlX9SoDvgF2Tl0ZKGYaMQfoDBt?=\n\t=?utf-8?q?agfRAANFTx7GSJFOGdgp2PqqVDOUyKHc3U3agf3aZrX59OlK2CTgFN57?=\n\t=?utf-8?q?3URnfdxgwx9hXvCWP+Ok/nxY7yoXA+anUO+9Tf6m8LuUwTQUeeH8RA6O?=\n\t=?utf-8?q?fVn9uy/FOrb2LniMnZc01bZF/otFOfRNVYRls6cqeAvjNcLBqlZjlBrV?=\n\t=?utf-8?q?j87UYkfkS7oZ+PlyzG4vMwwQCR0wfyiHHZwtvDL4/AIilHHMxC0rvK1s?=\n\t=?utf-8?q?EVEgm/tTu0iO6QFv4Xytcn16ZelcfXsb2MiL1zKM1geGty240TXXW3L9?=\n\t=?utf-8?q?s9NWC7H9NXG7cjmeBhpmqo9WIiznQ7IkMvegoE31ZmuN2hPLH/MPJRUc?=\n\t=?utf-8?q?ZERFeTvAciT/wGKi5l5qsquIQEtQqc0cJed7W47jcUWxXKF2riaOe1dB?=\n\t=?utf-8?q?7nqgJFAjZ2N9RWI6savsiMBlP+4b+a+3RQTytvfLK6YsYlBdr6tU9JbQ?=\n\t=?utf-8?q?4FSJvm3/0fDylRNvzv5aybcxChzdhqCN3Xv5ggn12O/x8vFzrObMK+na?=\n\t=?utf-8?q?cVWrJo8PJ/Np/4xbb262D8dOfg6rgoE3ak+xS9CA802cdVuCA7WCEsqO?=\n\t=?utf-8?q?3+rDbvrtu6KFvrCPMKLGyZeCiAvwugsfFjidBZbLijtUhsxLj/3VmQPe?=\n\t=?utf-8?q?qJUK8v1JIn+bDzgyKpewDnEnumqbldecciVa79UEFD1j5M71BXO6TCHm?=\n\t=?utf-8?q?RYkTMi8FSiYkO0PgFtn4l0DOcI2izv3IvafAZjDMloCxNInzZHtkPzDC?=\n\t=?utf-8?q?rsBEWF97dd8AH61p0YwuS4gGvX1npalrMhsQ4XpEQxwR4qEEI8hxHR9a?=\n\t=?utf-8?q?hw/tr0dUSsTZa+Pbh5BgMa+/8eJ1KBPkPEPpeOTMv3tbIE7Z9HGuj3Ch?=\n\t=?utf-8?q?sK07JxeZtyjM9pkQGcUxFU+Q6M0anL1EoVbTxodH2se0nnnDkV28wrgk?=\n\t=?utf-8?q?IpD3ZtDxJJpVaI/A5sfta1ABidVHF3WB3LRKe5DcOnny+mYEyEnrIoCO?=\n\t=?utf-8?q?fq/XMHr19/VYYZJu+9tiDlAfq9Hvcb+CzNsPbVwfJCaocOFBU3Np4pSX?=\n\t=?utf-8?q?uoGsKsix/zTKyirlpqzwLIPbcpz10kic0PVlfOcEm12Ier6sr4jDJBsC?=\n\t=?utf-8?q?j1HFYolpDv/2Rrqz/cHBzeNEbyxPkToK4l/RHuCN2WvsJnQCr1Sx64+7?=\n\t=?utf-8?q?nzRA2F28opTCDDNcXl9ACWwqMBpRPdP9t2tL5EHNWshW8HQHBW2eXV1t?=\n\t=?utf-8?q?Fbm5neN30SJpx8MOa27Lxvr2WkN5QVKnC+NlzCrjYkW/DvRptmldEXfG?=\n\t=?utf-8?q?pgRCmDqlG6FhxOohrnDpCNt9V5iQNhSVbK/7jCTlJqI4cTXx4rTOVKn6?=\n\t=?utf-8?q?etZsmPmYomYqaI9X6Of+YfcqJZQyMPiVAPh1ccf35GGyLnZDFeiJxhOq?=\n\t=?utf-8?q?umyb2Uox6v3Rygaeii5Ayhu6xZzvo2aQfSxQ5fhWD7+tLWa3S3IoUfS1?=\n\t=?utf-8?q?kNnjtCJXpS5nx69rrhlVgtfGpEg/tJAXzSPUI42/MAMMBUyplwVAnPOV?=\n\t=?utf-8?q?p9xh0ouHQ61GAJ/I8ypxyLgBNlEGAAna4K/eCVabHlZKhdqkjucKhuUg?=\n\t=?utf-8?q?jyV5+euJyFN6HdNXsDurdhigd+NIA=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"7e10b8a8-a54f-47e4-8185-08ddb55fc8cc","X-MS-Exchange-CrossTenant-AuthSource":"GVXPR04MB9831.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"27 Jun 2025 09:48:35.3806\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"LwnFCjw0HrN7N9qHLkuOeCknuQLH1kIpd8TOCEd0FqYhz1g7/NRPxS3ZOcX0sPlkGjqa+0EXISgKFx8X3gb0bA==","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"VI1PR04MB9860","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34691,"web_url":"https://patchwork.libcamera.org/comment/34691/","msgid":"<175101813625.3281735.14206832700941505619@ping.linuxembedded.co.uk>","date":"2025-06-27T09:55:36","subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Antoine Bouyer (2025-06-27 10:50:01)\n> \n> \n> On 27/06/2025 10:30, Kieran Bingham wrote:\n> > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> > \n> > \n> > Quoting Antoine Bouyer (2024-12-17 16:59:06)\n> >> On 17/12/2024 17:46, Laurent Pinchart wrote:\n> >>\n> >>> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> >>>\n> >>>\n> >>> On Tue, Dec 17, 2024 at 04:23:27PM +0000, Kieran Bingham wrote:\n> >>>> Quoting Antoine Bouyer (2024-12-17 14:02:39)\n> >>>>> During configuration stage, kms_sink doesn't check optional colorSpace\n> >>>>> parameter is valid while dereferencing it. Then it leads to a crash\n> >>>>> with below signature if parameter is not defined:\n> >>>>>\n> >>>>>     /opt/fsl-imx-internal-xwayland/6.12-styhead/sysroots/armv8a-poky-linux/usr/include/c++/14.2.0/optional:48\n> >>>>>     2: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = libcamera::ColorSp\n> >>>>>     ace; _Dp = std::_Optional_base<libcamera::ColorSpace, true, true>]: Assertion 'this->_M_is_engaged()' fai\n> >>>>>     led.\n> >>>>>     Aborted (core dumped)\n> >>>>>\n> >>>>> This behavior is confirmed in the \"operator->\" page:\n> >>>>>    (https://en.cppreference.com/w/cpp/utility/optional/operator*)\n> >>>>>\n> >>>>> \"This operator does not check whether the optional contains a value!\"\n> >>>>>\n> >>>>> This crash is observed with styhead's gcc compiler (version 14.2.0),\n> >>>>> while it was not with previous scarthgap's gcc compiler (version 13.2.0).\n> >>>>>\n> >>>>> Use has_value() as a fix to make sure this property exists and prevent\n> >>>>> crash.\n> >>>>>\n> >>>>> Fixes: 6d7ddace5278 (\"cam: Add KMS sink class\")\n> >>>>> Signed-off-by: Antoine Bouyer<antoine.bouyer@nxp.com>\n> >>>>> Reviewed-by: Kieran Bingham<kieran.bingham@ideasonboard.com>\n> >>>> Thanks for the updates that I suggested, however I'm weary that perhaps\n> >>>> Laurent was suggesting this /shouldn't/ be possible to happen and should\n> >>>> be fixed in the pipeline handler? (and perhaps enforced?, and maybe\n> >>>> guaranteed to be set to RAW if unknown?)\n> >>>>\n> >>>> We could perhaps have a common path in the configuration that sets RAW\n> >>>> when the pipeline handler didn't set it ? Or maybe we enforce it must be\n> >>>> set by lc-compliance or such...\n> >>> Let's see first, Antoine, which pipeline handler have you encountered\n> >>> this issue with ?\n> >>\n> >> This is with imx8-isi pipeline.\n> >> Actually I was thinking about such approach initially, then changed to has_value() fix\n> >> because of \"optional\" flag.\n> > \n> > Moving my questions from v1 to here: (I hope I haven't now missed a\n> > v3..)\n> > \n> > Antoine, do you plan to provide a fix for the ISI pipeline handler to\n> > always provide a colorSpace ? I don't see such a change currently in\n> > tree ?\n> > \n> > --\n> > Kieran\n> \n> Hi Kieran,\n> Good to see old patchset are still alive ;) I did not push any V3 (yet).\n> \n> Unfortunately no, I did not plan to force colorSpace in imx8 pipeline \n> handler. I assume that if the parameter must be present, it becomes \n> \"mandatory\" and not optional anymore. So optional property should be \n> removed as well I guess.\n\nLaurent replied to that point in a previous mail I think...\n\nhttps://patchwork.libcamera.org/patch/22373/#32812\n\n>>> The member is optional primarily to allow applications to get the\n>>> default color space. We also allow pipeline handlers to leave it\n>>> unset when they don't know what color space is produced, but that\n>>> shouldn't be the rule. Actually, except for the simple pipeline\n>>> handler (and possibly for UVC, I haven't checked), I would make it\n>>> mandatory for pipeline handlers to return a valid color space. We\n>>> could also require them to report ColorSpace::Raw when they don't\n>>> know.\n\n> \n> I don't know what would be the impact on other pipelines then ... so I \n> did not explore that option on my side.\n> \n> Best regards\n> Antoine","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 54AF8C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Jun 2025 09:55:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 37ED268DFD;\n\tFri, 27 Jun 2025 11:55:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3838262C43\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Jun 2025 11:55:39 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98B2F73B;\n\tFri, 27 Jun 2025 11:55:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TAPh81cR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751018119;\n\tbh=B8XZ67iGHgK0+vfohYePeKn+KZ8jakJFgmD7ZgDV/r8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=TAPh81cRqohvw2u54rLk2UYmaZRHvNcMH/mrZ8DUNCZsiQCw7YBMQwJgpEOsbiWCw\n\tAV9m3sg5XvlR/Zz3G4z9wP9dR1OIoO64kRL9WMhiwz8Xb7c7VlGMZt3JwLs7OWNq1D\n\t3LgNENrwCFqYULzpWiLMOHjypHqkGa73bX1nX8Ec=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<813db32b-2580-442f-b11f-ae0bc2bdec10@nxp.com>","References":"<20241217140238.215572-1-antoine.bouyer@nxp.com>\n\t<20241217140238.215572-2-antoine.bouyer@nxp.com>\n\t<173445260772.32744.10649910978649508007@ping.linuxembedded.co.uk>\n\t<20241217164650.GH20432@pendragon.ideasonboard.com>\n\t<bde515ca-ec84-4630-b497-693828d29388@nxp.com>\n\t<175101302158.3281735.15509943566216575950@ping.linuxembedded.co.uk>\n\t<813db32b-2580-442f-b11f-ae0bc2bdec10@nxp.com>","Subject":"Re: [EXT] Re: [PATCH v2 1/1] apps: cam: kms_sink: Verify colorSpace\n\tpresence before dereference","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, julien.vuillaumier@nxp.com","To":"Antoine Bouyer <antoine.bouyer@nxp.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Fri, 27 Jun 2025 10:55:36 +0100","Message-ID":"<175101813625.3281735.14206832700941505619@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]