Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] Missing cancel_work_sync() for ctx->reset_work during driver teardown leads to a Use-After-Free. - [High] Skipping enable_irq() on the error path causes an IRQ disable depth leak, leaving the interrupt permanently disabled. -- --- Patch [2]: [PATCH v3 2/3] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() --- commit 58246b5ef131d7aeea7b5c2b693088e7255aa435 Author: Esben Haabendal <[email protected]> drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work() This commit fixes the error path in sn65dsi83_reset_work() by adding a goto to ensure drm_bridge_exit() is called when sn65dsi83_reset_pipe() fails, safely closing the critical section. > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > @@ -419,11 +419,12 @@ static void sn65dsi83_reset_work(struct work_struct *ws) [Severity: Critical] This is a pre-existing issue, but could this work function execute after the device is unbound, resulting in a use-after-free? If the driver is removed and devres frees the ctx structure, the work could still be pending since sn65dsi83_remove() does not appear to call cancel_work_sync() before returning: sn65dsi83_remove() { ... drm_bridge_unplug(&ctx->bridge); // missing cancel_work_sync(&ctx->reset_work); } If the workqueue executes sn65dsi83_reset_work() after teardown, dereferencing ctx for drm_bridge_enter() might access freed memory. > ret = sn65dsi83_reset_pipe(ctx); > if (ret) { > dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret)); > - return; > + goto err_exit; [Severity: High] This isn't a bug introduced by this patch, but doesn't jumping to err_exit here leave the interrupt permanently disabled? If a hardware error triggers the IRQ handler, sn65dsi83_handle_errors() calls disable_irq_nosync(), which increments the IRQ disable depth. By skipping the enable_irq() call on this error path, the depth counter remains unbalanced. Would it be better to mask the interrupt source via hardware registers on failure, and still call enable_irq() so the kernel's depth counter remains balanced? > } > if (ctx->irq) > enable_irq(ctx->irq); > > +err_exit: > drm_bridge_exit(idx); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
