* [PATCH v2 1/11] drm: logicvc: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:30 ` sashiko-bot
2026-09-02 20:16 ` [PATCH v2 2/11] drm: hdlcd: " Mukesh Ojha
` (9 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Switch to devm_of_reserved_mem_device_init() so the reserved memory
region is released automatically on probe failure or device unbind.
Replace all error paths that jumped to error_reserved_mem: with
error_early: since the manual cleanup label is no longer needed, and
remove the explicit of_reserved_mem_device_release() call in the remove
function.
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Acked-by: Paul Kocialkowski <paulk@sys-base.io>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/logicvc/logicvc_drm.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c b/drivers/gpu/drm/logicvc/logicvc_drm.c
index bbebf4fc7f51..d6dbe52ff0a9 100644
--- a/drivers/gpu/drm/logicvc/logicvc_drm.c
+++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
@@ -306,7 +306,7 @@ static int logicvc_drm_probe(struct platform_device *pdev)
int irq;
int ret;
- ret = of_reserved_mem_device_init(dev);
+ ret = devm_of_reserved_mem_device_init(dev);
if (ret && ret != -ENODEV) {
dev_err(dev, "Failed to init memory region\n");
goto error_early;
@@ -327,14 +327,14 @@ static int logicvc_drm_probe(struct platform_device *pdev)
ret = of_address_to_resource(of_node, 0, &res);
if (ret) {
dev_err(dev, "Failed to get resource from address\n");
- goto error_reserved_mem;
+ goto error_early;
}
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) -
@@ -345,21 +345,21 @@ static int logicvc_drm_probe(struct platform_device *pdev)
if (IS_ERR(regmap)) {
dev_err(dev, "Failed to create regmap for I/O\n");
ret = PTR_ERR(regmap);
- goto error_reserved_mem;
+ goto error_early;
}
}
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);
if (IS_ERR(logicvc)) {
ret = PTR_ERR(logicvc);
- goto error_reserved_mem;
+ goto error_early;
}
platform_set_drvdata(pdev, logicvc);
@@ -371,7 +371,7 @@ static int logicvc_drm_probe(struct platform_device *pdev)
caps = logicvc_drm_caps_match(logicvc);
if (!caps) {
ret = -EINVAL;
- goto error_reserved_mem;
+ goto error_early;
}
logicvc->caps = caps;
@@ -382,7 +382,7 @@ static int logicvc_drm_probe(struct platform_device *pdev)
ret = logicvc_clocks_prepare(logicvc);
if (ret) {
drm_err(drm_dev, "Failed to prepare clocks\n");
- goto error_reserved_mem;
+ goto error_early;
}
ret = devm_request_irq(dev, irq, logicvc_drm_irq_handler, 0,
@@ -450,9 +450,6 @@ static int logicvc_drm_probe(struct platform_device *pdev)
error_clocks:
logicvc_clocks_unprepare(logicvc);
-error_reserved_mem:
- of_reserved_mem_device_release(dev);
-
error_early:
return ret;
}
@@ -469,8 +466,6 @@ static void logicvc_drm_remove(struct platform_device *pdev)
logicvc_mode_fini(logicvc);
logicvc_clocks_unprepare(logicvc);
-
- of_reserved_mem_device_release(dev);
}
static void logicvc_drm_shutdown(struct platform_device *pdev)
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 1/11] drm: logicvc: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 ` [PATCH v2 1/11] drm: logicvc: " Mukesh Ojha
@ 2026-09-02 20:30 ` sashiko-bot
0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-09-02 20:30 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: dri-devel, linux-sunxi
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 <mukesh.ojha@oss.qualcomm.com>
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/20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/11] drm: hdlcd: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 1/11] drm: logicvc: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 3/11] drm: pl111: " Mukesh Ojha
` (8 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Switch to devm_of_reserved_mem_device_init() so the reserved memory
region is released automatically on probe failure or device unbind.
Remove the three explicit of_reserved_mem_device_release() calls in
the setup_fail error path, the bind error path, and hdlcd_drm_unbind().
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/arm/hdlcd_drv.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 81d45f2dd6a7..67466fd441d5 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -134,7 +134,7 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
version & HDLCD_VERSION_MINOR_MASK);
/* Get the optional framebuffer memory resource */
- ret = of_reserved_mem_device_init(drm->dev);
+ ret = devm_of_reserved_mem_device_init(drm->dev);
if (ret && ret != -ENODEV)
return ret;
@@ -164,8 +164,6 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
irq_fail:
drm_crtc_cleanup(&hdlcd->crtc);
setup_fail:
- of_reserved_mem_device_release(drm->dev);
-
return ret;
}
@@ -316,7 +314,6 @@ static int hdlcd_drm_bind(struct device *dev)
of_node_put(hdlcd->crtc.port);
hdlcd->crtc.port = NULL;
hdlcd_irq_uninstall(hdlcd);
- of_reserved_mem_device_release(drm->dev);
err_free:
dev_set_drvdata(dev, NULL);
return ret;
@@ -338,7 +335,6 @@ static void hdlcd_drm_unbind(struct device *dev)
pm_runtime_put(dev);
if (pm_runtime_enabled(dev))
pm_runtime_disable(dev);
- of_reserved_mem_device_release(dev);
dev_set_drvdata(dev, NULL);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 3/11] drm: pl111: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 1/11] drm: logicvc: " Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 2/11] drm: hdlcd: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:33 ` sashiko-bot
2026-09-02 20:16 ` [PATCH v2 4/11] drm: komeda: " Mukesh Ojha
` (7 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Switch to devm_of_reserved_mem_device_init() so the reserved memory
region is released automatically on probe failure or device removal.
Remove the explicit of_reserved_mem_device_release() calls in the
dev_put error path and pl111_amba_remove().
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/pl111/pl111_drv.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 8ec659b3c08e..a07b17261961 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -255,7 +255,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
drm->dev_private = priv;
priv->variant = variant;
- ret = of_reserved_mem_device_init(dev);
+ ret = devm_of_reserved_mem_device_init(dev);
if (!ret) {
drm_info(drm, "using device-specific reserved memory\n");
priv->use_device_memory = true;
@@ -314,7 +314,6 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
dev_put:
drm_dev_put(drm);
- of_reserved_mem_device_release(dev);
return ret;
}
@@ -330,7 +329,6 @@ static void pl111_amba_remove(struct amba_device *amba_dev)
if (priv->panel)
drm_panel_bridge_remove(priv->bridge);
drm_dev_put(drm);
- of_reserved_mem_device_release(dev);
}
static void pl111_amba_shutdown(struct amba_device *amba_dev)
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 3/11] drm: pl111: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 ` [PATCH v2 3/11] drm: pl111: " Mukesh Ojha
@ 2026-09-02 20:33 ` sashiko-bot
0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-09-02 20:33 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: linux-sunxi, dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Pre-existing use-after-free and memory corruption risks due to tying resource lifetimes (`priv` structure and reserved memory) to `devres` (driver unbind) while the `drm_device` can outlive the unbind.
- [High] The `drm_panel_bridge` allocated via `drm_panel_bridge_add_typed()` in `pl111_modeset_init()` is leaked if `drm_dev_register()` fails.
- [High] When `INTEGRATOR_CLCD_CM` is matched, `pl111_versatile_init()` overwrites `np` without dropping the original reference. If the second match fails, probe incorrectly fails.
--
commit 76a1e18802677a9e3db4c051729d253910d2a13c
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
drm: pl111: Use devm_of_reserved_mem_device_init()
This patch converts the driver to use devm_of_reserved_mem_device_init,
allowing the reserved memory region to be released automatically on
probe failure or device removal. It removes the explicit cleanup calls.
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index 8ec659b3c08eb..a07b17261961c 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -255,7 +255,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
> drm->dev_private = priv;
> priv->variant = variant;
>
> - ret = of_reserved_mem_device_init(dev);
> + ret = devm_of_reserved_mem_device_init(dev);
[Severity: High]
This is a pre-existing issue, but does tying resource lifetimes to devres
here expose a use-after-free risk?
The priv structure is allocated via devm_kzalloc() and the reserved
memory is now managed via devres, tying their lifetimes to the driver
unbind lifecycle. However, the drm_device is allocated without devres via
drm_dev_alloc() and tied to a refcount.
If a local user holds an open DRM file descriptor (like /dev/dri/card0)
and the driver is unbound via sysfs, devres will free priv and the reserved
memory while the DRM device persists. Because pl111_amba_remove() calls
drm_dev_unregister() instead of drm_dev_unplug(), the device is not
marked as unplugged and subsequent ioctls are not blocked.
An ioctl like DRM_IOCTL_PRIME_FD_TO_HANDLE will call
pl111_gem_import_sg_table(), which accesses dev->dev_private (priv),
potentially causing a use-after-free. Should the driver be migrated to
drm_dev_unplug() and devm_drm_dev_alloc() to prevent this?
[ ... ]
> @@ -314,7 +314,6 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
>
> dev_put:
> drm_dev_put(drm);
> - of_reserved_mem_device_release(dev);
>
> return ret;
> }
[Severity: High]
This is a pre-existing issue, but does this error path leak the panel
bridge?
During probe, a bridge is created by calling drm_panel_bridge_add_typed()
inside pl111_modeset_init(), which adds the bridge to the global DRM bridge
list with a reference count of 2.
If drm_dev_register() subsequently fails, the probe function jumps to
the dev_put label. The error path calls drm_dev_put(drm) but fails to
call drm_panel_bridge_remove(priv->bridge). When the panel device is
eventually unbound, its devres cleanup drops one reference on the bridge,
but the bridge is never removed from the global list, leaving a dangling
pointer. Does this need an explicit drm_panel_bridge_remove() call here?
Also, while reviewing the probe path, I noticed a potential node reference
issue in pl111_versatile_init() called earlier in this function:
drivers/gpu/drm/pl111/pl111_versatile.c:pl111_versatile_init() {
...
np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
&clcd_id);
...
if (clcd_id == INTEGRATOR_CLCD_CM) {
np = of_find_matching_node_and_match(NULL, impd1_clcd_of_match,
&clcd_id);
}
[Severity: High]
This is a pre-existing issue, but does overwriting np without dropping the
original reference cause a memory leak?
For the INTEGRATOR_CLCD_CM type, a second call to
of_find_matching_node_and_match() is made, and its result overwrites np
without calling of_node_put() on the original node. If the IM-PD1 syscon
is not found, the second call returns NULL. The probe then calls
syscon_node_to_regmap(NULL), which fails and returns an error, preventing
the driver from probing successfully on valid standard Integrator AP
hardware. Should of_node_put() be called before reassigning np?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/11] drm: komeda: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (2 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 3/11] drm: pl111: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 5/11] drm: malidp: " Mukesh Ojha
` (6 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index 9aad1d1d28ec..8016f5a68cba 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -145,7 +145,7 @@ static int komeda_parse_dt(struct device *dev, struct komeda_dev *mdev)
}
/* Get the optional framebuffer memory resource */
- ret = of_reserved_mem_device_init(dev);
+ ret = devm_of_reserved_mem_device_init(dev);
if (ret && ret != -ENODEV)
return ret;
@@ -292,8 +292,6 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
mdev->n_pipelines = 0;
- of_reserved_mem_device_release(dev);
-
if (funcs && funcs->cleanup)
funcs->cleanup(mdev);
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 5/11] drm: malidp: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (3 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 4/11] drm: komeda: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:35 ` sashiko-bot
2026-09-02 20:16 ` [PATCH v2 6/11] drm: ingenic: " Mukesh Ojha
` (5 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/arm/malidp_drv.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 23fa942ae4bb..3e17513c3114 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -758,7 +758,7 @@ static int malidp_bind(struct device *dev)
return PTR_ERR(hwdev->pxlclk);
/* Get the optional framebuffer memory resource */
- ret = of_reserved_mem_device_init(dev);
+ ret = devm_of_reserved_mem_device_init(dev);
if (ret && ret != -ENODEV)
return ret;
@@ -887,7 +887,6 @@ static int malidp_bind(struct device *dev)
else
malidp_runtime_pm_suspend(dev);
dev_set_drvdata(dev, NULL);
- of_reserved_mem_device_release(dev);
return ret;
}
@@ -913,7 +912,6 @@ static void malidp_unbind(struct device *dev)
else
malidp_runtime_pm_suspend(dev);
dev_set_drvdata(dev, NULL);
- of_reserved_mem_device_release(dev);
}
static const struct component_master_ops malidp_master_ops = {
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 5/11] drm: malidp: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 ` [PATCH v2 5/11] drm: malidp: " Mukesh Ojha
@ 2026-09-02 20:35 ` sashiko-bot
0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-09-02 20:35 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: dri-devel, linux-sunxi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Permanent CMA capacity leak due to clearing dev->cma_area before GEM objects are freed during device unbind.
--
commit a1f947cbe50ede0e9b650dcb9e31cc36cb85952d
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
drm: malidp: Use devm_of_reserved_mem_device_init()
This commit simplifies the driver by transitioning to the devres-managed
devm_of_reserved_mem_device_init(), removing manual cleanup calls during
bind failures and unbind.
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 23fa942ae4bbe..3e17513c31147 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -758,7 +758,7 @@ static int malidp_bind(struct device *dev)
> return PTR_ERR(hwdev->pxlclk);
>
> /* Get the optional framebuffer memory resource */
> - ret = of_reserved_mem_device_init(dev);
> + ret = devm_of_reserved_mem_device_init(dev);
[Severity: High]
This isn't a bug introduced by this patch, but does this devres conversion
preserve a pre-existing CMA memory capacity leak during device unbind?
In malidp_bind(), devm_drm_dev_alloc() is called before this initialization
function. Because devres actions execute in reverse order of allocation, the
cleanup action registered here will execute before the drm_dev_put() action.
When malidp_unbind() returns, the component framework immediately releases
the component's devres group, clearing dev->cma_area. Later, when the DRM
device is freed (either immediately or when userspace eventually closes its
file descriptors), drm_gem_object_free() calls dma_free_contiguous().
Because dev->cma_area is already NULL at that point, it bypasses
cma_release() and falls back to the buddy allocator, permanently leaking
the CMA region's bitmap capacity.
The previous code suffered from this exact same ordering issue since the
manual of_reserved_mem_device_release() call happened before drm_dev_put()
ran. Is there a way to adjust the cleanup order so the CMA area outlives
the GEM objects?
> if (ret && ret != -ENODEV)
> return ret;
>
[ ... ]
> @@ -913,7 +912,6 @@ static void malidp_unbind(struct device *dev)
> else
> malidp_runtime_pm_suspend(dev);
> dev_set_drvdata(dev, NULL);
> - of_reserved_mem_device_release(dev);
> }
>
> static const struct component_master_ops malidp_master_ops = {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 6/11] drm: ingenic: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (4 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 5/11] drm: malidp: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 7/11] drm: kmb: " Mukesh Ojha
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Replace the hand-rolled devm wrapper (ingenic_drm_release_rmem +
devm_add_action_or_reset) with the standard
devm_of_reserved_mem_device_init(), letting the device resource
manager handle cleanup automatically.
Reviewed-by: Paul Cercueil <paul@crapouillou.net>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 738a80b2550f..2c4dd6ed2d41 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -1060,11 +1060,6 @@ static void ingenic_drm_unbind_all(void *d)
component_unbind_all(priv->dev, &priv->drm);
}
-static void __maybe_unused ingenic_drm_release_rmem(void *d)
-{
- of_reserved_mem_device_release(d);
-}
-
static void ingenic_drm_configure_hwdesc(struct ingenic_drm *priv,
unsigned int hwdesc,
unsigned int next_hwdesc, u32 id)
@@ -1127,16 +1122,10 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
}
if (IS_ENABLED(CONFIG_OF_RESERVED_MEM)) {
- ret = of_reserved_mem_device_init(dev);
+ ret = devm_of_reserved_mem_device_init(dev);
if (ret && ret != -ENODEV)
dev_warn(dev, "Failed to get reserved memory: %d\n", ret);
-
- if (!ret) {
- ret = devm_add_action_or_reset(dev, ingenic_drm_release_rmem, dev);
- if (ret)
- return ret;
- }
}
priv = devm_drm_dev_alloc(dev, &ingenic_drm_driver_data,
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 7/11] drm: kmb: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (5 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 6/11] drm: ingenic: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:31 ` sashiko-bot
2026-09-02 20:16 ` [PATCH v2 8/11] drm: sun4i: " Mukesh Ojha
` (3 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.
Also simplify the IRQ error path since the release before init was a no-op.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/kmb/kmb_drv.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index 7c2eb1152fc2..88fe377940f3 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -140,13 +140,12 @@ static int kmb_hw_init(struct drm_device *drm, unsigned long flags)
/* Allocate LCD interrupt resources */
irq_lcd = platform_get_irq(pdev, 0);
if (irq_lcd < 0) {
- ret = irq_lcd;
drm_err(&kmb->drm, "irq_lcd not found");
- goto setup_fail;
+ return irq_lcd;
}
/* Get the optional framebuffer memory resource */
- ret = of_reserved_mem_device_init(drm->dev);
+ ret = devm_of_reserved_mem_device_init(drm->dev);
if (ret && ret != -ENODEV)
return ret;
@@ -155,11 +154,6 @@ static int kmb_hw_init(struct drm_device *drm, unsigned long flags)
kmb->irq_lcd = irq_lcd;
return 0;
-
- setup_fail:
- of_reserved_mem_device_release(drm->dev);
-
- return ret;
}
static const struct drm_mode_config_funcs kmb_mode_config_funcs = {
@@ -465,8 +459,6 @@ static void kmb_remove(struct platform_device *pdev)
pm_runtime_put_sync(drm->dev);
pm_runtime_disable(drm->dev);
- of_reserved_mem_device_release(drm->dev);
-
/* Release clks */
kmb_display_clk_disable(kmb);
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 7/11] drm: kmb: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 ` [PATCH v2 7/11] drm: kmb: " Mukesh Ojha
@ 2026-09-02 20:31 ` sashiko-bot
0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-09-02 20:31 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: dri-devel, linux-sunxi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The return value of kmb_initialize_clocks() is ignored in kmb_hw_init(), leading to potential hardware hangs.
- [High] Calling drm_atomic_helper_shutdown() after disabling clocks and PM runtime leads to hardware access on a powered-down device and a system hang.
- [High] Dereferencing an ERR_PTR during error handling of DSI initialization causes a kernel panic.
--
commit ce6024d25a2085ae8c8a68d80d92293c0cb8ad15
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
drm: kmb: Use devm_of_reserved_mem_device_init()
This commit updates the driver to use the devres-managed variant of
of_reserved_mem_device_init(), simplifying the error and removal paths by
removing manual cleanup calls.
> diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
> index 7c2eb1152fc2f..88fe377940f30 100644
> --- a/drivers/gpu/drm/kmb/kmb_drv.c
> +++ b/drivers/gpu/drm/kmb/kmb_drv.c
> @@ -140,13 +140,12 @@ static int kmb_hw_init(struct drm_device *drm, unsigned long flags)
> /* Allocate LCD interrupt resources */
> irq_lcd = platform_get_irq(pdev, 0);
[Severity: High]
This is a pre-existing issue, but I noticed that just before this hunk in
kmb_hw_init(), the return value of kmb_initialize_clocks() is discarded:
/* Enable display clocks */
kmb_initialize_clocks(kmb, &pdev->dev);
/* Register irqs here - section 17.3 in databook
If kmb_initialize_clocks() fails, the driver proceeds with probe and will
eventually access unclocked MMIO registers during modeset. Can this lead to
synchronous external aborts and system hangs?
[ ... ]
> @@ -465,8 +459,6 @@ static void kmb_remove(struct platform_device *pdev)
> pm_runtime_put_sync(drm->dev);
> pm_runtime_disable(drm->dev);
>
> - of_reserved_mem_device_release(drm->dev);
> -
> /* Release clks */
> kmb_display_clk_disable(kmb);
>
[Severity: High]
This isn't a bug introduced by this patch, but does the shutdown sequence
in kmb_remove() cause hardware access on a powered-down device?
Right after this hunk, the function completes with:
/* Unregister DSI host */
kmb_dsi_host_unregister(kmb->kmb_dsi);
drm_atomic_helper_shutdown(drm);
}
Since pm_runtime_disable() and kmb_display_clk_disable() are called prior to
drm_atomic_helper_shutdown(), atomic_disable callbacks will read and write
hardware registers while clocks are disabled. Will this cause a bus fault or
system hang during driver unbind?
[Severity: High]
This is also a pre-existing issue, but outside the diff in kmb_probe(), does
the error handling for kmb_dsi_init() cause an invalid pointer dereference?
If kmb_dsi_init() fails, it returns an ERR_PTR:
kmb->kmb_dsi = kmb_dsi_init(dsi_pdev);
if (IS_ERR(kmb->kmb_dsi)) {
drm_err(&kmb->drm, "failed to initialize DSI\n");
ret = PTR_ERR(kmb->kmb_dsi);
goto err_free1;
}
The error handling path unconditionally passes this ERR_PTR to
kmb_dsi_host_unregister(), which passes it to kmb_dsi_clk_disable(). This
dereferences the pointer when attempting to access kmb_dsi->clk_mipi:
drivers/gpu/drm/kmb/kmb_dsi.c:kmb_dsi_clk_disable() {
clk_disable_unprepare(kmb_dsi->clk_mipi);
Will this result in a kernel panic due to dereferencing the ERR_PTR on memory
allocation failure during initialization?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com?part=7
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 8/11] drm: sun4i: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (6 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 7/11] drm: kmb: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 9/11] drm: xlnx: zynqmp_dpsub: " Mukesh Ojha
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/sun4i/sun4i_drv.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index e9baca360160..c24f96bdd135 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -80,7 +80,7 @@ static int sun4i_drv_bind(struct device *dev)
INIT_LIST_HEAD(&drv->engine_list);
INIT_LIST_HEAD(&drv->tcon_list);
- ret = of_reserved_mem_device_init(dev);
+ ret = devm_of_reserved_mem_device_init(dev);
if (ret && ret != -ENODEV) {
dev_err(drm->dev, "Couldn't claim our memory region\n");
goto free_drm;
@@ -125,7 +125,6 @@ static int sun4i_drv_bind(struct device *dev)
component_unbind_all(dev, NULL);
cleanup_mode_config:
drm_mode_config_cleanup(drm);
- of_reserved_mem_device_release(dev);
free_drm:
drm_dev_put(drm);
return ret;
@@ -142,7 +141,6 @@ static void sun4i_drv_unbind(struct device *dev)
drm_mode_config_cleanup(drm);
component_unbind_all(dev, NULL);
- of_reserved_mem_device_release(dev);
drm_dev_put(drm);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 9/11] drm: xlnx: zynqmp_dpsub: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (7 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 8/11] drm: sun4i: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:31 ` sashiko-bot
2026-09-02 20:55 ` Laurent Pinchart
2026-09-02 20:16 ` [PATCH v2 10/11] drm: aspeed: " Mukesh Ojha
2026-09-02 20:16 ` [PATCH v2 11/11] drm: arcpgu: " Mukesh Ojha
10 siblings, 2 replies; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha,
Radhey Shyam Pandey
Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
index 53ab1a2a5aaf..e93a7a299b52 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
@@ -203,7 +203,7 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
/* Try the reserved memory. Proceed if there's none. */
- of_reserved_mem_device_init(&pdev->dev);
+ devm_of_reserved_mem_device_init(&pdev->dev);
ret = zynqmp_dpsub_init_clocks(dpsub);
if (ret < 0)
@@ -255,7 +255,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
clk_disable_unprepare(dpsub->apb_clk);
err_mem:
- of_reserved_mem_device_release(&pdev->dev);
if (!dpsub->drm)
zynqmp_dpsub_release(dpsub);
return ret;
@@ -276,7 +275,6 @@ static void zynqmp_dpsub_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
clk_disable_unprepare(dpsub->apb_clk);
- of_reserved_mem_device_release(&pdev->dev);
if (!dpsub->drm)
zynqmp_dpsub_release(dpsub);
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 9/11] drm: xlnx: zynqmp_dpsub: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 ` [PATCH v2 9/11] drm: xlnx: zynqmp_dpsub: " Mukesh Ojha
@ 2026-09-02 20:31 ` sashiko-bot
2026-09-02 20:55 ` Laurent Pinchart
1 sibling, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-09-02 20:31 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: dri-devel, linux-sunxi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] CMA area is released while DRM GEM objects might still be alive, leading to memory corruption when freeing DMA memory.
- [High] The APB clock (`apb_clk`) prepare and enable counts are leaked on multiple error paths during probe and clock initialization.
- [High] The allocated `dpsub` structure is leaked if `dma_set_mask` fails.
--
commit 608bb00e624e20e20a7867fa3c7bb9e1c899263d
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
drm: xlnx: zynqmp_dpsub: Use devm_of_reserved_mem_device_init()
This commit replaces the manual of_reserved_mem_device_init and
of_reserved_mem_device_release pair with the devres-managed
devm_of_reserved_mem_device_init, letting the device resource manager
handle cleanup automatically.
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> index 53ab1a2a5aafa..e93a7a299b525 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> @@ -203,7 +203,7 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
> dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>
> /* Try the reserved memory. Proceed if there's none. */
> - of_reserved_mem_device_init(&pdev->dev);
> + devm_of_reserved_mem_device_init(&pdev->dev);
[Severity: Critical]
This is a pre-existing issue, but does tying the CMA area release to the
platform device's devres cleanup happen too early for DRM GEM objects?
When a DRM device is unbound while userspace holds an open file descriptor,
the GEM objects can outlive the platform device. When userspace eventually
closes the file descriptor, drm_gem_dma_free() calls into the DMA API.
Because the CMA area is already cleaned up upon platform device removal,
can the DMA API fail to recognize the memory as CMA and incorrectly free it
into the buddy allocator, causing memory corruption?
[Severity: High]
This isn't a bug introduced by this patch, but looking at the earlier
sections of zynqmp_dpsub_probe(), we have this error path:
ret = dma_set_mask(dpsub->dev, DMA_BIT_MASK(ZYNQMP_DISP_MAX_DMA_BIT));
if (ret)
return ret;
Does this early return leak the dynamically allocated dpsub structure?
It returns directly instead of jumping to an error handling label where
zynqmp_dpsub_release() or a free operation would be called.
[ ... ]
> @@ -255,7 +255,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
> pm_runtime_disable(&pdev->dev);
> clk_disable_unprepare(dpsub->apb_clk);
> err_mem:
> - of_reserved_mem_device_release(&pdev->dev);
> if (!dpsub->drm)
> zynqmp_dpsub_release(dpsub);
> return ret;
[Severity: High]
This is a pre-existing issue, but does this error handling correctly clean up
the apb_clk resource?
If zynqmp_dpsub_parse_dt() fails earlier in the probe function, the code
jumps to the err_mem label. Jumping here skips the err_pm label where
clk_disable_unprepare() is called, which seems to leave the clock enabled
permanently.
Similarly, in zynqmp_dpsub_init_clocks(), if it successfully enables apb_clk
but then fails to acquire vid_clk with -EPROBE_DEFER:
else if (PTR_ERR(dpsub->vid_clk) == -EPROBE_DEFER)
return PTR_ERR(dpsub->vid_clk);
Does this return the error immediately without unwinding and disabling the
clock?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com?part=9
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v2 9/11] drm: xlnx: zynqmp_dpsub: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 ` [PATCH v2 9/11] drm: xlnx: zynqmp_dpsub: " Mukesh Ojha
2026-09-02 20:31 ` sashiko-bot
@ 2026-09-02 20:55 ` Laurent Pinchart
1 sibling, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-09-02 20:55 UTC (permalink / raw)
To: Mukesh Ojha
Cc: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Tomi Valkeinen, Michal Simek, Ryan Chen,
Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Radhey Shyam Pandey
On Thu, Sep 03, 2026 at 01:46:38AM +0530, Mukesh Ojha wrote:
> Use the devres-managed devm_of_reserved_mem_device_init() instead of
> the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
> pair, letting the device resource manager handle cleanup automatically.
>
> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> index 53ab1a2a5aaf..e93a7a299b52 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> @@ -203,7 +203,7 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
> dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>
> /* Try the reserved memory. Proceed if there's none. */
> - of_reserved_mem_device_init(&pdev->dev);
> + devm_of_reserved_mem_device_init(&pdev->dev);
>
> ret = zynqmp_dpsub_init_clocks(dpsub);
> if (ret < 0)
> @@ -255,7 +255,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
> pm_runtime_disable(&pdev->dev);
> clk_disable_unprepare(dpsub->apb_clk);
> err_mem:
> - of_reserved_mem_device_release(&pdev->dev);
> if (!dpsub->drm)
> zynqmp_dpsub_release(dpsub);
> return ret;
> @@ -276,7 +275,6 @@ static void zynqmp_dpsub_remove(struct platform_device *pdev)
>
> pm_runtime_disable(&pdev->dev);
> clk_disable_unprepare(dpsub->apb_clk);
> - of_reserved_mem_device_release(&pdev->dev);
>
> if (!dpsub->drm)
> zynqmp_dpsub_release(dpsub);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 10/11] drm: aspeed: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (8 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 9/11] drm: xlnx: zynqmp_dpsub: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
2026-09-02 20:35 ` sashiko-bot
2026-09-02 20:16 ` [PATCH v2 11/11] drm: arcpgu: " Mukesh Ojha
10 siblings, 1 reply; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Use the devres-managed devm_of_reserved_mem_device_init() to ensure
the reserved memory region is released on device removal, fixing a
missing cleanup in the original code.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
index d4577663a1f0..2c7e54fc35d9 100644
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
+++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
@@ -168,7 +168,7 @@ static int aspeed_gfx_load(struct drm_device *drm)
}
}
- ret = of_reserved_mem_device_init(drm->dev);
+ ret = devm_of_reserved_mem_device_init(drm->dev);
if (ret) {
dev_err(&pdev->dev,
"failed to initialize reserved mem: %d\n", ret);
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 10/11] drm: aspeed: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 ` [PATCH v2 10/11] drm: aspeed: " Mukesh Ojha
@ 2026-09-02 20:35 ` sashiko-bot
0 siblings, 0 replies; 19+ messages in thread
From: sashiko-bot @ 2026-09-02 20:35 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: linux-sunxi, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using devm_of_reserved_mem_device_init ties the reserved memory release to the physical device unbind, which executes before DRM GEM objects are freed. This leads to a kernel panic in dma_free_wc() during driver unload or when userspace closes the device.
--
commit af4228b344b701076f4bd88aa57c810ff4e26c48
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
drm: aspeed: Use devm_of_reserved_mem_device_init()
This commit replaces of_reserved_mem_device_init() with the devres-managed
devm_of_reserved_mem_device_init() to ensure the reserved memory region is
released on device removal.
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> index d4577663a1f0e..2c7e54fc35d93 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> @@ -168,7 +168,7 @@ static int aspeed_gfx_load(struct drm_device *drm)
> }
> }
>
> - ret = of_reserved_mem_device_init(drm->dev);
> + ret = devm_of_reserved_mem_device_init(drm->dev);
[Severity: High]
Will tying the reserved memory release to the physical device unbind phase
cause a kernel panic if a user holds an open DRM file descriptor?
The devm_of_reserved_mem_device_init() cleanup runs when the platform device
unbinds, calling of_reserved_mem_device_release() which clears dev->dma_mem.
However, if userspace holds an open DRM file descriptor, the DRM device and
its GEM objects outlive the platform device's devres cleanup phase. When the
user eventually closes the file descriptor, the GEM objects are freed.
At that point, drm_gem_dma_free() calls dma_free_wc(), which relies on
dev->dma_mem to identify the reserved memory pool. Since dev->dma_mem was
already cleared during the unbind devres cleanup, it falls back to the system
page allocator, which results in a kernel panic.
> if (ret) {
> dev_err(&pdev->dev,
> "failed to initialize reserved mem: %d\n", ret);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com?part=10
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 11/11] drm: arcpgu: Use devm_of_reserved_mem_device_init()
2026-09-02 20:16 [PATCH v2 0/11] drm: Use devm_of_reserved_mem_device_init() Mukesh Ojha
` (9 preceding siblings ...)
2026-09-02 20:16 ` [PATCH v2 10/11] drm: aspeed: " Mukesh Ojha
@ 2026-09-02 20:16 ` Mukesh Ojha
10 siblings, 0 replies; 19+ messages in thread
From: Mukesh Ojha @ 2026-09-02 20:16 UTC (permalink / raw)
To: Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Joel Stanley, Andrew Jeffery,
Paul Cercueil, Anitha Chrisanthus, Paul Kocialkowski,
Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Alexey Brodkin, Laurent Pinchart, Tomi Valkeinen, Michal Simek
Cc: Ryan Chen, Billy Tsai, dri-devel, linux-kernel, linux-aspeed,
linux-arm-kernel, linux-mips, linux-sunxi, Mukesh Ojha
Use the devres-managed devm_of_reserved_mem_device_init() to ensure
the optional reserved memory region is released on device removal,
fixing a missing cleanup in the original code.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
drivers/gpu/drm/tiny/arcpgu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c
index c93d61ac0bb7..fa01407570a8 100644
--- a/drivers/gpu/drm/tiny/arcpgu.c
+++ b/drivers/gpu/drm/tiny/arcpgu.c
@@ -278,7 +278,7 @@ static int arcpgu_load(struct arcpgu_drm_private *arcpgu)
arc_pgu_read(arcpgu, ARCPGU_REG_ID));
/* Get the optional framebuffer memory resource */
- ret = of_reserved_mem_device_init(drm->dev);
+ ret = devm_of_reserved_mem_device_init(drm->dev);
if (ret && ret != -ENODEV)
return ret;
--
2.55.0
^ permalink raw reply related [flat|nested] 19+ messages in thread