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] [thread-next>] [day] [month] [year] [list]

Message-ID: <xgycmti27be4ah64o4mflj6i2nzh34tbnlfqghiizqhtkusuif@ia3idwgrjoil>
Date: Fri, 20 Jun 2025 10:22:14 +0200
From: Mehdi Djait <mehdi.djait@...ux.intel.com>
To: Michael Riesch <michael.riesch@...labora.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 Michael,

On Wed, Jun 18, 2025 at 02:21:51PM +0200, Michael Riesch wrote:
> Hi Mehdi,
> 
> On 6/13/25 14:33, Mehdi Djait wrote:
> > Hi Michael,
> > 
> > [...]
> 
> Let's cut the long story short, shall we :-)
> 

Yes, sorry for that :)

> >> +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.
> 

Of course! The solution from the driver side is to check the return
code.

I will investigate the issue with v4l2_fill_pixfmt_mp() separately.

> Best regards,
> Michael

--
Kind Regards
Mehdi Djait