[{"id":33590,"web_url":"https://patchwork.libcamera.org/comment/33590/","msgid":"<174194299442.1289405.15170002880350501354@ping.linuxembedded.co.uk>","date":"2025-03-14T09:03:14","subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-03-10 11:06:30)\n> cfg.colorSpace may be unset in KMSSink::configure, resulting in a crash\n> when it is accessed.  If cfg.colorSpace is unset, simply return, the\n> same way as when YcbcrEncoding is set to None.\n\nI think this is something that we should ensure is trapped by\nlc-compliance in fact.\n\nI believe pipeline handlers /must/ always set the correct colorSpace\nafter validate - so it's incorrect for applications to ever hit an\nundefined color space ...\n\nOf course crashing isn't nice either ... but is this occuring in\nSoftISP/simple pipeline handler ?\n\n\n\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\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 672c985a..aa9459cf 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 ||\n> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>                 return 0;\n>  \n>         /*\n> -- \n> 2.48.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 62DD9C32F4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 14 Mar 2025 09:03:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7102768941;\n\tFri, 14 Mar 2025 10:03:19 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F63768779\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 Mar 2025 10:03:17 +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 DEDDB664;\n\tFri, 14 Mar 2025 10:01:37 +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=\"NnC+GU/4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1741942898;\n\tbh=fsovvZBvemTUUlaCY1nTf37cz07XM6csbl7zud9Q/fM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=NnC+GU/4HD8xpfRds31sW5BCHn51HuleytuHO+85vt3uZgDjlraP6nnM9rdAwxMhi\n\t0bF3SYn1zAOMhE/3Y4lUKpZzql+IHaDJPjPcJO5ZLfnVt+mAK1/Vddok3kffvQSqJH\n\tYIiUAEwjskuQyczzjmvUrpNSNwyDnwg27TkZCSB4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250310110630.34857-1-mzamazal@redhat.com>","References":"<20250310110630.34857-1-mzamazal@redhat.com>","Subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Milan Zamazal <mzamazal@redhat.com>","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","Date":"Fri, 14 Mar 2025 09:03:14 +0000","Message-ID":"<174194299442.1289405.15170002880350501354@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":33591,"web_url":"https://patchwork.libcamera.org/comment/33591/","msgid":"<85wmcr91ud.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-03-14T13:30:18","subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Kieran,\n\nKieran Bingham <kieran.bingham@ideasonboard.com> writes:\n\n> Quoting Milan Zamazal (2025-03-10 11:06:30)\n>> cfg.colorSpace may be unset in KMSSink::configure, resulting in a crash\n>> when it is accessed.  If cfg.colorSpace is unset, simply return, the\n>> same way as when YcbcrEncoding is set to None.\n>\n> I think this is something that we should ensure is trapped by\n> lc-compliance in fact.\n>\n> I believe pipeline handlers /must/ always set the correct colorSpace\n> after validate - so it's incorrect for applications to ever hit an\n> undefined color space ...\n\nI'm not sure whether all the pipelines do that; at least `simple'\ndoesn't.  I can fix `simple' pipeline but maybe some others have the\nproblem too.\n\n> Of course crashing isn't nice either ... \n\nLet's have an assertion there then to still expose the problem while\ncrashing in a civilized way?\n\n> but is this occuring in SoftISP/simple pipeline handler ?\n\nYes.  It started happening to me once I reinstalled my development\nsystem to Fedora 41.  I can't see any obvious reason why it crashes now\nand not before (maybe some change in gcc? -- using 14.2.1).\n\n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\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 672c985a..aa9459cf 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 ||\n>> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>>                 return 0;\n>>  \n>>         /*\n>> -- \n>> 2.48.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 C6C10C32F7\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 14 Mar 2025 13:30:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C609568941;\n\tFri, 14 Mar 2025 14:30:26 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11298687FA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 Mar 2025 14:30:24 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-515-aL6ZNlecMbOLM69DIq-UEA-1; Fri, 14 Mar 2025 09:30:22 -0400","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-43d00017e9dso12677855e9.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 Mar 2025 06:30:22 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-3978ef9a23bsm1176443f8f.15.2025.03.14.06.30.18\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 14 Mar 2025 06:30:19 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"F0D0hVgA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1741959023;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=SZ+6LBO4MQeIJ0kbu9rWkopBBNcvmJ/2CM3TDPYFqIc=;\n\tb=F0D0hVgA2oThIB4jp/Hn2822a8JteEY3vUdaKsQ7I5kCzPvSAGsbneyHyZ5xbC3GuOz43E\n\ttzBEvSGdFgZ+KTMx7thzO7CbYEQxzjEWAKTPnby84g8MEmvYolTAmUrez5RoaX9ucp5VwC\n\tFtrbjuFLqXDVoEolqFPzco7KhoWhnEk=","X-MC-Unique":"aL6ZNlecMbOLM69DIq-UEA-1","X-Mimecast-MFC-AGG-ID":"aL6ZNlecMbOLM69DIq-UEA_1741959021","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1741959021; x=1742563821;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=SZ+6LBO4MQeIJ0kbu9rWkopBBNcvmJ/2CM3TDPYFqIc=;\n\tb=XWTIjRYn+uDPtESEd9cUga9b4OHOZJLC9B2FthrEPduz85+mUxIblWdgIJANhV27h1\n\t4r0eNxLt68SNiUO4o3KqgtjLya/sy5vjwbn4E/viW6qNFLaJAUW80ZW0LCuFtomrM04/\n\t0rscFFt1on16xHLYjZgi6doYWQzY+V8HhNqY0Vk2T1q27i5SCY9F5Wx+yWv5XAs5GXqf\n\tI3HSy5jATzaBeZrOkRuJQ4dKiR37D763edQQBRFSu8Imxip6eLrdFkowvYZR55lsqKHt\n\tiGnBO/AMBgCw4cRcE/LdL1Ob0JbKwO/GJJiyp4RFQQfToWxtsG1kqSvn91+0RcBTetzF\n\tATDQ==","X-Gm-Message-State":"AOJu0Yw310GxcTc5pgS49uJpMCyH9Wy+eoQwKddiWjP+KmlM5X0Ssl9l\n\tLUkwbeaxPqdan628BmiI9tEk/wuiBU8UeKA0map/Lbj9CYPHLhoI2KSWTvjVVODknJgZOB9sk3U\n\tXIMe53srbSp1R51epnLa5/7Mf+xXXNmZDaKvwIizCqQk+gmnuVlvOcJDoiYJpkQW1pejIJ0IcQK\n\ttT7zpV/8vMc6IHSBW7i0/WHWfcyOPFV1VIfDaD8o0MxX3Thl2Zi3jRhfw=","X-Gm-Gg":"ASbGncsA/cZgxJb/4wyB3fo58PN2PMkb2y2sG6dZf8GNs0Yz1TS/Gc7EfyxKNCrJ6pA\n\tgksXx4cgvvugIi6zIHmluto08ecbB5jzucIDP8v+A300580BXwraqhVtDqJ45Ed60FDkzeRyRbE\n\tW3LOOyMYE9BJq2sj6ibh1Zpr3MOt4CT2Kl5MMUQ6doP9py+crYOkaC28WUR65lcqfn9JD9d7MhZ\n\tYCwInaR0f1rX4mcT0T/jyuFrNgmGMy51Q53ttB2cgtY9tvihrZMdlzv9EMZtT3URSqELcnBQ5oC\n\taxai3wTNi6In7CHI0DI6Oi7v6QAlkpinB5C/USetomKlYr0V7bA3CxxH5VKFDRAFccvH","X-Received":["by 2002:a05:600c:19cd:b0:43c:ea40:ae4a with SMTP id\n\t5b1f17b1804b1-43d1ecd932fmr33980605e9.31.1741959020857; \n\tFri, 14 Mar 2025 06:30:20 -0700 (PDT)","by 2002:a05:600c:19cd:b0:43c:ea40:ae4a with SMTP id\n\t5b1f17b1804b1-43d1ecd932fmr33979755e9.31.1741959020220; \n\tFri, 14 Mar 2025 06:30:20 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IF+cBrDwWz0XncPJ1duVShTdGtVw9iLpcsChiKStfdCcwokvdM5MXSiD4NoQptepMOVzr+G7Q==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","In-Reply-To":"<174194299442.1289405.15170002880350501354@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Fri, 14 Mar 2025 09:03:14 +0000\")","References":"<20250310110630.34857-1-mzamazal@redhat.com>\n\t<174194299442.1289405.15170002880350501354@ping.linuxembedded.co.uk>","Date":"Fri, 14 Mar 2025 14:30:18 +0100","Message-ID":"<85wmcr91ud.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"3rnS2BfpzH2dR4QJrgc4xdFbDalVMuEufTOixOJrlyM_1741959021","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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":33593,"web_url":"https://patchwork.libcamera.org/comment/33593/","msgid":"<174204480988.3467613.7490676062480537223@ping.linuxembedded.co.uk>","date":"2025-03-15T13:20:09","subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-03-14 13:30:18)\n> Hi Kieran,\n> \n> Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n> \n> > Quoting Milan Zamazal (2025-03-10 11:06:30)\n> >> cfg.colorSpace may be unset in KMSSink::configure, resulting in a crash\n> >> when it is accessed.  If cfg.colorSpace is unset, simply return, the\n> >> same way as when YcbcrEncoding is set to None.\n> >\n> > I think this is something that we should ensure is trapped by\n> > lc-compliance in fact.\n> >\n> > I believe pipeline handlers /must/ always set the correct colorSpace\n> > after validate - so it's incorrect for applications to ever hit an\n> > undefined color space ...\n> \n> I'm not sure whether all the pipelines do that; at least `simple'\n> doesn't.  I can fix `simple' pipeline but maybe some others have the\n> problem too.\n\nThat's why I think we need to do better on lc-complicance tests to\nensure 'rules' are always validated.\n\n> > Of course crashing isn't nice either ... \n> \n> Let's have an assertion there then to still expose the problem while\n> crashing in a civilized way?\n\nIn kmssink? I'm fine adding an assertion here - but what about the other\nsinks ? What about the other applciations. That's why 'this' should be\nvalidated for /all/ pipeline handlers.\n\nThere should be a contract somewhere that states a pipeline handler\n/must/ fill in specific fields.\n\nI have an upcoming proposal that could let us add a validation layer to\nall runtimes too which could be another place to add runtime\nvalidations.\n\nHopefully I'll get that out next week as an initial RFC.\n\n> > but is this occuring in SoftISP/simple pipeline handler ?\n> \n> Yes.  It started happening to me once I reinstalled my development\n> system to Fedora 41.  I can't see any obvious reason why it crashes now\n> and not before (maybe some change in gcc? -- using 14.2.1).\n> \n\nIt will be curious if this only appears as a result of changing external\ntooling though !\n\n> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\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 672c985a..aa9459cf 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 ||\n> >> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n> >>                 return 0;\n> >>  \n> >>         /*\n> >> -- \n> >> 2.48.1\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 B6FD3C32F4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 15 Mar 2025 13:20:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5BD5968942;\n\tSat, 15 Mar 2025 14:20:16 +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 CE276617F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 15 Mar 2025 14:20:13 +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 9078A74C;\n\tSat, 15 Mar 2025 14:18:33 +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=\"PEc2nlUV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742044713;\n\tbh=cn1TMeixMzWDMGAVENZ6ZpCcD50XRKIjeX20mCtq508=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=PEc2nlUV+0ChHfUQMgg5Y0hyrpkinOJ77xcCVG2d4XJRPempRI1RCtptMscwm5J/O\n\tSrGzW+aQd5Hwy1TcOGnPMVFWACwAGrMK4Uicgi7ja3qEh4u8TAtBY8NTm9htyG5mwN\n\tNdyi2XmL0OBEaAw7ZZGc14FXn+1Hqcjv5RIsLz1g=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85wmcr91ud.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20250310110630.34857-1-mzamazal@redhat.com>\n\t<174194299442.1289405.15170002880350501354@ping.linuxembedded.co.uk>\n\t<85wmcr91ud.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","To":"Milan Zamazal <mzamazal@redhat.com>","Date":"Sat, 15 Mar 2025 13:20:09 +0000","Message-ID":"<174204480988.3467613.7490676062480537223@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":33671,"web_url":"https://patchwork.libcamera.org/comment/33671/","msgid":"<85y0wzhb7p.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-03-20T17:18:50","subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n\n> Quoting Milan Zamazal (2025-03-14 13:30:18)\n>> Hi Kieran,\n>> \n>\n>> Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n>> \n>> > Quoting Milan Zamazal (2025-03-10 11:06:30)\n>> >> cfg.colorSpace may be unset in KMSSink::configure, resulting in a crash\n>> >> when it is accessed.  If cfg.colorSpace is unset, simply return, the\n>> >> same way as when YcbcrEncoding is set to None.\n>> >\n>> > I think this is something that we should ensure is trapped by\n>> > lc-compliance in fact.\n>> >\n>> > I believe pipeline handlers /must/ always set the correct colorSpace\n>> > after validate - so it's incorrect for applications to ever hit an\n>> > undefined color space ...\n>> \n>> I'm not sure whether all the pipelines do that; at least `simple'\n>> doesn't.  I can fix `simple' pipeline but maybe some others have the\n>> problem too.\n>\n> That's why I think we need to do better on lc-complicance tests to\n> ensure 'rules' are always validated.\n>\n>> > Of course crashing isn't nice either ... \n>> \n>> Let's have an assertion there then to still expose the problem while\n>> crashing in a civilized way?\n>\n> In kmssink? I'm fine adding an assertion here - but what about the other\n> sinks ? What about the other applciations. That's why 'this' should be\n> validated for /all/ pipeline handlers.\n\nWell, I posted a patch that sets colorSpace in the simple pipeline.  It\nfixes the crash for me.\n\nAs for the assertion, I realized it wouldn't be a good idea to add it\nnow.  Since the crash seems to be environment dependent in some way,\nadding the assertion might cause previously unmet crashes with pipelines\nnot setting colorSpace.\n\nUp to you how to proceed with the rest.\n\n> There should be a contract somewhere that states a pipeline handler\n> /must/ fill in specific fields.\n>\n> I have an upcoming proposal that could let us add a validation layer to\n> all runtimes too which could be another place to add runtime\n> validations.\n>\n> Hopefully I'll get that out next week as an initial RFC.\n>\n>> > but is this occuring in SoftISP/simple pipeline handler ?\n>> \n>> Yes.  It started happening to me once I reinstalled my development\n>> system to Fedora 41.  I can't see any obvious reason why it crashes now\n>> and not before (maybe some change in gcc? -- using 14.2.1).\n>> \n>\n> It will be curious if this only appears as a result of changing external\n> tooling though !\n\nIt looks compiler version dependent.  It doesn't crash on my Debix but\nwhen I add an assertion there then it fails.  So the bug was there but\nit got exposed to me only in current Fedora.\n\n>> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\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 672c985a..aa9459cf 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 ||\n>> >> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>> >>                 return 0;\n>> >>  \n>> >>         /*\n>> >> -- \n>> >> 2.48.1\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 81451C3301\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 20 Mar 2025 17:18:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A52068954;\n\tThu, 20 Mar 2025 18:18:57 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 079626894F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 20 Mar 2025 18:18:55 +0100 (CET)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-527-ZuOIWyPYOQq_VfUjn5KekA-1; Thu, 20 Mar 2025 13:18:53 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-43935e09897so7381205e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 20 Mar 2025 10:18:53 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-3997f9b3ebcsm148122f8f.47.2025.03.20.10.18.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 20 Mar 2025 10:18:50 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"VDrm+jkY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1742491134;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=9tV7pPitfMnEeewedsc+KUHI8lZbkKWbyuS+mX46nhY=;\n\tb=VDrm+jkYyIVrU8RuRh15YYoJRjwmJght1rXuhajeVO5GWhZawp0S9Fw9iW40qzUY8yjyqU\n\tOLj25H1CqCDENFVHFwWHYZHrYHCoFE8iZLsDjbs/PNa3U4tkXs+2SE0vQr35ecb88OWJxz\n\tvC+G64UCzKCZ908wxppKT4ZQ0/5i+dI=","X-MC-Unique":"ZuOIWyPYOQq_VfUjn5KekA-1","X-Mimecast-MFC-AGG-ID":"ZuOIWyPYOQq_VfUjn5KekA_1742491132","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1742491132; x=1743095932;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=9tV7pPitfMnEeewedsc+KUHI8lZbkKWbyuS+mX46nhY=;\n\tb=AUP32zubFSaghBkjQUuLjP9GwF0CeUjOVWsWawXkXRi4RCSCmQlh03uuEP9wJ1t92E\n\tvm4WlaLfBwcMnbnWHNMQPZr/r7T1r1M116swcz2VloNbF58J/Z/yM7zmqvSD7b4QiJOD\n\tDxFGeuYNi/Qi4R7OaBk8OFzSDUKp4zXOPpOyOKml4ydEDHTNvgvrzB2W0n/pbiA09VVO\n\tgIMouI0M+GNr2cjJJvdiMXeN/WCwginPDqgj032JYwSK2TUYdSMGhyCAqhKU008wewZY\n\tOcVfqurcT+m1hHkkGHe6nPAMYiDuU8Sx+HB1Rz1gMSmVvULtxhYEl1c3AnRZra8PH+Lw\n\tDlog==","X-Gm-Message-State":"AOJu0YxR2Hi0ig/ld7+s29d0vN0tCGJRnIKDkKMcFxLlQRSKFkDaj9ck\n\tAWDXydH15FxV3MNBXoE9uI4tOUyKozRJUoyWkPsI+1zeWXYdSKGNKY0SWLOviWkEArcrjHZR/0y\n\ttqwgNfV2VmCQURKM4ei7g6UFrIX5g827Rw/tviSd7vmdDx5MxMGHrOSO4En44ZLMQ/eBu9jaefA\n\t5xSuG8haJB1J5In5SiK2KEAjHJvYu5hAJe024uU5SLRwuoIJA8HZa56zA/Ng==","X-Gm-Gg":"ASbGncsXlPXuypAWRAW0oESCIaVAgY3qvi/QDsiMCjJIAEzZ5dk1C0xTTfMHKq4Tjun\n\t9WWUOTvBI4KDQs5GPteuggWfpemfkmCwKJF/Qysd7kXFAPvj3m45VU6A3wh3StkLUYzq5Zex5Th\n\tyA6e8ThfKmNTXESyq5MjvNhBJXwjlBENweeNrTkBM2L2McH0spmmzmnt6gHX/IORdXxobtXdijo\n\tU8uvjSGSiVDPa/kDyGXCTFsb5jelHcczuxY6JbGssrTT3245hTTp/D9wku2lZBOvk2OBSZYaAMN\n\t8aSoBFz43UCKNkiO7L4m2h5Ou++vMVtf8HAeYwISGWhNF3PqnDzGDNN2CU9Sj5+PfieT","X-Received":["by 2002:a5d:64af:0:b0:391:2bcc:11f2 with SMTP id\n\tffacd0b85a97d-3997f8f8321mr420514f8f.1.1742491131922; \n\tThu, 20 Mar 2025 10:18:51 -0700 (PDT)","by 2002:a5d:64af:0:b0:391:2bcc:11f2 with SMTP id\n\tffacd0b85a97d-3997f8f8321mr420478f8f.1.1742491131368; \n\tThu, 20 Mar 2025 10:18:51 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGvFi9md3iVLR1iZiXiFvHJqTfGg5LlPybulBSqcl2CGv4BzywO7kQoYneafznQelewfWd7Wg==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","In-Reply-To":"<174204480988.3467613.7490676062480537223@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Sat, 15 Mar 2025 13:20:09 +0000\")","References":"<20250310110630.34857-1-mzamazal@redhat.com>\n\t<174194299442.1289405.15170002880350501354@ping.linuxembedded.co.uk>\n\t<85wmcr91ud.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<174204480988.3467613.7490676062480537223@ping.linuxembedded.co.uk>","Date":"Thu, 20 Mar 2025 18:18:50 +0100","Message-ID":"<85y0wzhb7p.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"dM4Y7dB9Nrsnkar0OlFB6xfStkA5eCywGnjZafIYaEQ_1742491132","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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":34688,"web_url":"https://patchwork.libcamera.org/comment/34688/","msgid":"<175101708687.3281735.10980550821756873153@ping.linuxembedded.co.uk>","date":"2025-06-27T09:38:06","subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"And now I'm working my way up from December to March I see both Milan\nand Antoine have posted a similar fix but for usage on different\npipelines.\n\nIt seems we need to do something to resolve these...\n\nSee https://patchwork.libcamera.org/patch/22376/\n\n\nQuoting Milan Zamazal (2025-03-20 17:18:50)\n> Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n> \n> > Quoting Milan Zamazal (2025-03-14 13:30:18)\n> >> Hi Kieran,\n> >> \n> >\n> >> Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n> >> \n> >> > Quoting Milan Zamazal (2025-03-10 11:06:30)\n> >> >> cfg.colorSpace may be unset in KMSSink::configure, resulting in a crash\n> >> >> when it is accessed.  If cfg.colorSpace is unset, simply return, the\n> >> >> same way as when YcbcrEncoding is set to None.\n> >> >\n> >> > I think this is something that we should ensure is trapped by\n> >> > lc-compliance in fact.\n> >> >\n> >> > I believe pipeline handlers /must/ always set the correct colorSpace\n> >> > after validate - so it's incorrect for applications to ever hit an\n> >> > undefined color space ...\n> >> \n> >> I'm not sure whether all the pipelines do that; at least `simple'\n> >> doesn't.  I can fix `simple' pipeline but maybe some others have the\n> >> problem too.\n> >\n> > That's why I think we need to do better on lc-complicance tests to\n> > ensure 'rules' are always validated.\n> >\n> >> > Of course crashing isn't nice either ... \n> >> \n> >> Let's have an assertion there then to still expose the problem while\n> >> crashing in a civilized way?\n> >\n> > In kmssink? I'm fine adding an assertion here - but what about the other\n> > sinks ? What about the other applciations. That's why 'this' should be\n> > validated for /all/ pipeline handlers.\n> \n> Well, I posted a patch that sets colorSpace in the simple pipeline.  It\n> fixes the crash for me.\n> \n> As for the assertion, I realized it wouldn't be a good idea to add it\n> now.  Since the crash seems to be environment dependent in some way,\n> adding the assertion might cause previously unmet crashes with pipelines\n> not setting colorSpace.\n> \n> Up to you how to proceed with the rest.\n> \n> > There should be a contract somewhere that states a pipeline handler\n> > /must/ fill in specific fields.\n> >\n> > I have an upcoming proposal that could let us add a validation layer to\n> > all runtimes too which could be another place to add runtime\n> > validations.\n> >\n> > Hopefully I'll get that out next week as an initial RFC.\n> >\n> >> > but is this occuring in SoftISP/simple pipeline handler ?\n> >> \n> >> Yes.  It started happening to me once I reinstalled my development\n> >> system to Fedora 41.  I can't see any obvious reason why it crashes now\n> >> and not before (maybe some change in gcc? -- using 14.2.1).\n> >> \n> >\n> > It will be curious if this only appears as a result of changing external\n> > tooling though !\n> \n> It looks compiler version dependent.  It doesn't crash on my Debix but\n> when I add an assertion there then it fails.  So the bug was there but\n> it got exposed to me only in current Fedora.\n> \n> >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\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 672c985a..aa9459cf 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 ||\n> >> >> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n> >> >>                 return 0;\n> >> >>  \n> >> >>         /*\n> >> >> -- \n> >> >> 2.48.1\n> >> >>\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 51FAEBDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Jun 2025 09:38:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAE9168DFC;\n\tFri, 27 Jun 2025 11:38:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A8CE62C43\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Jun 2025 11:38:10 +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 86235351;\n\tFri, 27 Jun 2025 11:37:50 +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=\"dklfXoI3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1751017070;\n\tbh=+ktrIJsN5ZSytiMgM5iHp6ML43i5O7UDsM3aPJ8IlrI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=dklfXoI3kYojhopPnlCC8aCdlrJUh3K8gnnBHD2ONFq3Qj6HdFKHxbSvZANfKLOXK\n\tpDKDTseP61ozw9w+l2CYKSC1PX7+mLusypfMRI4veHKkBd2KxPbQBs5QUHLA6WyRxI\n\tRM5pkw6qrJEHPd39Q+/0dohMYapvqf7ccQf9JGCY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85y0wzhb7p.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20250310110630.34857-1-mzamazal@redhat.com>\n\t<174194299442.1289405.15170002880350501354@ping.linuxembedded.co.uk>\n\t<85wmcr91ud.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<174204480988.3467613.7490676062480537223@ping.linuxembedded.co.uk>\n\t<85y0wzhb7p.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","To":"Milan Zamazal <mzamazal@redhat.com>,\n\tAntoine Bouyer <antoine.bouyer@nxp.com>","Date":"Fri, 27 Jun 2025 10:38:06 +0100","Message-ID":"<175101708687.3281735.10980550821756873153@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":34693,"web_url":"https://patchwork.libcamera.org/comment/34693/","msgid":"<85plepwk0k.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-27T10:34:19","subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi,\n\nKieran Bingham <kieran.bingham@ideasonboard.com> writes:\n\n> And now I'm working my way up from December to March I see both Milan\n> and Antoine have posted a similar fix but for usage on different\n> pipelines.\n>\n> It seems we need to do something to resolve these...\n\nThe crash is quite annoying, I have to apply the patch in each branch I\nwork on.\n\nIIRC Laurent asked me to fix the problem in `simple' pipeline so I added\na (different) fix to my raw stream patches:\nhttps://patchwork.libcamera.org/patch/23432/.  But those are stuck in\nreview -- could we perhaps move on with at least that first patch?\n\nThis could clarify what's the right thing to do in all the affected\npipelines; or we can simply merge the Antoine's or my kms_sink.cpp\none-liner to prevent the crash.\n\n> See https://patchwork.libcamera.org/patch/22376/\n>\n>\n> Quoting Milan Zamazal (2025-03-20 17:18:50)\n>> Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n>> \n>> > Quoting Milan Zamazal (2025-03-14 13:30:18)\n>> >> Hi Kieran,\n>> >> \n>> >\n>> >> Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n>> >> \n>> >> > Quoting Milan Zamazal (2025-03-10 11:06:30)\n>> >> >> cfg.colorSpace may be unset in KMSSink::configure, resulting in a crash\n>> >> >> when it is accessed.  If cfg.colorSpace is unset, simply return, the\n>> >> >> same way as when YcbcrEncoding is set to None.\n>> >> >\n>> >> > I think this is something that we should ensure is trapped by\n>> >> > lc-compliance in fact.\n>> >> >\n>> >> > I believe pipeline handlers /must/ always set the correct colorSpace\n>> >> > after validate - so it's incorrect for applications to ever hit an\n>> >> > undefined color space ...\n>> >> \n>> >> I'm not sure whether all the pipelines do that; at least `simple'\n>> >> doesn't.  I can fix `simple' pipeline but maybe some others have the\n>> >> problem too.\n>> >\n>> > That's why I think we need to do better on lc-complicance tests to\n>> > ensure 'rules' are always validated.\n>> >\n>> >> > Of course crashing isn't nice either ... \n>> >> \n>> >> Let's have an assertion there then to still expose the problem while\n>> >> crashing in a civilized way?\n>> >\n>> > In kmssink? I'm fine adding an assertion here - but what about the other\n>> > sinks ? What about the other applciations. That's why 'this' should be\n>> > validated for /all/ pipeline handlers.\n>> \n>> Well, I posted a patch that sets colorSpace in the simple pipeline.  It\n>> fixes the crash for me.\n>> \n>> As for the assertion, I realized it wouldn't be a good idea to add it\n>> now.  Since the crash seems to be environment dependent in some way,\n>> adding the assertion might cause previously unmet crashes with pipelines\n>> not setting colorSpace.\n>> \n>> Up to you how to proceed with the rest.\n>> \n>> > There should be a contract somewhere that states a pipeline handler\n>> > /must/ fill in specific fields.\n>> >\n>> > I have an upcoming proposal that could let us add a validation layer to\n>> > all runtimes too which could be another place to add runtime\n>> > validations.\n>> >\n>> > Hopefully I'll get that out next week as an initial RFC.\n>> >\n>> >> > but is this occuring in SoftISP/simple pipeline handler ?\n>> >> \n>> >> Yes.  It started happening to me once I reinstalled my development\n>> >> system to Fedora 41.  I can't see any obvious reason why it crashes now\n>> >> and not before (maybe some change in gcc? -- using 14.2.1).\n>> >> \n>> >\n>> > It will be curious if this only appears as a result of changing external\n>> > tooling though !\n>> \n>> It looks compiler version dependent.  It doesn't crash on my Debix but\n>> when I add an assertion there then it fails.  So the bug was there but\n>> it got exposed to me only in current Fedora.\n>> \n>> >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\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 672c985a..aa9459cf 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 ||\n>> >> >> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)\n>> >> >>                 return 0;\n>> >> >>  \n>> >> >>         /*\n>> >> >> -- \n>> >> >> 2.48.1\n>> >> >>\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 00E6CC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Jun 2025 10:34:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A8E4D68DFE;\n\tFri, 27 Jun 2025 12:34:27 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E49F068DE5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Jun 2025 12:34:25 +0200 (CEST)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-252-q4o1fpJLMpWGx3Wz9ilAnw-1; Fri, 27 Jun 2025 06:34:23 -0400","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-ad89c32a8a6so212425566b.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Jun 2025 03:34:23 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-ae35363a167sm98962866b.21.2025.06.27.03.34.19\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 27 Jun 2025 03:34:20 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"A0FrLFMr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1751020464;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=VeI9TCcmkF4mEFKwz/JvT5dBcHBfcdWMvnwIu+FQv00=;\n\tb=A0FrLFMr0hzJ8kKSp/CYWVH/M+MffSOK1289eGdyR7WVeF43en+/Y5dNPDmzJ9GBTzOLAc\n\ttVuqk1nvjsdyQkTtYqgBgs8KOQrTLrdj1bsyqu3teYyFotLH0BocAaXGkJ5PChwhcG2bBp\n\tXco4gb/DNmi8TqVJXHFh3/YpEswyKK0=","X-MC-Unique":"q4o1fpJLMpWGx3Wz9ilAnw-1","X-Mimecast-MFC-AGG-ID":"q4o1fpJLMpWGx3Wz9ilAnw_1751020462","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1751020462; x=1751625262;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=VeI9TCcmkF4mEFKwz/JvT5dBcHBfcdWMvnwIu+FQv00=;\n\tb=ukzfmXeEN4NqcZbIJ1vxEu1hXuA7/L8pGcsyuUdUgZwqyZ5ulrZVa+ReISQr7gd2Go\n\tHPAn7dmIqkxD9irukMYKR2Npw68l6GC8WhjJRAc1ibtDItGpTMGw9q7oq1Kxl8F1P2NX\n\tPLa4Vj8ccpvDPSbsbwIHNDrpWbJptTeson+kX9BY0KejKC0FfJtj+M47aJXwFYEp/Wr2\n\t5ej2XgLuEWVG0KeUEOyVyq5XRYEHDR4OCr/XW+AU2pSnpbNs81QNH/HIFyVuJ0EmTv46\n\tTdh2EfsXitDDL3oi50NUegQBO/2fSbXxlI9b+RKBy5qg/6eKrWS0U+WdEcMiO4iBbURM\n\tVseA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUCd6mRimEKBQ5xWA59WPqnY1kpdU/H4dSdDLx8EXSeHgXfbUrRl+qVRCeMSVrGVoV85/GsPT7hclqhOnC4LlU=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yx6BKdvHorCEGJ83Rv9nDpERd8sogZdB/BqopUoZtlWI9Ew9vn8\n\ta9Y0bLIwUSkQt8SntLU0bSmVVU4TTuYOs63L1O4qBKd2iGvXHig7Kybsf/Iq3s42D8VYn+AiMB0\n\t1ruBKDRrjt8pC9xRv8Z+/2wu8gH/gMbQ6xJiMcNQmLSfLFiNRYlUY7hWB3yTd18YCK9RR6dSC34\n\t4CtY5tLqRee6D/KmH5okCK4LN3rsJYGjwni2V7IDhiPrHISQl4G+XC2EhrN3s=","X-Gm-Gg":"ASbGncv7IgcrlIMFjrbTKv1UipyvZ22ZaTQJ9JE/NRcpHgtcMV2ZK6uL4kct+bL1Z6j\n\t/W/rbE0hvfmgXBR1ZCFQ/QT65V1mTu96r6GqvKXgP6tnJ0csZ14Tj15sizBagZZsRt7GEyQOB54\n\tN/jXG6+8FQBHxemFjwzS+24kLXrU9pmffXdSL77WZXYUMxZxlHdxq1L0u1hZGcM5JPDm48VG+E+\n\tIO8yxQ7kN3RsWosVBsVTpICn/6olCyqsXiXd+zbFurUBR09pLgvFtfg3Tt7yDL4fXHIzlSy+Wyd\n\tLM5EiDwznURQiMAR6U/E1oet7AZ1JVCPs0UmSU5v+tiy80afYQdbzk29EFsd6hQCLtQNEGE0ovc\n\t=","X-Received":["by 2002:a17:907:d29:b0:ae0:da2f:dcf3 with SMTP id\n\ta640c23a62f3a-ae3502093acmr210917666b.59.1751020461575; \n\tFri, 27 Jun 2025 03:34:21 -0700 (PDT)","by 2002:a17:907:d29:b0:ae0:da2f:dcf3 with SMTP id\n\ta640c23a62f3a-ae3502093acmr210915666b.59.1751020461045; \n\tFri, 27 Jun 2025 03:34:21 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IG6chHehP9vQVfhMRs3wa/dcA944nBm1oCLSEiuYn6AQIHQRIyvIoquLDO/ooQvi4RolJZeKg==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Antoine Bouyer <antoine.bouyer@nxp.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] apps: cam: Fix colorSpace access crash in\n\tKMSSink::configure","In-Reply-To":"<175101708687.3281735.10980550821756873153@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Fri, 27 Jun 2025 10:38:06 +0100\")","References":"<20250310110630.34857-1-mzamazal@redhat.com>\n\t<174194299442.1289405.15170002880350501354@ping.linuxembedded.co.uk>\n\t<85wmcr91ud.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<174204480988.3467613.7490676062480537223@ping.linuxembedded.co.uk>\n\t<85y0wzhb7p.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<175101708687.3281735.10980550821756873153@ping.linuxembedded.co.uk>","Date":"Fri, 27 Jun 2025 12:34:19 +0200","Message-ID":"<85plepwk0k.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"AE5Rnt8n8IRwAftDgrzJ81vVOaCibWvcS3Upc6Kt2cE_1751020462","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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>"}}]