Thank you for your contribution! Sashiko AI review found 3 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Uncanceled timer in the probe error path leads to a Use-After-Free 
when the timer fires.
- [High] Uncanceled delayed work in the probe error path and early IRQ request 
lead to Use-After-Free.
- [High] The return value of extcon_rtk_type_c_edev_register() is assigned but 
never checked, leading to silent failures.
--

commit fd9fdd473dd5d54a8f94bae5e74133accbec590a
Author: Arnd Bergmann <[email protected]>

gpiolib: introduce gpio_name() helper

Most remaining users of desc_to_gpio() only call it for printing debug
information.

Replace this with a new gpiod_name() helper that returns the
gpio_desc->name string after checking the gpio_desc pointer.

> diff --git a/drivers/extcon/extcon-rtk-type-c.c 
> b/drivers/extcon/extcon-rtk-type-c.c
> index 82b60b927e41a..fb57e9d7ddb63 100644
> --- a/drivers/extcon/extcon-rtk-type-c.c
> +++ b/drivers/extcon/extcon-rtk-type-c.c
> @@ -1356,8 +1356,8 @@ static int extcon_rtk_type_c_probe(struct 
> platform_device *pdev)
>                               (int)PTR_ERR(gpio));
>               } else {
>                       type_c->rd_ctrl_gpio_desc = gpio;
> -                     dev_dbg(dev, "%s get rd-ctrl-gpios (id=%d) OK\n",
> -                             __func__, desc_to_gpio(gpio));
> +                     dev_dbg(dev, "%s get rd-ctrl-gpios (id=%s) OK\n",
> +                             __func__, gpiod_name(gpio));
>               }
>       }

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free if
extcon_rtk_type_c_init() fails.

In extcon_rtk_type_c_probe(), the code requests an IRQ before the delayed
work is initialized:

        ret = devm_request_irq(dev, type_c->irq, type_c_detect_irq, ...);
        ...
        INIT_DELAYED_WORK(&type_c->delayed_work, host_device_switch);

        ret = extcon_rtk_type_c_init(type_c);
        if (ret) {
                dev_err(dev, "%s failed to init type_c\n", __func__);
                goto err;
        }

Could an early interrupt schedule the uninitialized work?

Also, if extcon_rtk_type_c_init() fails, the code jumps to the err label
and returns without canceling the delayed work. Since the structure is
freed by devres, will this cause a use-after-free when the work executes?

[Severity: High]
This is also a pre-existing issue, but further down in the same function,
the return value of extcon_rtk_type_c_edev_register() is assigned to ret
but is not checked:

        ret = extcon_rtk_type_c_edev_register(type_c);

        create_debug_files(type_c);

        return 0;

Could this mask a registration failure, since the function unconditionally
returns 0?

[ ... ]

> diff --git a/drivers/usb/gadget/udc/at91_udc.c 
> b/drivers/usb/gadget/udc/at91_udc.c
> index 5aa360ba4f039..0993136043870 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -1896,8 +1896,8 @@ static int at91udc_probe(struct platform_device *pdev)
>                                       gpiod_to_irq(udc->board.vbus_pin),
>                                       at91_vbus_irq, 0, driver_name, udc);
>                       if (retval) {
> -                             DBG("request vbus irq %d failed\n",
> -                                 desc_to_gpio(udc->board.vbus_pin));
> +                             DBG("request vbus irq %s failed\n",
> +                                 gpiod_name(udc->board.vbus_pin));
>                               goto err_unprepare_iclk;
>                       }
>               }

[Severity: High]
This is a pre-existing issue, but there appears to be a missing timer cleanup
in the probe error path.

In at91udc_probe(), when vbus_polled is true, a timer is armed:

                if (udc->board.vbus_polled) {
                        INIT_WORK(&udc->vbus_timer_work, at91_vbus_timer_work);
                        timer_setup(&udc->vbus_timer, at91_vbus_timer, 0);
                        mod_timer(&udc->vbus_timer,
                                  jiffies + VBUS_POLL_TIMEOUT);
                }

If usb_add_gadget_udc() fails later, the code jumps to err_unprepare_iclk
and returns without deleting the timer:

        retval = usb_add_gadget_udc(dev, &udc->gadget);
        if (retval)
                goto err_unprepare_iclk;
        ...
err_unprepare_iclk:
        clk_unprepare(udc->iclk);
err_unprepare_fclk:
        clk_unprepare(udc->fclk);

        return retval;

If the probe fails, devres will free the udc structure. Does this mean the
timer can still fire and access the freed udc memory, causing a
use-after-free?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to