Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Use-after-free of layer structures (containing drm_plane) during driver cleanup due to incorrect devm vs. drmm initialization order. - [High] Concurrent probe overwrites shared static global regmap_config, causing a data race. -- commit 3a0c473b709e63281231182c727f9e5d25ce8b6f Author: Mukesh Ojha <[email protected]> drm: logicvc: Use devm_of_reserved_mem_device_init() Switch to devm_of_reserved_mem_device_init() so the reserved memory region is released automatically on probe failure or device unbind. Replace manual cleanup labels with error_early. > diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c > b/drivers/gpu/drm/logicvc/logicvc_drm.c > index bbebf4fc7f51a..d6dbe52ff0a99 100644 > --- a/drivers/gpu/drm/logicvc/logicvc_drm.c > +++ b/drivers/gpu/drm/logicvc/logicvc_drm.c [ ... ] > @@ -327,14 +327,14 @@ static int logicvc_drm_probe(struct platform_device > *pdev) [ ... ] > base = devm_ioremap_resource(dev, &res); > if (IS_ERR(base)) { > dev_err(dev, "Failed to map I/O base\n"); > ret = PTR_ERR(base); > - goto error_reserved_mem; > + goto error_early; > } > > logicvc_drm_regmap_config.max_register = resource_size(&res) - [Severity: High] This isn't a bug introduced by this patch, but does this concurrent probe overwrite the shared static global logicvc_drm_regmap_config? Modifying a static global structure without locking can cause a data race if multiple devices probe concurrently (e.g., if multiple IP cores are defined in the device tree). Depending on the race window, one device instance might be initialized with the max_register value of another instance, leading to either restricted MMIO access or out-of-bounds MMIO access. > @@ -345,21 +345,21 @@ static int logicvc_drm_probe(struct platform_device > *pdev) [ ... ] > irq = platform_get_irq(pdev, 0); > if (irq < 0) { > ret = -ENODEV; > - goto error_reserved_mem; > + goto error_early; > } > > logicvc = devm_drm_dev_alloc(dev, &logicvc_drm_driver, > struct logicvc_drm, drm_dev); [Severity: High] This is a pre-existing issue, but can this devm_drm_dev_alloc() call lead to a use-after-free of layer structures during driver cleanup? This call establishes the point in the devres stack where drm_dev_put() will execute. Because logicvc_layer_init() allocates layer structures with devm_kzalloc() after devm_drm_dev_alloc() executes: drivers/gpu/drm/logicvc/logicvc_layer.c:logicvc_layer_init() { ... layer = devm_kzalloc(dev, sizeof(*layer), GFP_KERNEL); ... } devres will execute in LIFO order on unbind. This means devm_kfree() frees the layer memory before the DRM device's devres action calls drm_dev_put(). When the DRM device is finally destroyed, drmm actions execute (including drm_mode_config_cleanup()), which iterates over registered planes and calls their destroy hook (drm_plane_cleanup). Since the plane is embedded in the already-freed layer struct, this causes a use-after-free. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
