Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Unchecked request_mem_region combined with unconditional release_mem_region causes resource corruption and Use-After-Free. -- --- Patch [1]: [PATCH v2] fbdev: platinumfb: add error checking for ioremap calls --- commit a9dd2a2a25aeae119ea17d0f40c6cbdddde67dfd 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(), avoiding NULL pointer dereferences by introducing goto-based cleanup paths on failure. > 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 [ ... ] > @@ -567,15 +567,30 @@ static int platinumfb_probe(struct platform_device* > odev) [ ... ] > pinfo->cmap_regs_phys = 0xf301b000; /* XXX not in prom? */ > request_mem_region(pinfo->cmap_regs_phys, 0x1000, "platinumfb cmap"); > pinfo->cmap_regs = ioremap(pinfo->cmap_regs_phys, 0x1000); > + if (!pinfo->cmap_regs) { > + dev_err(&odev->dev, "failed to ioremap cmap registers\n"); > + rc = -ENOMEM; > + goto err_release_cmap; > + } [ ... ] > +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 I noticed the return value of request_mem_region() is ignored just before the ioremap() call. If request_mem_region() fails because the physical resource is already claimed by platform code or another driver, probe continues. If a subsequent step then fails, such as ioremap() returning NULL or platinum_init_fb() failing, this newly added error path branches to err_release_cmap. Since err_release_cmap unconditionally calls release_mem_region(), does this risk freeing a resource that wasn't actually acquired? If the region size and start address exactly match an existing claim, this could unlink and free a resource owned by another driver, potentially causing a use-after-free later. Could we add a check for the return value of request_mem_region() so we only release it if the driver successfully claimed it? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
