Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Low] The `isr_tables_copy` field is no longer used but was not removed from `struct dsi_data`, leaving unused memory. Pre-existing issues: - [Critical] The exact same Use-After-Free (UAF) vulnerability exists in the legacy fbdev OMAP DSI driver but was not patched. -- commit 18bc0b56dc801c9ead9c7b993f8c1e1f228ff1c0 Author: Andreas Kemnade <[email protected]> Subject: drm/omap: dsi: do not copy isr table This commit removes the copying of the ISR table in the omapdrm DSI IRQ handler. This change prevents a use-after-free scenario when stack-allocated objects are passed to unregistered ISRs. > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c > b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 27fe7bca9e2cf..70cfb779d6ae5 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -455,15 +455,10 @@ static irqreturn_t omap_dsi_irq_handler(int irq, void > *arg) > timer_delete(&dsi->te_timer); > #endif > > - /* make a copy and unlock, so that isrs can unregister > - * themselves */ > - memcpy(&dsi->isr_tables_copy, &dsi->isr_tables, > - sizeof(dsi->isr_tables)); > + dsi_handle_isrs(&dsi->isr_tables, irqstatus, vcstatus, ciostatus); > > spin_unlock(&dsi->irq_lock); > > - dsi_handle_isrs(&dsi->isr_tables_copy, irqstatus, vcstatus, ciostatus); > - [Severity: Low] Since isr_tables_copy is no longer used here, should it also be removed from struct dsi_data in drivers/gpu/drm/omapdrm/dss/dsi.h to avoid leaving unused memory? drivers/gpu/drm/omapdrm/dss/dsi.h: struct dsi_data { ... struct dsi_isr_tables isr_tables; /* space for a copy used by the interrupt handler */ struct dsi_isr_tables isr_tables_copy; ... }; [Severity: Critical] This isn't a bug introduced by this patch, but the exact same issue seems to still exist in the legacy omapfb driver. Looking at omap_dsi_irq_handler() in drivers/video/fbdev/omap2/omapfb/dss/dsi.c: /* make a copy and unlock, so that isrs can unregister * themselves */ memcpy(&dsi->isr_tables_copy, &dsi->isr_tables, sizeof(dsi->isr_tables)); spin_unlock(&dsi->irq_lock); dsi_handle_isrs(&dsi->isr_tables_copy, irqstatus, vcstatus, ciostatus); If a hardware timeout occurs during operations like dsi_vc_send_bta_sync(), causing wait_for_completion_timeout() to return 0, could the waiting thread destroy its local stack frame while the IRQ handler continues using the copy? Should a similar fix be applied to the fbdev driver as well? > dsi_handle_irq_errors(dsi, irqstatus, vcstatus, ciostatus); > > dsi_collect_irq_stats(dsi, irqstatus, vcstatus, ciostatus); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
