Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The return value of `request_mem_region` for `cmap_regs_phys` is ignored, and the error path unconditionally calls `release_mem_region` on it. -- --- Patch [1]: [PATCH] fbdev: platinumfb: add error checking for ioremap calls --- commit 71a977c44688a9c49d5d28cb513b56fa5a810a1d Author: yuebingkun <[email protected]> fbdev: platinumfb: add error checking for ioremap calls This commit adds error checking for ioremap() and ioremap_wt() calls in platinumfb_probe() to prevent NULL pointer dereferences. It also transitions the function to use goto-based cleanup for handling initialization failures. > diff --git a/drivers/video/fbdev/platinumfb.c > b/drivers/video/fbdev/platinumfb.c > index a08d955d9b439..f74c937c839f8 100644 > --- a/drivers/video/fbdev/platinumfb.c > +++ b/drivers/video/fbdev/platinumfb.c [ ... ] > @@ -623,13 +638,22 @@ static int platinumfb_probe(struct platform_device* > odev) > dev_set_drvdata(&odev->dev, info); > > rc = platinum_init_fb(info); > - if (rc != 0) { > - iounmap(pinfo->frame_buffer); > - iounmap(pinfo->platinum_regs); > - iounmap(pinfo->cmap_regs); > - framebuffer_release(info); > - } > + if (rc != 0) > + goto err_unmap_cmap; > + > + return 0; > > +err_unmap_cmap: > + iounmap(pinfo->cmap_regs); > +err_release_cmap: > + release_mem_region(pinfo->cmap_regs_phys, 0x1000); [Severity: High] This is a pre-existing issue, but by adding this new error path, the code propagates an unsafe assumption. Earlier in platinumfb_probe(), the return value of request_mem_region() is ignored: request_mem_region(pinfo->cmap_regs_phys, 0x1000, "platinumfb cmap"); pinfo->cmap_regs = ioremap(pinfo->cmap_regs_phys, 0x1000); If request_mem_region() fails (for instance, if the region is already claimed), execution continues. If a subsequent step like ioremap() or platinum_init_fb() then fails, the code will jump to this newly added err_release_cmap label and unconditionally release the region. Because __release_region() searches by address and doesn't verify the caller, could this forcefully free an I/O memory region that actually belongs to another driver, potentially leading to resource tree corruption or a use-after-free? > +err_unmap_regs: > + iounmap(pinfo->platinum_regs); > +err_unmap_fb: > + iounmap(pinfo->frame_buffer); [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
