linux-kernel - Re: [PATCH] media: uvcvideo: Fix deferred probing error

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: <20250223143617.GA27463@pendragon.ideasonboard.com>
Date: Sun, 23 Feb 2025 16:36:17 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Ricardo Ribalda <ribalda@...omium.org>
Cc: Hans de Goede <hdegoede@...hat.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] media: uvcvideo: Fix deferred probing error

Hi Ricardo,

Thank you for the patch.

On Wed, Jan 29, 2025 at 12:39:46PM +0000, Ricardo Ribalda wrote:
> uvc_gpio_parse() can return -EPROBE_DEFER when the GPIOs it depends on
> have not yet been probed. This return code should be propagated to the
> caller of uvc_probe() to ensure that probing is retried when the required
> GPIOs become available.
> 
> Currently, this error code is incorrectly converted to -ENODEV,
> causing some internal cameras to be ignored.
> 
> This commit fixes this issue by propagating the -EPROBE_DEFER error.
> 
> Cc: stable@...r.kernel.org
> Fixes: 2886477ff987 ("media: uvcvideo: Implement UVC_EXT_GPIO_UNIT")
> Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index a10d4f4d9f95..73a7f23b616c 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2253,9 +2253,10 @@ static int uvc_probe(struct usb_interface *intf,
>  	}
>  
>  	/* Parse the associated GPIOs. */
> -	if (uvc_gpio_parse(dev) < 0) {
> +	ret = uvc_gpio_parse(dev);
> +	if (ret < 0) {
>  		uvc_dbg(dev, PROBE, "Unable to parse UVC GPIOs\n");
> -		goto error;
> +		goto error_retcode;
>  	}
>  
>  	dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n",
> @@ -2328,9 +2329,11 @@ static int uvc_probe(struct usb_interface *intf,
>  	return 0;
>  
>  error:
> +	ret = -ENODEV;
> +error_retcode:

This isn't very nice. Could we instead also propagate error codes from
other locations in the uvc_probe() function ? If you want to minimize
changes, you can initialize ret to -ENODEV, and turn the (ret < 0) check
for uvc_gpio_parse() to a (ret) check.

>  	uvc_unregister_video(dev);
>  	kref_put(&dev->ref, uvc_delete);
> -	return -ENODEV;
> +	return ret;
>  }
>  
>  static void uvc_disconnect(struct usb_interface *intf)
> 
> ---
> base-commit: c4b7779abc6633677e6edb79e2809f4f61fde157
> change-id: 20250129-uvc-eprobedefer-b5ebb4db63cc

-- 
Regards,

Laurent Pinchart