* [PATCH v1 0/1] [RESEND] media: uvcvideo: Handle cameras with invalid usb_strings
@ 2022-10-25 14:41 Ricardo Ribalda
2022-10-25 14:41 ` [PATCH v1 1/1] media: uvcvideo: Handle errors from calls to usb_string Ricardo Ribalda
0 siblings, 1 reply; 3+ messages in thread
From: Ricardo Ribalda @ 2022-10-25 14:41 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab
Cc: linux-media, Guenter Roeck, linux-kernel, Ricardo Ribalda
Resend a patch from Guenter that felt intro the cracks...
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Guenter Roeck (1):
media: uvcvideo: Handle errors from calls to usb_string
drivers/media/usb/uvc/uvc_driver.c | 48 +++++++++++++++-----------------------
1 file changed, 19 insertions(+), 29 deletions(-)
---
base-commit: 1a2dcbdde82e3a5f1db9b2f4c48aa1aeba534fb2
change-id: 20221025-usb-string-cff7f1f06e0f
Best regards,
--
Ricardo Ribalda <ribalda@chromium.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v1 1/1] media: uvcvideo: Handle errors from calls to usb_string
2022-10-25 14:41 [PATCH v1 0/1] [RESEND] media: uvcvideo: Handle cameras with invalid usb_strings Ricardo Ribalda
@ 2022-10-25 14:41 ` Ricardo Ribalda
2022-10-25 18:38 ` Laurent Pinchart
0 siblings, 1 reply; 3+ messages in thread
From: Ricardo Ribalda @ 2022-10-25 14:41 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab
Cc: linux-media, Guenter Roeck, linux-kernel, Ricardo Ribalda
From: Guenter Roeck <linux@roeck-us.net>
On a Webcam from Quanta, we see the following error.
usb 3-5: New USB device found, idVendor=0408, idProduct=30d2, bcdDevice= 0.03
usb 3-5: New USB device strings: Mfr=3, Product=1, SerialNumber=2
usb 3-5: Product: USB2.0 HD UVC WebCam
usb 3-5: Manufacturer: Quanta
usb 3-5: SerialNumber: 0x0001
...
uvcvideo: Found UVC 1.10 device USB2.0 HD UVC WebCam (0408:30d2)
uvcvideo: Failed to initialize entity for entity 5
uvcvideo: Failed to register entities (-22).
The Webcam reports an entity of type UVC_VC_EXTENSION_UNIT. It reports a
string index of '7' associated with that entity. The attempt to read that
string from the camera fails with error -32 (-EPIPE). usb_string() returns
that error, but it is ignored. As result, the entity name is empty. This
later causes v4l2_device_register_subdev() to return -EINVAL, and no
entities are registered as result.
While this appears to be a firmware problem with the camera, the kernel
should still handle the situation gracefully. To do that, check the return
value from usb_string(). If it reports an error, assign the entity's
default name.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 215fb483efb0..828c443faaa9 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -879,10 +879,8 @@ static int uvc_parse_vendor_control(struct uvc_device *dev,
+ n;
memcpy(unit->extension.bmControls, &buffer[23+p], 2*n);
- if (buffer[24+p+2*n] != 0)
- usb_string(udev, buffer[24+p+2*n], unit->name,
- sizeof(unit->name));
- else
+ if (buffer[24+p+2*n] == 0 ||
+ usb_string(udev, buffer[24+p+2*n], unit->name, sizeof(unit->name)) < 0)
sprintf(unit->name, "Extension %u", buffer[3]);
list_add_tail(&unit->list, &dev->entities);
@@ -1006,15 +1004,15 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
memcpy(term->media.bmTransportModes, &buffer[10+n], p);
}
- if (buffer[7] != 0)
- usb_string(udev, buffer[7], term->name,
- sizeof(term->name));
- else if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA)
- sprintf(term->name, "Camera %u", buffer[3]);
- else if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT)
- sprintf(term->name, "Media %u", buffer[3]);
- else
- sprintf(term->name, "Input %u", buffer[3]);
+ if (buffer[7] == 0 ||
+ usb_string(udev, buffer[7], term->name, sizeof(term->name)) < 0) {
+ if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA)
+ sprintf(term->name, "Camera %u", buffer[3]);
+ if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT)
+ sprintf(term->name, "Media %u", buffer[3]);
+ else
+ sprintf(term->name, "Input %u", buffer[3]);
+ }
list_add_tail(&term->list, &dev->entities);
break;
@@ -1047,10 +1045,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
memcpy(term->baSourceID, &buffer[7], 1);
- if (buffer[8] != 0)
- usb_string(udev, buffer[8], term->name,
- sizeof(term->name));
- else
+ if (buffer[8] == 0 ||
+ usb_string(udev, buffer[8], term->name, sizeof(term->name)) < 0)
sprintf(term->name, "Output %u", buffer[3]);
list_add_tail(&term->list, &dev->entities);
@@ -1072,10 +1068,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
memcpy(unit->baSourceID, &buffer[5], p);
- if (buffer[5+p] != 0)
- usb_string(udev, buffer[5+p], unit->name,
- sizeof(unit->name));
- else
+ if (buffer[5+p] == 0 ||
+ usb_string(udev, buffer[5+p], unit->name, sizeof(unit->name)) < 0)
sprintf(unit->name, "Selector %u", buffer[3]);
list_add_tail(&unit->list, &dev->entities);
@@ -1105,10 +1099,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
if (dev->uvc_version >= 0x0110)
unit->processing.bmVideoStandards = buffer[9+n];
- if (buffer[8+n] != 0)
- usb_string(udev, buffer[8+n], unit->name,
- sizeof(unit->name));
- else
+ if (buffer[8+n] == 0 ||
+ usb_string(udev, buffer[8+n], unit->name, sizeof(unit->name)) < 0)
sprintf(unit->name, "Processing %u", buffer[3]);
list_add_tail(&unit->list, &dev->entities);
@@ -1136,10 +1128,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
unit->extension.bmControls = (u8 *)unit + sizeof(*unit);
memcpy(unit->extension.bmControls, &buffer[23+p], n);
- if (buffer[23+p+n] != 0)
- usb_string(udev, buffer[23+p+n], unit->name,
- sizeof(unit->name));
- else
+ if (buffer[23+p+n] == 0 ||
+ usb_string(udev, buffer[23+p+n], unit->name, sizeof(unit->name)) < 0)
sprintf(unit->name, "Extension %u", buffer[3]);
list_add_tail(&unit->list, &dev->entities);
--
b4 0.11.0-dev-d93f8
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/1] media: uvcvideo: Handle errors from calls to usb_string
2022-10-25 14:41 ` [PATCH v1 1/1] media: uvcvideo: Handle errors from calls to usb_string Ricardo Ribalda
@ 2022-10-25 18:38 ` Laurent Pinchart
0 siblings, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2022-10-25 18:38 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, linux-media, Guenter Roeck, linux-kernel
Hi Ricardo and Guenter,
Thank you for the patch.
On Tue, Oct 25, 2022 at 04:41:01PM +0200, Ricardo Ribalda wrote:
> From: Guenter Roeck <linux@roeck-us.net>
>
> On a Webcam from Quanta, we see the following error.
>
> usb 3-5: New USB device found, idVendor=0408, idProduct=30d2, bcdDevice= 0.03
> usb 3-5: New USB device strings: Mfr=3, Product=1, SerialNumber=2
> usb 3-5: Product: USB2.0 HD UVC WebCam
> usb 3-5: Manufacturer: Quanta
> usb 3-5: SerialNumber: 0x0001
> ...
> uvcvideo: Found UVC 1.10 device USB2.0 HD UVC WebCam (0408:30d2)
> uvcvideo: Failed to initialize entity for entity 5
> uvcvideo: Failed to register entities (-22).
>
> The Webcam reports an entity of type UVC_VC_EXTENSION_UNIT. It reports a
> string index of '7' associated with that entity. The attempt to read that
> string from the camera fails with error -32 (-EPIPE). usb_string() returns
> that error, but it is ignored. As result, the entity name is empty. This
> later causes v4l2_device_register_subdev() to return -EINVAL, and no
> entities are registered as result.
>
> While this appears to be a firmware problem with the camera, the kernel
> should still handle the situation gracefully. To do that, check the return
> value from usb_string(). If it reports an error, assign the entity's
> default name.
Sounds good. The perfect world in which firmwares could be fixed doesn't
exist.
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 215fb483efb0..828c443faaa9 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -879,10 +879,8 @@ static int uvc_parse_vendor_control(struct uvc_device *dev,
> + n;
> memcpy(unit->extension.bmControls, &buffer[23+p], 2*n);
>
> - if (buffer[24+p+2*n] != 0)
> - usb_string(udev, buffer[24+p+2*n], unit->name,
> - sizeof(unit->name));
> - else
> + if (buffer[24+p+2*n] == 0 ||
> + usb_string(udev, buffer[24+p+2*n], unit->name, sizeof(unit->name)) < 0)
> sprintf(unit->name, "Extension %u", buffer[3]);
There's quite a bit of common code between all the cases. I'll submit a
patch that refactors it on top. There's thus no need for nitpicking
here, so
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> list_add_tail(&unit->list, &dev->entities);
> @@ -1006,15 +1004,15 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
> memcpy(term->media.bmTransportModes, &buffer[10+n], p);
> }
>
> - if (buffer[7] != 0)
> - usb_string(udev, buffer[7], term->name,
> - sizeof(term->name));
> - else if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA)
> - sprintf(term->name, "Camera %u", buffer[3]);
> - else if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT)
> - sprintf(term->name, "Media %u", buffer[3]);
> - else
> - sprintf(term->name, "Input %u", buffer[3]);
> + if (buffer[7] == 0 ||
> + usb_string(udev, buffer[7], term->name, sizeof(term->name)) < 0) {
> + if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA)
> + sprintf(term->name, "Camera %u", buffer[3]);
> + if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT)
> + sprintf(term->name, "Media %u", buffer[3]);
> + else
> + sprintf(term->name, "Input %u", buffer[3]);
> + }
>
> list_add_tail(&term->list, &dev->entities);
> break;
> @@ -1047,10 +1045,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
>
> memcpy(term->baSourceID, &buffer[7], 1);
>
> - if (buffer[8] != 0)
> - usb_string(udev, buffer[8], term->name,
> - sizeof(term->name));
> - else
> + if (buffer[8] == 0 ||
> + usb_string(udev, buffer[8], term->name, sizeof(term->name)) < 0)
> sprintf(term->name, "Output %u", buffer[3]);
>
> list_add_tail(&term->list, &dev->entities);
> @@ -1072,10 +1068,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
>
> memcpy(unit->baSourceID, &buffer[5], p);
>
> - if (buffer[5+p] != 0)
> - usb_string(udev, buffer[5+p], unit->name,
> - sizeof(unit->name));
> - else
> + if (buffer[5+p] == 0 ||
> + usb_string(udev, buffer[5+p], unit->name, sizeof(unit->name)) < 0)
> sprintf(unit->name, "Selector %u", buffer[3]);
>
> list_add_tail(&unit->list, &dev->entities);
> @@ -1105,10 +1099,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
> if (dev->uvc_version >= 0x0110)
> unit->processing.bmVideoStandards = buffer[9+n];
>
> - if (buffer[8+n] != 0)
> - usb_string(udev, buffer[8+n], unit->name,
> - sizeof(unit->name));
> - else
> + if (buffer[8+n] == 0 ||
> + usb_string(udev, buffer[8+n], unit->name, sizeof(unit->name)) < 0)
> sprintf(unit->name, "Processing %u", buffer[3]);
>
> list_add_tail(&unit->list, &dev->entities);
> @@ -1136,10 +1128,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
> unit->extension.bmControls = (u8 *)unit + sizeof(*unit);
> memcpy(unit->extension.bmControls, &buffer[23+p], n);
>
> - if (buffer[23+p+n] != 0)
> - usb_string(udev, buffer[23+p+n], unit->name,
> - sizeof(unit->name));
> - else
> + if (buffer[23+p+n] == 0 ||
> + usb_string(udev, buffer[23+p+n], unit->name, sizeof(unit->name)) < 0)
> sprintf(unit->name, "Extension %u", buffer[3]);
>
> list_add_tail(&unit->list, &dev->entities);
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-25 18:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-25 14:41 [PATCH v1 0/1] [RESEND] media: uvcvideo: Handle cameras with invalid usb_strings Ricardo Ribalda
2022-10-25 14:41 ` [PATCH v1 1/1] media: uvcvideo: Handle errors from calls to usb_string Ricardo Ribalda
2022-10-25 18:38 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.