| 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
| ||
|
Message-ID: <20251021164443.GA727@pendragon.ideasonboard.com>
Date: Tue, 21 Oct 2025 19:44:43 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Ricardo Ribalda <ribalda@...omium.org>
Cc: Hans de Goede <hansg@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Hans Verkuil <hverkuil+cisco@...nel.org>,
Thadeu Lima de Souza Cascardo <cascardo@...lia.com>,
Angel4005 <ooara1337@...il.com>, linux-media@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] media: uvcvideo: Use heuristic to find stream entity
Hi Ricardo,
Thank you for the patch.
On Tue, Oct 21, 2025 at 10:36:17AM +0000, Ricardo Ribalda wrote:
> Some devices, like the Grandstream GUV3100 webcam, have an invalid UVC
> descriptor where multiple entities share the same ID, this is invalid
> and makes it impossible to make a proper entity tree without heuristics.
>
> We have recently introduced a change in the way that we handle invalid
> entities that has caused a regression on broken devices.
>
> Implement a new heuristic to handle these devices properly.
>
> Reported-by: Angel4005 <ooara1337@...il.com>
> Closes: https://lore.kernel.org/linux-media/CAOzBiVuS7ygUjjhCbyWg-KiNx+HFTYnqH5+GJhd6cYsNLT=DaA@mail.gmail.com/
> Fixes: 0e2ee70291e6 ("media: uvcvideo: Mark invalid entities with id UVC_INVALID_ENTITY_ID")
> Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
> I have managed to get my hands into a Grandstream GUV3100 and
> implemented a new heuristics. (Thanks Yunke and Hidenori!).
>
> With this heuristics we can use this camera again (see the /dev/video0
> in the topology).
>
> I have tested this change in a 6.6 kernel. Because the notebook that I
> used for testing has some issues running master. But for the purpose of
> this change this test should work.
>
> ~ # media-ctl --print-topology
> Media controller API version 6.6.99
>
> Media device information
> ------------------------
> driver uvcvideo
> model GRANDSTREAM GUV3100: GRANDSTREA
> serial
> bus info usb-0000:00:14.0-9
> hw revision 0x409
> driver version 6.6.99
>
> Device topology
> - entity 1: GRANDSTREAM GUV3100: GRANDSTREA (1 pad, 1 link)
> type Node subtype V4L flags 1
> device node name /dev/video0
> pad0: SINK
> <- "Extension 3":1 [ENABLED,IMMUTABLE]
>
> - entity 4: GRANDSTREAM GUV3100: GRANDSTREA (0 pad, 0 link)
> type Node subtype V4L flags 0
> device node name /dev/video1
>
> - entity 8: Extension 3 (2 pads, 2 links, 0 routes)
> type V4L2 subdev subtype Unknown flags 0
> pad0: SINK
> <- "Processing 2":1 [ENABLED,IMMUTABLE]
> pad1: SOURCE
> -> "GRANDSTREAM GUV3100: GRANDSTREA":0 [ENABLED,IMMUTABLE]
>
> - entity 11: Processing 2 (2 pads, 3 links, 0 routes)
> type V4L2 subdev subtype Unknown flags 0
> pad0: SINK
> <- "Camera 1":0 [ENABLED,IMMUTABLE]
> pad1: SOURCE
> -> "Extension 3":0 [ENABLED,IMMUTABLE]
> -> "Extension 4":0 [ENABLED,IMMUTABLE]
>
> - entity 14: Extension 4 (2 pads, 1 link, 0 routes)
> type V4L2 subdev subtype Unknown flags 0
> pad0: SINK
> <- "Processing 2":1 [ENABLED,IMMUTABLE]
> pad1: SOURCE
>
> - entity 17: Camera 1 (1 pad, 1 link, 0 routes)
> type V4L2 subdev subtype Sensor flags 0
> pad0: SOURCE
> -> "Processing 2":0 [ENABLED,IMMUTABLE]
> ---
> Changes in v2:
> - Fix : invalid reference to the index variable of the iterator.
> - Link to v1: https://lore.kernel.org/r/20251021-uvc-grandstream-v1-1-801e3d08b271@chromium.org
> ---
> drivers/media/usb/uvc/uvc_driver.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index fb6afb8e84f00961f86fd8f840fba48d706d7a9a..ee4f54d6834962414979a046afc59c5036455124 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -167,13 +167,26 @@ static struct uvc_entity *uvc_entity_by_reference(struct uvc_device *dev,
>
> static struct uvc_streaming *uvc_stream_by_id(struct uvc_device *dev, int id)
> {
> - struct uvc_streaming *stream;
> + struct uvc_streaming *stream, *last_stream;
> + unsigned int count = 0;
>
> list_for_each_entry(stream, &dev->streams, list) {
> + count += 1;
> + last_stream = stream;
> if (stream->header.bTerminalLink == id)
> return stream;
> }
>
> + /*
> + * If the streaming entity is referenced by an invalid ID, notify the
> + * user and use heuristics to guess the correct entity.
> + */
> + if (count == 1 && id == UVC_INVALID_ENTITY_ID) {
> + dev_warn(&dev->intf->dev,
> + "UVC non compliance: Invalid USB header. The streaming entity has an invalid ID, guessing the correct one.");
> + return last_stream;
> + }
As far as I understand, the reason why we can't find the streaming
interface here is because we have set the streaming output terminal ID
to UVC_INVALID_ENTITY_ID, due to the extension unit being previously
registered with the same ID. We're therefore adding a workaround on top
of another workaround.
Looking at the UVC descriptors for this camera, ID 4 is shared between
an extension unit with GUID ffe52d21-8030-4e2c-82d9-f587d00540bd and the
streaming output terminal. That GUID is apparently a Logitech GUID
(according to https://github.com/soyersoyer/cameractrls/blob/c920e/cameractrls.py).
I don't know if the XU actually works in that camera, but it could be
useful to keep it functional.
I think we could make the handling of non-unique IDs more clever. If an
output streaming terminal and a unit share the same ID, we could
allocate an unused ID for the output streaming terminal, and patch the
bTerminalLink of the stream headers accordingly. The ID remapping should
not cause other issues, as
- streaming output terminals have no controls, so the ID isn't used to
reference controls
- the units graph is build from sink to source, with UVC descriptors
containing the IDs of source units, so streaming output terminals are
never referenced by ID from descriptors except for the bTerminalLink.
This would produce a valid graph without duplicated IDs in this case.
> +
> return NULL;
> }
>
>
> ---
> base-commit: ea299a2164262ff787c9d33f46049acccd120672
> change-id: 20251021-uvc-grandstream-05ecf0288f62
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists