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

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

[Jiaqi]

This is v2 of the patch series that was previously withdrawn due to
a critical bug in Patch 3/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. The corrected patch simply jumps to
    err_crtcs label for proper cleanup via vop2_destroy_crtcs().
  - 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.

The bugs range from Critical (of_node reference leaks, use-after-free)
to Medium (unchecked return values), and span 5 different source files.
Each patch addresses exactly one bug to facilitate review and bisection.

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>
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c       |  5 +++--
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c  | 14 +++++++++----
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c  |  1 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c  | 11 ++++++++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  2 +-
 5 files changed, 35 insertions(+), 9 deletions(-)
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 3/6 v2] 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

[Fushuai Wang]

> 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) {
> --

Hi, Jiaqi.

Reviewed-by: Fushuai Wang <fushuai.wang@linux.dev>

-- 
Regards,
WANG

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip