public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: uvcvideo: Handle errors from calls to usb_string
@ 2020-11-12 18:17 Guenter Roeck
  2020-11-22 13:48 ` Guenter Roeck
  0 siblings, 1 reply; 2+ messages in thread
From: Guenter Roeck @ 2020-11-12 18:17 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Guenter Roeck

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>
---
 drivers/media/usb/uvc/uvc_driver.c | 48 ++++++++++++------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index ddb9eaa11be7..15f5a0673237 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1118,10 +1118,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);
@@ -1246,15 +1244,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;
@@ -1286,10 +1284,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);
@@ -1311,10 +1307,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);
@@ -1344,10 +1338,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);
@@ -1375,10 +1367,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);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] media: uvcvideo: Handle errors from calls to usb_string
  2020-11-12 18:17 [PATCH] media: uvcvideo: Handle errors from calls to usb_string Guenter Roeck
@ 2020-11-22 13:48 ` Guenter Roeck
  0 siblings, 0 replies; 2+ messages in thread
From: Guenter Roeck @ 2020-11-22 13:48 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

ping ...

On Thu, Nov 12, 2020 at 10:17:16AM -0800, Guenter Roeck wrote:
> 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>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 48 ++++++++++++------------------
>  1 file changed, 19 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index ddb9eaa11be7..15f5a0673237 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1118,10 +1118,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);
> @@ -1246,15 +1244,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;
> @@ -1286,10 +1284,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);
> @@ -1311,10 +1307,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);
> @@ -1344,10 +1338,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);
> @@ -1375,10 +1367,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);
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-11-22 13:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-12 18:17 [PATCH] media: uvcvideo: Handle errors from calls to usb_string Guenter Roeck
2020-11-22 13:48 ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox