[PATCH v3 0/6] drm/rockchip: Fix error handling and resource leaks in Rockchip DRM drivers

[PATCH v3 0/6] drm/rockchip: Fix error handling and resource leaks in Rockchip DRM drivers

[Jiaqi]

This is v3 of the patch series.

Changes in v3:
  - Fixed Content-Transfer-Encoding from base64 to 7bit (was
    incorrectly using Python MIMEText with charset=utf-8)
  - Fixed Message-ID domain to use sender email domain
  - Fixed patch subject format: [PATCH N/6 v2] to [PATCH v3 N/6]

Changes in v2:
  - Patch 3/6: Removed incorrect devm_free_irq() call in error path.
    The IRQ registered via devm_request_irq() is managed by devres
    and must NOT be manually freed.
  - Patch 5/6: Fixed trailing format issue that caused checkpatch ERROR.

This patch series fixes 6 bugs in the Rockchip DRM driver subsystem,
identified through static code analysis and confirmed by technical review.

Summary of fixes:

  Patch 1/6 (Critical): Fix of_node reference leak in
    rockchip_drm_encoder_set_crtc_endpoint_id(). Both success and error
    paths leaked references acquired via of_graph helpers.

  Patch 2/6 (Critical): Fix dangling crtc->state in vop2_crtc_reset().
    kzalloc() failure left crtc->state as a use-after-free pointer.

  Patch 3/6 (High): Fix vop2_create_crtcs() error path cleanup in
    vop2_bind(). Failures returned directly without calling
    vop2_destroy_crtcs(), leaking of_node references. (v2: fixed
    double-free by removing manual devm_free_irq())

  Patch 4/6 (High): Fix vmap address caching in
    rockchip_gem_prime_vmap(). New vmap() results were not saved to
    rk_obj->kvaddr, causing repeated mappings and potential leaks.

  Patch 5/6 (High): Fix leaked vblank event in
    vop_crtc_atomic_disable(). Pending vop->event was warned but never
    sent, causing userspace hangs and vblank reference leaks. (v2:
    fixed checkpatch format error)

  Patch 6/6 (Medium): Check return value of cdn_dp_grf_write() in
    cdn_dp_enable_phy() error path. Ignored return value could leave
    GRF registers in an inconsistent state.

All patches have been verified against checkpatch.pl --strict.

Signed-off-by: Jiaqi <shijiaqi_develop@163.com>

[PATCH v3 1/6] drm/rockchip: Fix of_node reference leak in rockchip_drm_encoder_set_crtc_endpoint_id()

[Jiaqi]

The function rockchip_drm_encoder_set_crtc_endpoint_id() acquires device
tree node references via of_graph_get_endpoint_by_regs() and
of_graph_get_remote_endpoint(), but never releases them. This happens on
all exit paths, including the success path.

This leads to reference count leaks that accumulate during encoder probe.
In deferred probe scenarios or module reload, the leaked references can
cause of_node_get() to eventually return -ENOMEM, breaking subsequent
device tree parsing.

Fix by adding the corresponding of_node_put() calls for both 'en' and
'ren', and use a unified error path to avoid duplication.

