linux-kernel - Re: [PATCH v8 06/13] media: rockchip: add a driver for the rockchip camera interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5d3418a3-70f5-46d8-a538-72e6958d5d02@collabora.com>
Date: Wed, 18 Jun 2025 14:21:51 +0200
From: Michael Riesch <michael.riesch@...labora.com>
To: Mehdi Djait <mehdi.djait@...ux.intel.com>
Cc: Maxime Chevallier <maxime.chevallier@...tlin.com>,
 Théo Lebrun <theo.lebrun@...tlin.com>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Gerald Loacker <gerald.loacker@...fvision.net>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Markus Elfring <Markus.Elfring@....de>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring
 <robh+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 Kever Yang <kever.yang@...k-chips.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Collabora Kernel Team <kernel@...labora.com>,
 Paul Kocialkowski <paulk@...-base.io>,
 Alexander Shiyan <eagle.alexander923@...il.com>,
 Val Packett <val@...kett.cool>, Rob Herring <robh@...nel.org>,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-media@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 Mehdi Djait <mehdi.djait@...tlin.com>
Subject: Re: [PATCH v8 06/13] media: rockchip: add a driver for the rockchip
 camera interface

Hi Mehdi,

On 6/13/25 14:33, Mehdi Djait wrote:
> Hi Michael,
> 
> [...]

Let's cut the long story short, shall we :-)

>> +static void rkcif_stream_fill_format(struct rkcif_stream *stream,
>> +				     struct v4l2_pix_format_mplane *pix)
>> +{
>> +	const struct rkcif_output_fmt *fmt;
>> +	u32 height, width;
>> +
>> +	fmt = rkcif_stream_find_output_fmt(stream, true, pix->pixelformat);
>> +	height = clamp_t(u32, pix->height, CIF_MIN_HEIGHT, CIF_MAX_HEIGHT);
>> +	width = clamp_t(u32, pix->width, CIF_MIN_WIDTH, CIF_MAX_WIDTH);
>> +	v4l2_fill_pixfmt_mp(pix, fmt->fourcc, width, height);
> 
> The rkcif supports the SRGGB10P: the packed version of the SRGGB10.
> 
> When you try to capture with SRGGB10P, the following fails in
> v4l2_fill_pixfmt_mp()
> 
> 	info = v4l2_format_info(pixelformat);
> 	if (!info)
> 		return -EINVAL;
> 
> The return value is not checked in rkcif_stream_fill_format() resulting
> in a call to queue_setup returning with sizes[0] = 0

Thanks for pointing that out. I failed to realize that this helper
function may return an error. What I can do is to implement the error
propagation correctly. Of course this will only avoid the kernel
warning, you still won't be able to stream.

> This will cause a kernel Warning in the vb2_core_reqbufs() because
> plane_size = 0
> 
> Exactly here:
> 
> 	for (i = 0; i < num_planes; i++)
> 		if (WARN_ON(!plane_sizes[i])) {
> 			ret = -EINVAL;
> 			goto error;
> 		}
> 
> I still don't have the solution here but wanted to let you know about
> it.

I only had a quick look at it, but apparently the compact formats are
missing in the format list here:
https://elixir.bootlin.com/linux/v6.16-rc2/source/drivers/media/v4l2-core/v4l2-common.c#L244
for whatever reason.

Have you by chance tried adding a line like
{
	.format = V4L2_PIX_FMT_SRGGB10P,
	.pixel_enc = V4L2_PIXEL_ENC_BAYER,
	.mem_planes = 1,
	.comp_planes = 1,
	.bpp = { 5, 0, 0, 0 },
	.bpp_div = { 4, 1, 1, 1 },
	.hdiv = 1,
	.vdiv = 1,
}
?


Maybe we can fix this, but please note that this issue should not block
merging this patch series.

Best regards,
Michael