Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues: - [High] Calling drmm_* initializations inside the component bind callback leads to memory leaks and DRM structure corruption if bind fails and is retried. - [High] Calling dma_alloc_coherent() with GFP_KERNEL in an atomic context. - [High] Missing NULL check for the return value of dma_alloc_coherent(). - [High] adp_drm_gem_dumb_create modifies height after validation, causing a 32-bit integer overflow and undersized buffer allocation. -- commit 8a1bec26bfe0bb63ece93f656fc4dc72b509f897 Author: Myeonghun Pak <[email protected]> drm/adp: Free FE IRQ on bind failure adp_drm_bind() requests the front-end IRQ before registering the DRM device. If drm_dev_register() fails, the function returns immediately and leaves the IRQ handler registered. > diff --git a/drivers/gpu/drm/adp/adp_drv.c b/drivers/gpu/drm/adp/adp_drv.c > index ce9dd6222634d..b7fc456043f34 100644 > --- a/drivers/gpu/drm/adp/adp_drv.c > +++ b/drivers/gpu/drm/adp/adp_drv.c > @@ -533,9 +533,13 @@ static int adp_drm_bind(struct device *dev) > > err = drm_dev_register(&adp->drm, 0); > if (err) > - return err; > + goto err_free_irq; > > return 0; > + > +err_free_irq: > + free_irq(adp->fe_irq, adp); > + return err; > } [Severity: High] This is a pre-existing issue, but does calling drmm_ initializations inside the component bind callback lead to memory leaks if bind fails here? The drm_device is allocated in probe with devm_, so it persists across bind retries: drivers/gpu/drm/adp/adp_drv.c:adp_probe() { ... adp = devm_drm_dev_alloc(&pdev->dev, &adp_driver, struct adp_drv_private, drm); ... } But adp_drm_bind() calls adp_setup_mode_config(), which initializes drmm_ resources: drivers/gpu/drm/adp/adp_drv.c:adp_drm_bind() { ... err = adp_setup_mode_config(adp); ... } If a later step in bind fails (such as drm_dev_register()) and bind is retried, will re-running drmm_mode_config_init() on the same drm_device re-initialize dev->mode_config mutexes and IDRs without destroying the old ones, leading to memory leaks and corruption? [Severity: High] This is a pre-existing issue, but is it safe to use GFP_KERNEL in the atomic commit tail path? drivers/gpu/drm/adp/adp_drv.c:adp_crtc_atomic_flush() { ... if (new_size != 0) { adp->mask_buf = dma_alloc_coherent(crtc->dev->dev, new_size, &adp->mask_iova, GFP_KERNEL); memset(adp->mask_buf, 0xFF, new_size); writel(adp->mask_iova, adp->be + ADBE_MASK_BUF); } ... } Since adp_crtc_atomic_flush() is part of the atomic commit pipeline, it runs in an atomic context where sleeping is forbidden. Does calling dma_alloc_coherent() with GFP_KERNEL here allow the function to sleep, causing kernel warnings or deadlocks? [Severity: High] This is a pre-existing issue, but is there a missing NULL check for the return value of dma_alloc_coherent() here? drivers/gpu/drm/adp/adp_drv.c:adp_crtc_atomic_flush() { ... adp->mask_buf = dma_alloc_coherent(crtc->dev->dev, new_size, &adp->mask_iova, GFP_KERNEL); memset(adp->mask_buf, 0xFF, new_size); ... } If memory is exhausted and dma_alloc_coherent() returns NULL, will the immediate memset() cause a NULL pointer dereference? [Severity: High] This is a pre-existing issue, but does aligning args->height after validation cause an integer overflow? drivers/gpu/drm/adp/adp_drv.c:adp_drm_gem_dumb_create() { ... args->height = ALIGN(args->height, 64); args->size = args->pitch * args->height; return drm_gem_dma_dumb_create_internal(file_priv, drm, args); } If userspace requests dimensions that pass the core drm_mode_create_dumb() checks but overflow when args->height is increased, will the 32-bit multiplication in drm_gem_dma_dumb_create_internal() wrap around? Could this result in an undersized DMA buffer allocation that allows out-of-bounds memory accesses when userspace maps and writes to it? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
