Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Critical] Type confusion and memory corruption in wled_remove() due to incorrect drvdata pointer. Pre-existing issues: - [High] Use-after-free on probe error path due to uncancelled delayed work. -- commit 925c19cfe92bc196afb0b7a2001d793450d1b94f Author: David Heidelberg <[email protected]> backlight: qcom-wled: Read back the programmed brightness at probe Since the previous change wled_probe() seeds wled->brightness from MOD_EN, but with default-brightness rather than the level the hardware is actually driving, so a backlight left lit by the bootloader still visibly steps on the first update. Add wled_read_brightness() to seed both wled->brightness and the initial backlight property. > diff --git a/drivers/video/backlight/qcom-wled.c > b/drivers/video/backlight/qcom-wled.c > index eb742a6598173..471a0cb2b81f6 100644 > --- a/drivers/video/backlight/qcom-wled.c > +++ b/drivers/video/backlight/qcom-wled.c [ ... ] > @@ -1757,7 +1809,7 @@ static int wled_probe(struct platform_device *pdev) > > memset(&props, 0, sizeof(struct backlight_properties)); > props.type = BACKLIGHT_RAW; > - props.brightness = val; > + props.brightness = wled->brightness ?: val; > props.max_brightness = wled->max_brightness; > bl = devm_backlight_device_register(&pdev->dev, wled->name, > &pdev->dev, wled, > &wled_ops, &props); > platform_set_drvdata(pdev, bl); [Severity: Critical] Does this code incorrectly set the platform drvdata to a backlight_device pointer instead of the expected wled structure pointer? Looking at wled_remove(), it retrieves the platform drvdata but dereferences it as a struct wled: static void wled_remove(struct platform_device *pdev) { struct wled *wled = platform_get_drvdata(pdev); mutex_destroy(&wled->lock); cancel_delayed_work_sync(&wled->ovp_work); ... } Because the memory layout does not match, can this cause type confusion, destruction of uninitialized or unrelated mutexes, and guaranteed memory corruption or a kernel panic on removal? > return PTR_ERR_OR_ZERO(bl); [Severity: High] This is a pre-existing issue, but if devm_backlight_device_register() fails, this function returns an error and devm frees the wled structure without canceling any potentially scheduled delayed work. Is it possible for a spurious IRQ to trigger a use-after-free here? wled_probe() configures and implicitly enables the short IRQ earlier via devm_request_threaded_irq(). It also configures the OVP IRQ, which sets wled->ovp_irq > 0. If the short IRQ triggers during or just before this failure, wled_short_irq_handler() will execute and call wled_module_enable(wled, true). Because wled->ovp_irq > 0, wled_module_enable() schedules the delayed work: schedule_delayed_work(&wled->ovp_work, HZ / 100); Since the probe error path does not call cancel_delayed_work_sync(), this leaves the work on the queue. When the delayed work executes, it calls: static void wled_ovp_work(struct work_struct *work) { struct wled *wled = container_of(work, struct wled, ovp_work.work); enable_irq(wled->ovp_irq); } Can this access the already freed wled structure during the delayed work execution, resulting in a use-after-free? > }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
