[4/5] drm/bridge: release panel reference on all lookup exit paths
Commit Message
of_drm_find_panel() and drm_of_find_panel_or_bridge() now return a
counted reference that the caller must release with drm_panel_put().
For bridge drivers that immediately wrap the panel in a panel_bridge
(which acquires its own reference), release the lookup reference right
after the bridge creation call.
For analogix-anx6345, which stores the panel for direct use, release
the reference in the i2c remove path.
For platform drivers using analogix_dp_core with a component lifecycle
(exynos_dp, rockchip analogix_dp), release the lookup reference in the
platform remove() function. The panel_bridge created during bind() holds
a separate reference that devm cleanup releases after remove() returns.
Also fix devm_drm_of_get_bridge() and drmm_of_get_bridge() in
bridge/panel.c itself: both call drm_of_find_panel_or_bridge() and
then pass the panel to devm/drmm_panel_bridge_add(), which acquires
its own reference via drm_panel_bridge_add_typed(). The lookup
reference was never released; add drm_panel_put() after each bridge
creation call.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 3 +++
drivers/gpu/drm/bridge/panel.c | 8 ++++++--
drivers/gpu/drm/exynos/exynos_dp.c | 10 ++++++++++
drivers/gpu/drm/exynos/exynos_drm_dpi.c | 3 +++
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 18 ++++++++++++++++++
drivers/gpu/drm/logicvc/logicvc_interface.c | 12 ++++++++++++
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 11 +++++++++++
drivers/gpu/drm/sti/sti_dvo.c | 3 +++
drivers/gpu/drm/stm/lvds.c | 3 +++
drivers/gpu/drm/sun4i/sun4i_lvds.c | 13 +++++++++++++
drivers/gpu/drm/sun4i/sun4i_rgb.c | 13 +++++++++++++
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 2 ++
drivers/gpu/drm/tegra/dsi.c | 1 +
drivers/gpu/drm/tegra/output.c | 3 +++
14 files changed, 101 insertions(+), 2 deletions(-)
Comments
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Dropping the panel reference before removing the bridge creates a Use-After-Free window during driver removal.
- [Medium] Panel references acquired during probe/attach are not released on error paths, causing memory leaks.
- [Medium] Panel reference leaks when wrapping a panel in a bridge without dropping the lookup reference.
--
commit 621578b5ce602b599c8bef0b18966f05081671b7
Author: Albert Esteve <aesteve@redhat.com>
drm/bridge: release panel reference on all lookup exit paths
of_drm_find_panel() and drm_of_find_panel_or_bridge() now return a
counted reference that the caller must release with drm_panel_put().
For bridge drivers that immediately wrap the panel in a panel_bridge
(which acquires its own reference), release the lookup reference right
after the bridge creation call.
[Severity: Medium]
Since the lookup functions now return a counted reference, does this
change introduce memory leaks on driver probe error paths?
If a driver successfully acquires the panel reference during probe or
attach, but a subsequent initialization step fails (like clock
registration, component addition, or memory allocation), it appears the
reference is leaked if drm_panel_put is not called before returning the
error.
For example, in exynos_dp_probe, if component_add fails, the panel
reference is not released. Similar leaks appear to happen on error
paths in analogix_dp_aux_done_probing, sun6i_dsi_attach,
tegra_output_probe, and lvds_probe.
[Severity: Medium]
Additionally, are there missing conversions for drivers that wrap the
panel in a bridge?
The commit message notes that bridge drivers immediately wrapping the
panel must release the lookup reference right after the bridge creation
call, since the bridge acquires its own reference.
However, it looks like several drivers were missed, such as
lvds_codec_probe, fsl_ldb_probe, lt9211_parse_dt, ssd2825_probe,
tc358767_probe, and tc358768_probe. In these functions, the panel is
wrapped with devm_drm_panel_bridge_add or drm_panel_bridge_add_typed,
but the initial lookup reference is never dropped.
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> index f3fe47b12edca..1fe11b075f860 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> @@ -756,6 +756,9 @@ static void anx6345_i2c_remove(struct i2c_client *client)
> {
> struct anx6345 *anx6345 = i2c_get_clientdata(client);
>
> + if (anx6345->panel)
> + drm_panel_put(anx6345->panel);
> +
> drm_bridge_remove(&anx6345->bridge);
[Severity: High]
Does this create a use-after-free window?
By dropping the last reference to the panel here, the panel object might
be freed. However, the bridge is removed after this step. During this
window, the bridge remains globally visible and registered.
If a concurrent drm commit or modeset (like an atomic disable) executes
on the display pipeline before drm_bridge_remove is called, it could
invoke the bridge callbacks which would dereference the already-freed
panel pointer.
Would it be safer to unregister the bridge before dropping the reference
to the internal structures it relies on?
[ ... ]
> diff --git a/drivers/gpu/drm/stm/lvds.c b/drivers/gpu/drm/stm/lvds.c
> index 50a878688e477..77735e26c56e3 100644
> --- a/drivers/gpu/drm/stm/lvds.c
> +++ b/drivers/gpu/drm/stm/lvds.c
> @@ -1189,6 +1189,9 @@ static void lvds_remove(struct platform_device *pdev)
> {
> struct stm_lvds *lvds = platform_get_drvdata(pdev);
>
> + if (lvds->panel)
> + drm_panel_put(lvds->panel);
> +
> lvds_pixel_clk_unregister(lvds);
>
> drm_bridge_remove(&lvds->lvds_bridge);
[Severity: High]
Could this also trigger a use-after-free for the panel?
Similar to the anx6345 driver, the panel reference is dropped before the
bridge is removed. A concurrent atomic disable could execute in this
window and dereference the freed panel inside the bridge callbacks.
On Fri, Jun 26, 2026 at 02:03:26PM +0200, Albert Esteve wrote:
> of_drm_find_panel() and drm_of_find_panel_or_bridge() now return a
> counted reference that the caller must release with drm_panel_put().
>
> For bridge drivers that immediately wrap the panel in a panel_bridge
> (which acquires its own reference), release the lookup reference right
> after the bridge creation call.
>
> For analogix-anx6345, which stores the panel for direct use, release
> the reference in the i2c remove path.
>
> For platform drivers using analogix_dp_core with a component lifecycle
> (exynos_dp, rockchip analogix_dp), release the lookup reference in the
> platform remove() function. The panel_bridge created during bind() holds
> a separate reference that devm cleanup releases after remove() returns.
>
> Also fix devm_drm_of_get_bridge() and drmm_of_get_bridge() in
> bridge/panel.c itself: both call drm_of_find_panel_or_bridge() and
> then pass the panel to devm/drmm_panel_bridge_add(), which acquires
> its own reference via drm_panel_bridge_add_typed(). The lookup
> reference was never released; add drm_panel_put() after each bridge
> creation call.
>
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
I think this one should be either split into one patch per driver, or
merged with the of_drm_find_panel patch. I'm still not quite sure which
would be the best, maybe the latter?
Maxime
On Fri, Jun 26, 2026 at 2:53 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Fri, Jun 26, 2026 at 02:03:26PM +0200, Albert Esteve wrote:
> > of_drm_find_panel() and drm_of_find_panel_or_bridge() now return a
> > counted reference that the caller must release with drm_panel_put().
> >
> > For bridge drivers that immediately wrap the panel in a panel_bridge
> > (which acquires its own reference), release the lookup reference right
> > after the bridge creation call.
> >
> > For analogix-anx6345, which stores the panel for direct use, release
> > the reference in the i2c remove path.
> >
> > For platform drivers using analogix_dp_core with a component lifecycle
> > (exynos_dp, rockchip analogix_dp), release the lookup reference in the
> > platform remove() function. The panel_bridge created during bind() holds
> > a separate reference that devm cleanup releases after remove() returns.
> >
> > Also fix devm_drm_of_get_bridge() and drmm_of_get_bridge() in
> > bridge/panel.c itself: both call drm_of_find_panel_or_bridge() and
> > then pass the panel to devm/drmm_panel_bridge_add(), which acquires
> > its own reference via drm_panel_bridge_add_typed(). The lookup
> > reference was never released; add drm_panel_put() after each bridge
> > creation call.
> >
> > Assisted-by: Claude:claude-opus-4-6
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
>
> I think this one should be either split into one patch per driver, or
> merged with the of_drm_find_panel patch. I'm still not quite sure which
> would be the best, maybe the latter?
I have spent some time myself thinking about how to approach this.
Initially I thought about doing one patch per driver as you suggested,
but since there are many similar fixes that are mostly one-liners, In
the end I decided that grouping them would make the review easier and
result in a less inflated series. Maybe merging with of_drm_find_panel
patch makes sense to avoid having one patch introducing a transient
reference leak (even if it is fixed right after and is bisectable).
But that will also create a giant patch (harder to review?). Another
option could be to merge patches 4 and 5, which basically update all
callers, and keep the API update separated. Either way, I am happy
with whatever you decide is better. I will take note of this for v2.
>
> Maxime
@@ -756,6 +756,9 @@ static void anx6345_i2c_remove(struct i2c_client *client)
{
struct anx6345 *anx6345 = i2c_get_clientdata(client);
+ if (anx6345->panel)
+ drm_panel_put(anx6345->panel);
+
drm_bridge_remove(&anx6345->bridge);
unregister_i2c_dummy_clients(anx6345);
@@ -513,8 +513,10 @@ struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
if (ret)
return ERR_PTR(ret);
- if (panel)
+ if (panel) {
bridge = devm_drm_panel_bridge_add(dev, panel);
+ drm_panel_put(panel);
+ }
return bridge;
}
@@ -547,8 +549,10 @@ struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
if (ret)
return ERR_PTR(ret);
- if (panel)
+ if (panel) {
bridge = drmm_panel_bridge_add(drm, panel);
+ drm_panel_put(panel);
+ }
return bridge;
}
@@ -193,6 +193,16 @@ static int exynos_dp_probe(struct platform_device *pdev)
static void exynos_dp_remove(struct platform_device *pdev)
{
+ struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+
+ /*
+ * Release the probe-time reference from of_drm_find_panel(). If bind
+ * ran, the panel_bridge holds a second reference that devm cleanup
+ * will release when the bridge is destroyed after remove() returns.
+ */
+ if (dp->plat_data.panel)
+ drm_panel_put(dp->plat_data.panel);
+
component_del(&pdev->dev, &exynos_dp_ops);
}
@@ -245,5 +245,8 @@ int exynos_dpi_remove(struct drm_encoder *encoder)
exynos_dpi_disable(&ctx->encoder);
+ if (ctx->panel)
+ drm_panel_put(ctx->panel);
+
return 0;
}
@@ -109,6 +109,13 @@ static int fsl_dcu_attach_panel(struct fsl_dcu_drm_device *fsl_dev,
return ret;
}
+static void fsl_dcu_panel_put_action(void *data)
+{
+ struct drm_panel *panel = data;
+
+ drm_panel_put(panel);
+}
+
int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev)
{
struct device_node *panel_node;
@@ -124,6 +131,12 @@ int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev)
if (IS_ERR(fsl_dev->connector.panel))
return PTR_ERR(fsl_dev->connector.panel);
+ ret = devm_add_action_or_reset(fsl_dev->dev,
+ fsl_dcu_panel_put_action,
+ fsl_dev->connector.panel);
+ if (ret)
+ return ret;
+
return fsl_dcu_attach_panel(fsl_dev, fsl_dev->connector.panel);
}
@@ -132,6 +145,11 @@ int fsl_dcu_create_outputs(struct fsl_dcu_drm_device *fsl_dev)
return ret;
if (panel) {
+ ret = devm_add_action_or_reset(fsl_dev->dev,
+ fsl_dcu_panel_put_action, panel);
+ if (ret)
+ return ret;
+
fsl_dev->connector.panel = panel;
return fsl_dcu_attach_panel(fsl_dev, panel);
}
@@ -28,6 +28,11 @@
#define logicvc_interface_from_drm_connector(c) \
container_of(c, struct logicvc_interface, drm_connector)
+static void logicvc_panel_put_action(void *data)
+{
+ drm_panel_put(data);
+}
+
static void logicvc_encoder_enable(struct drm_encoder *drm_encoder)
{
struct logicvc_drm *logicvc = logicvc_drm(drm_encoder->dev);
@@ -160,6 +165,13 @@ int logicvc_interface_init(struct logicvc_drm *logicvc)
if (ret == -EPROBE_DEFER)
goto error_early;
+ if (interface->drm_panel) {
+ ret = devm_add_action_or_reset(dev, logicvc_panel_put_action,
+ interface->drm_panel);
+ if (ret)
+ goto error_early;
+ }
+
ret = drm_encoder_init(drm_dev, &interface->drm_encoder,
&logicvc_encoder_funcs, encoder_type, NULL);
if (ret) {
@@ -27,6 +27,7 @@
#include <drm/drm_bridge_connector.h>
#include <drm/bridge/analogix_dp.h>
#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
#include <drm/drm_simple_kms_helper.h>
@@ -472,6 +473,16 @@ static int rockchip_dp_probe(struct platform_device *pdev)
static void rockchip_dp_remove(struct platform_device *pdev)
{
+ struct rockchip_dp_device *dp = platform_get_drvdata(pdev);
+
+ /*
+ * Release the probe-time reference from of_drm_find_panel(). If bind
+ * ran, the panel_bridge holds a second reference that devm cleanup
+ * will release when the bridge is destroyed after remove() returns.
+ */
+ if (dp->plat_data.panel)
+ drm_panel_put(dp->plat_data.panel);
+
component_del(&pdev->dev, &rockchip_dp_component_ops);
}
@@ -492,6 +492,9 @@ static void sti_dvo_unbind(struct device *dev,
{
struct sti_dvo *dvo = dev_get_drvdata(dev);
+ if (dvo->panel)
+ drm_panel_put(dvo->panel);
+
drm_bridge_remove(&dvo->bridge);
}
@@ -1189,6 +1189,9 @@ static void lvds_remove(struct platform_device *pdev)
{
struct stm_lvds *lvds = platform_get_drvdata(pdev);
+ if (lvds->panel)
+ drm_panel_put(lvds->panel);
+
lvds_pixel_clk_unregister(lvds);
drm_bridge_remove(&lvds->lvds_bridge);
@@ -18,6 +18,11 @@
#include "sun4i_tcon.h"
#include "sun4i_lvds.h"
+static void sun4i_panel_put_action(void *data)
+{
+ drm_panel_put(data);
+}
+
struct sun4i_lvds {
struct drm_connector connector;
struct drm_encoder encoder;
@@ -113,6 +118,14 @@ int sun4i_lvds_init(struct drm_device *drm, struct sun4i_tcon *tcon)
return 0;
}
+ if (lvds->panel) {
+ ret = devm_add_action_or_reset(tcon->dev,
+ sun4i_panel_put_action,
+ lvds->panel);
+ if (ret)
+ return ret;
+ }
+
drm_encoder_helper_add(&lvds->encoder,
&sun4i_lvds_enc_helper_funcs);
ret = drm_simple_encoder_init(drm, &lvds->encoder,
@@ -43,6 +43,11 @@ drm_encoder_to_sun4i_rgb(struct drm_encoder *encoder)
encoder);
}
+static void sun4i_panel_put_action(void *data)
+{
+ drm_panel_put(data);
+}
+
static int sun4i_rgb_get_modes(struct drm_connector *connector)
{
struct sun4i_rgb *rgb =
@@ -205,6 +210,14 @@ int sun4i_rgb_init(struct drm_device *drm, struct sun4i_tcon *tcon)
return 0;
}
+ if (rgb->panel) {
+ ret = devm_add_action_or_reset(tcon->dev,
+ sun4i_panel_put_action,
+ rgb->panel);
+ if (ret)
+ return ret;
+ }
+
drm_encoder_helper_add(&rgb->encoder,
&sun4i_rgb_enc_helper_funcs);
ret = drm_simple_encoder_init(drm, &rgb->encoder,
@@ -985,6 +985,8 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
{
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
+ if (dsi->panel)
+ drm_panel_put(dsi->panel);
dsi->panel = NULL;
dsi->device = NULL;
@@ -1516,6 +1516,7 @@ static int tegra_dsi_host_detach(struct mipi_dsi_host *host,
struct tegra_output *output = &dsi->output;
if (output->panel && &device->dev == output->panel->dev) {
+ drm_panel_put(output->panel);
output->panel = NULL;
if (output->connector.dev)
@@ -195,6 +195,9 @@ int tegra_output_probe(struct tegra_output *output)
void tegra_output_remove(struct tegra_output *output)
{
+ if (output->panel)
+ drm_panel_put(output->panel);
+
if (output->hpd_gpio)
free_irq(output->hpd_irq, output);