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

Message-ID: <CAD=FV=XDUmo=OEFtoOt6vkpCVQ628QK2xX8+PBnqJVvj0y7pug@mail.gmail.com>
Date: Tue, 4 Feb 2025 09:22:56 -0800
From: Doug Anderson <dianders@...omium.org>
To: Ricardo Ribalda <ribalda@...omium.org>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>, 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,

On Wed, Jan 29, 2025 at 4:40 AM Ricardo Ribalda <ribalda@...omium.org> 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;

FWIW, since you're specifically considering the -EPROBE_DEFER case,
it's probably worthwhile to make sure that dev_err_probe() is called.
That eventually calls device_set_deferred_probe_reason() which can be
helpful for tracking down problems.

It looks like uvc_gpio_parse() already calls this if gpiod_to_irq()
returns an error code probably you also want to make sure that
dev_err_probe() also gets called in the case where
devm_gpiod_get_optional() returns -EPROBE_DEFER. In that case,
potentially one could also get rid of the uvc_dbg() above.

In any case, IMO even without those changes your patch is still worth
landing. ...and maybe my suggestion should be in a separate follow-on
patch anyway. Thus:

Reviewed-by: Douglas Anderson <dianders@...omium.org>