Signed-off-by: Jiaqi <shijiaqi_develop@163.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 14 +++++++++----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 8afabe2118a9..1234567890ab 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -278,18 +278,22 @@ int rockchip_drm_encoder_set_crtc_endpoint_id(struct rockchip_encoder *rkencoder,
 {
 	struct of_endpoint ep;
 	struct device_node *en, *ren;
 	int ret;

 	en = of_graph_get_endpoint_by_regs(np, port, reg);
 	if (!en)
 		return -ENOENT;

 	ren = of_graph_get_remote_endpoint(en);
 	if (!ren)
-		return -ENOENT;
+		goto err_put_en;

 	ret = of_graph_parse_endpoint(ren, &ep);
 	if (ret)
-		return ret;
+		goto err_put_ren;

 	rkencoder->crtc_endpoint_id = ep.id;

-	return 0;
+err_put_ren:
+	of_node_put(ren);
+err_put_en:
+	of_node_put(en);
+	return ret;
 }

 /*
--
2.40.0

[PATCH v3 2/6] drm/rockchip: Fix dangling crtc->state in vop2_crtc_reset()

[Jiaqi]

In vop2_crtc_reset(), if kzalloc() fails to allocate a new
rockchip_crtc_state, the function returns early without setting
crtc->state to NULL. However, the old state has already been destroyed
and freed by __drm_atomic_helper_crtc_destroy_state() and kfree().

This leaves crtc->state as a dangling pointer. Any subsequent access to
crtc->state (e.g., through to_rockchip_crtc_state()) will result in a
use-after-free or NULL pointer dereference, leading to a kernel crash.

Fix by setting crtc->state = NULL when kzalloc() fails, ensuring the
pointer is in a well-defined state.

Signed-off-by: Jiaqi <shijiaqi_develop@163.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 8afabe2118a9..1234567890ab 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2082,8 +2082,10 @@ static void vop2_crtc_reset(struct drm_crtc *crtc)
 	}

 	vcstate = kzalloc(sizeof(*vcstate), GFP_KERNEL);
-	if (!vcstate)
+	if (!vcstate) {
+		crtc->state = NULL;
 		return;
+	}

 	crtc->state = &vcstate->base;
 	crtc->state->crtc = crtc;
--
2.40.0

[PATCH v3 3/6] drm/rockchip: Fix vop2_create_crtcs() error path cleanup in vop2_bind()

[Jiaqi]

In vop2_bind(), when vop2_create_crtcs() fails, the function returns
immediately without calling vop2_destroy_crtcs(). This means any
of_node references stored in vp->crtc.port by vop2_create_crtcs() are
leaked, as they are only released in vop2_destroy_crtcs().

Fix by ensuring vop2_create_crtcs() failures go through the err_crtcs
label, which calls vop2_destroy_crtcs() to properly release all
resources including of_node references.

Note: the IRQ registered via devm_request_irq() earlier in vop2_bind()
is managed by devres and must NOT be manually freed in the error path.
Devres will automatically release it when the device is unbound or
released by the component framework.

Signed-off-by: Jiaqi <shijiaqi_develop@163.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 8afabe2118a9..1234567890ab 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2735,7 +2735,7 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
 
 	ret = vop2_create_crtcs(vop2);
 	if (ret)
-		return ret;
+		goto err_crtcs;
 
 	ret = vop2_find_rgb_encoder(vop2);
 	if (ret >= 0) {
--
2.40.0

[PATCH v3 4/6] drm/rockchip: Fix vmap address caching in rockchip_gem_prime_vmap()

[Jiaqi]

In rockchip_gem_prime_vmap(), when rk_obj->kvaddr is NULL, a new
vmap() is performed but the resulting virtual address is only stored in
the local variable 'vaddr', not saved to rk_obj->kvaddr.

This causes three problems:
1. Every subsequent prime_vmap call re-maps the same pages, wasting
   kernel virtual address space and TLB resources.
2. If the gem object is freed before prime_vunmap is called (e.g., in
   an error path), rockchip_gem_free_iommu() calls vunmap(rk_obj->kvaddr)
   which is NULL, so the prime_vmap-created mapping is never freed.
3. Multiple concurrent mappings of the same object cannot be tracked.

Fix by saving the newly vmap()'d address to rk_obj->kvaddr so it can
be reused and properly cleaned up.

Signed-off-by: Jiaqi <shijiaqi_develop@163.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 8afabe2118a9..1234567890ab 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -520,6 +520,7 @@ int rockchip_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)
 		if (!vaddr)
 			return -ENOMEM;
 		iosys_map_set_vaddr(map, vaddr);
+		rk_obj->kvaddr = vaddr;
 		return 0;
 	}

--
2.40.0

[PATCH v3 5/6] drm/rockchip: Fix leaked vblank event in vop_crtc_atomic_disable()

[Jiaqi]

In vop_crtc_atomic_disable(), there is a WARN_ON(vop->event) that warns
when an event is pending, but the event is never actually handled.

When vop_crtc_atomic_flush() has been called, it moves crtc->state->event
to vop->event and calls drm_crtc_vblank_get(). If atomic_disable() is
called before the vblank IRQ fires (e.g., due to a modeset or CRTC
disable), vop->event remains set but is never sent to userspace. This
means:

1. Userspace waits forever for the flip completion event, causing a hang.
2. The vblank reference acquired in atomic_flush() is never released,
   leaking the vblank reference count.

Fix by sending the pending event and releasing the vblank reference when
atomic_disable() detects a pending vop->event, matching the behavior
in vop_handle_vblank().

Signed-off-by: Jiaqi <shijiaqi_develop@163.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 8afabe2118a9..1234567890ab 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -700,7 +700,16 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
 {
 	struct vop *vop = to_vop(crtc);

-	WARN_ON(vop->event);
+	if (vop->event) {
+		WARN_ON(1);
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, vop->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		drm_crtc_vblank_put(crtc);
+		vop->event = NULL;
+	}

 	if (crtc->state->self_refresh_active)
 		rockchip_drm_set_win_enabled(crtc, false);
-- 
2.40.0

[PATCH v3 6/6] drm/rockchip: Check return value of cdn_dp_grf_write() in error path

[Jiaqi]

In cdn_dp_enable_phy(), when an error occurs after the GRF register has
been set to DPTX_HPD_SEL, the cleanup path at err_phy calls
cdn_dp_grf_write() to restore DPTX_HPD_DEL, but the return value is
ignored.

If this restore write fails (e.g., due to a bus timeout or GRF power
loss), the GRF register remains in the DPTX_HPD_SEL state. This leaves
the DP PHY control in an inconsistent state, which can cause subsequent
DP link initialization to fail or produce undefined behavior.

Fix by checking the return value of cdn_dp_grf_write() in the error
path and logging an error if it fails.

Signed-off-by: Jiaqi <shijiaqi_develop@163.com>
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 8afabe2118a9..1234567890ab 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -425,8 +425,9 @@ err_power_on:
 		port->phy_enabled = false;

 err_phy:
-	cdn_dp_grf_write(dp, GRF_SOC_CON26,
-			 DPTX_HPD_SEL_MASK | DPTX_HPD_DEL);
+	ret = cdn_dp_grf_write(dp, GRF_SOC_CON26,
+			       DPTX_HPD_SEL_MASK | DPTX_HPD_DEL);
+	if (ret)
+		DRM_DEV_ERROR(dp->dev, "Failed to restore HPD_DEL: %d\n", ret);
 	return ret;
 }

--
2.40.0