[PATCH v6 0/9] drm: writeback: clean up writeback connector initialization

[PATCH v6 0/9] drm: writeback: clean up writeback connector initialization

[Dmitry Baryshkov]

Drivers using drm_writeback_connector_init() / _with_encoder() don't
perform cleanup in a manner similar to drmm_writeback_connector_init()
(see drm_writeback_connector_cleanup()). Migrate all existing drivers
to use drmm_writeback_connector_init(), drop
drm_writeback_connector_init() and drm_writeback_connector::encoder
(it's unused afterwards).

This series leaves former drm_writeback_connector_init_with_encoder()
(renamed to drm_writeback_connector_init as a non-managed counterpart
for drmm_writeback_connector_init()). It is supposed to be used by
drivers which can not use drmm functions (like Intel). However I think
it would be better to drop it completely.

---
Changes in v6:
- Dropped applied patch
- Implemented suggestion by Alex to use drmm allocation for the
  writeback connector.
- Link to v5: https://patch.msgid.link/20260505-wb-drop-encoder-v5-0-42567b7c7af2@oss.qualcomm.com

Changes in v5:
- Rebased on top of drm-misc-next
- Expanded commit message for msm patch, describing devm vs drmm issues (Laurent)
- Expanded commit messages, describing why the drivers are converted to
  drmm_writeback_connector_init() (Laurent)
- Link to v4: https://lore.kernel.org/r/20251228-wb-drop-encoder-v4-0-58d28e668901@oss.qualcomm.com

Changes in v4:
- Rebase on top of drm-misc-next, dropping applied patch.
- Added a note regarding memory leak in the AMDGPU driver.
- Fixed a devm vs drmm issue in the msm/dpu driver.
- Link to v3: https://lore.kernel.org/r/20250819-wb-drop-encoder-v3-0-b48a6af7903b@oss.qualcomm.com

Changes in v3:
- Fixed subject prefix for the rcar-du patch (Jessica Zhang)
- Link to v2: https://lore.kernel.org/r/20250816-wb-drop-encoder-v2-0-f951de04f4f9@oss.qualcomm.com

Changes in v2:
- Switched to drm_crtc_mask() where applicable (Louis Chauvet)
- Link to v1: https://lore.kernel.org/r/20250801-wb-drop-encoder-v1-0-824646042f7d@oss.qualcomm.com

To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Harry Wentland <harry.wentland@amd.com>
To: Leo Li <sunpeng.li@amd.com>
To: Rodrigo Siqueira <siqueira@igalia.com>
To: Alex Deucher <alexander.deucher@amd.com>
To: Christian König <christian.koenig@amd.com>
To: Liviu Dudau <liviu.dudau@arm.com>
To: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
To: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
To: Geert Uytterhoeven <geert+renesas@glider.be>
To: Magnus Damm <magnus.damm@gmail.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
To: Maíra Canal <mcanal@igalia.com>
To: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-renesas-soc@vger.kernel.org

---
Dmitry Baryshkov (9):
      drm/managed: implement managed versions of kzalloc_obj/objs()
      drm/amd/display: use drmm allocation for writeback connector
      drm/amd/display: use drmm_writeback_connector_init()
      drm/komeda: use drmm_writeback_connector_init()
      drm/mali: use drmm_writeback_connector_init()
      drm: renesas: rcar-du: use drmm_writeback_connector_init()
      drm/vc4: use drmm_writeback_connector_init()
      drm: writeback: drop excess connector initialization functions
      drm: writeback: rename drm_writeback_connector_init_with_encoder()

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  7 ++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c   | 18 ++++--
 .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++----
 drivers/gpu/drm/arm/malidp_mw.c                    | 25 ++++----
 drivers/gpu/drm/drm_writeback.c                    | 69 +++-------------------
 .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 22 ++++---
 drivers/gpu/drm/vc4/vc4_txp.c                      |  9 ++-
 include/drm/drm_managed.h                          | 22 +++++++
 include/drm/drm_writeback.h                        | 22 +------
 9 files changed, 98 insertions(+), 126 deletions(-)
---
base-commit: 2afdfc658f7a7e9ee2a67ec6663922da9c799c53
change-id: 20250801-wb-drop-encoder-97a0c75bd5d7

Best regards,
--  
With best wishes
Dmitry

[PATCH v6 1/9] drm/managed: implement managed versions of kzalloc_obj/objs()

[Dmitry Baryshkov]

The kzalloc_obj() and kzalloc_objs() functions proven to be useful and
widely used for the reset of the kernel. Implement a drm_device-managed
versions of those macros to mix the nice interface with the automatic
freeing of the pointers.

Note: the original macros accept optional GFP_foo arguments. They are
skipped for now, making all allocations use GFP_KERNEL. If necessary,
support for overriding the GFP type can be introduced later.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 include/drm/drm_managed.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index 72d0d68be226..79049bec277d 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -105,6 +105,28 @@ static inline void *drmm_kcalloc(struct drm_device *dev,
 	return drmm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
 }
 
+/**
+ * drmm_kzalloc_objs - &drm_device-managed kzalloc_objs()
+ * @dev: DRM device
+ * @p: Variable or type to allocate an array of
+ * @count: How many elements in the array
+ *
+ * Returns: newly allocated pointer to the zeroed array of @P on success, or
+ * NULL on failure.
+ */
+#define drmm_kzalloc_objs(dev, p, count) \
+	drmm_kcalloc(dev, count, sizeof(typeof(p)), GFP_KERNEL)
+
+/**
+ * drmm_kzalloc_obj - &drm_device-managed kzalloc_obj()
+ * @dev: DRM device
+ * @p: Variable or type to allocate
+ *
+ * Returns: newly allocated pointer to a @p on success, or NULL on failure.
+ */
+#define drmm_kzalloc_obj(dev, P) \
+	drmm_kzalloc_objs(dev, P, 1)
+
 char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp);
 
 void drmm_kfree(struct drm_device *dev, void *data);

-- 
2.47.3

Re: [PATCH v6 1/9] drm/managed: implement managed versions of kzalloc_obj/objs()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing explicit return type cast in drmm_kzalloc_objs / drmm_kzalloc_obj permits silent heap buffer overflows.
- [Low] Kernel-doc parameter name mismatch in drmm_kzalloc_obj triggers documentation warnings.
--

commit 852017e1ea6892532ead5eeb34b599432f852a80
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/managed: implement managed versions of kzalloc_obj/objs()

This commit implements drm_device-managed versions of the kzalloc_obj() and
kzalloc_objs() macros. They provide a convenient interface for allocating
objects with automatic freeing via the DRM managed API, defaulting to
GFP_KERNEL.

> diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> index 72bfac002c062..d196387506c97 100644
> --- a/include/drm/drm_managed.h
> +++ b/include/drm/drm_managed.h
> @@ -105,6 +105,28 @@ static inline void *drmm_kcalloc(struct drm_device *dev,
>  	return drmm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
>  }
>  
> +/**
> + * drmm_kzalloc_objs - &drm_device-managed kzalloc_objs()
> + * @dev: DRM device
> + * @p: Variable or type to allocate an array of
> + * @count: How many elements in the array
> + *
> + * Returns: newly allocated pointer to the zeroed array of @P on success, or
> + * NULL on failure.
> + */
> +#define drmm_kzalloc_objs(dev, p, count) \
> +	drmm_kcalloc(dev, count, sizeof(typeof(p)), GFP_KERNEL)

[Severity: High]
Does this macro need an explicit return type cast like the standard
kzalloc_objs() macro has?

The standard kzalloc_obj(P) and kzalloc_objs(P, count) macros explicitly cast
their return values to (typeof(P) *). This cast provides a compile-time safety
guard: if a developer mistakenly passes a pointer variable ptr instead of the
dereferenced pointer *ptr (e.g., ptr = kzalloc_obj(ptr)), the macro evaluates
sizeof(typeof(ptr)) (allocating 8 bytes) and returns a double pointer
(typeof(ptr) *). Assigning this back to ptr triggers an "assignment from
incompatible pointer type" compiler warning, catching the error immediately.

Since drmm_kzalloc_objs directly returns void * from drmm_kcalloc, the exact
same mistake (ptr = drmm_kzalloc_obj(dev, ptr)) compiles silently without
warnings because C implicitly casts void * to any pointer type. This allocates
only the 8-byte pointer size instead of the full structure size, which could
lead to a silent out-of-bounds heap overwrite.

Should this macro wrap the drmm_kcalloc call in an explicit ((typeof(p) *) ...)
cast to maintain the API safety guarantees of the standard macros it mimics?

> +
> +/**
> + * drmm_kzalloc_obj - &drm_device-managed kzalloc_obj()
> + * @dev: DRM device
> + * @p: Variable or type to allocate
> + *
> + * Returns: newly allocated pointer to a @p on success, or NULL on failure.
> + */
> +#define drmm_kzalloc_obj(dev, P) \
> +	drmm_kzalloc_objs(dev, P, 1)

[Severity: Low]
This isn't a bug, but does the macro parameter case mismatch the kernel-doc?

In the kernel-doc comment for drmm_kzalloc_obj, the parameter is documented
as @p (lowercase). However, the actual macro definition uses P (uppercase)
as the parameter name.

Since kernel-doc is case-sensitive, this mismatch will trigger warnings for
an "Excess function parameter p" and "Function parameter P not described"
during document generation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-wb-drop-encoder-v6-0-9f3a54f81310@oss.qualcomm.com?part=1

[PATCH v6 2/9] drm/amd/display: use drmm allocation for writeback connector

[Dmitry Baryshkov]

Currently memory used for writeback connectors isn't freed on driver
unbinding, leaking the memory. Use drm_device-memory allocation
function for the writeback connector, making sure that the memory is
freed on the driver unbinding.

Reported-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Suggested-by: Alex Hung <alex.hung@amd.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index ece581609cf7..436562e32e63 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -91,6 +91,7 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_eld.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_mode.h>
 #include <drm/drm_utils.h>
 #include <drm/drm_vblank.h>
@@ -5807,7 +5808,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
 		link = dc_get_link_at_index(dm->dc, i);
 
 		if (link->connector_signal == SIGNAL_TYPE_VIRTUAL) {
-			struct amdgpu_dm_wb_connector *wbcon = kzalloc_obj(*wbcon);
+			struct amdgpu_dm_wb_connector *wbcon = drmm_kzalloc_obj(adev_to_drm(adev), *wbcon);
 
 			if (!wbcon) {
 				drm_err(adev_to_drm(adev), "KMS: Failed to allocate writeback connector\n");
@@ -5816,7 +5817,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
 
 			if (amdgpu_dm_wb_connector_init(dm, wbcon, i)) {
 				drm_err(adev_to_drm(adev), "KMS: Failed to initialize writeback connector\n");
-				kfree(wbcon);
+				drmm_kfree(wbcon);
 				continue;
 			}
 

-- 
2.47.3

Re: [PATCH v6 2/9] drm/amd/display: use drmm allocation for writeback connector

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Compilation error due to missing argument in drmm_kfree().

Pre-existing issues:
- [High] Use-After-Free of `aencoder` and `aconnector` on error path.
--

commit 0f53c9b8f894290640afe2ed4a23503b2ea87d24
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/amd/display: use drmm allocation for writeback connector

Currently memory used for writeback connectors isn't freed on driver
unbinding, leaking the memory. Use drm_device-memory allocation
function for the writeback connector, making sure that the memory is
freed on the driver unbinding.

Reported-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Suggested-by: Alex Hung <alex.hung@amd.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 97ab1e83b3186..638335ff7d019 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

[ ... ]

> @@ -5874,7 +5875,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>  
>  			if (amdgpu_dm_wb_connector_init(dm, wbcon, i)) {
>  				drm_err(adev_to_drm(adev), "KMS: Failed to initialize writeback connector\n");
> -				kfree(wbcon);
> +				drmm_kfree(wbcon);

[Severity: Low]
Does this code fail to compile?  Looking at include/drm/drm_managed.h, the
function drmm_kfree() requires both a drm_device pointer and the data pointer,
but it is being called with only the data pointer here.

[Severity: High]
This isn't a bug introduced by this patch, but can the fail block in
amdgpu_dm_initialize_drm_device() trigger a use-after-free during teardown?

If amdgpu_dm_encoder_init() succeeds, the encoder is registered to the DRM
core list.  If a subsequent step fails, the code jumps to the fail block:

fail:
        kfree(aencoder);
        kfree(aconnector);

        return -EINVAL;

Since the encoder remains registered in the DRM subsystem, will
drm_mode_config_cleanup() later traverse the list and call the destroy hook,
resulting in a double-free of the encoder?

>  				continue;
>  			}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-wb-drop-encoder-v6-0-9f3a54f81310@oss.qualcomm.com?part=2

[PATCH v6 3/9] drm/amd/display: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

The driver uses drm_writeback_connector_init() instead of its drmm
counterpart, but it doesn't perform the job queue cleanup (neither
manually nor by calling drm_writeback_connector_cleanup()). On the
contrary, the drmm_writeback_connector_init() function ensures the
proper cleanup of the job queue.

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    |  2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c | 18 +++++++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 436562e32e63..066ba6b6ef76 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10996,7 +10996,7 @@ static void dm_set_writeback(struct amdgpu_display_manager *dm,
 		return;
 	}
 
-	acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc);
+	acrtc = to_amdgpu_crtc(crtc_state->base.crtc);
 	if (!acrtc) {
 		drm_err(adev_to_drm(adev), "no amdgpu_crtc found\n");
 		kfree(wb_info);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
index 110f0173eee6..fdc3da40452f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
@@ -169,7 +169,6 @@ static const struct drm_encoder_helper_funcs amdgpu_dm_wb_encoder_helper_funcs =
 
 static const struct drm_connector_funcs amdgpu_dm_wb_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
 	.reset = amdgpu_dm_connector_funcs_reset,
 	.atomic_duplicate_state = amdgpu_dm_connector_atomic_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -188,17 +187,26 @@ int amdgpu_dm_wb_connector_init(struct amdgpu_display_manager *dm,
 	struct dc *dc = dm->dc;
 	struct dc_link *link = dc_get_link_at_index(dc, link_index);
 	int res = 0;
+	struct drm_encoder *encoder;
+
+	encoder = drmm_plain_encoder_alloc(&dm->adev->ddev, NULL,
+					   DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &amdgpu_dm_wb_encoder_helper_funcs);
+
+	encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(dm->adev);
 
 	wbcon->link = link;
 
 	drm_connector_helper_add(&wbcon->base.base, &amdgpu_dm_wb_conn_helper_funcs);
 
-	res = drm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
+	res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
 					    &amdgpu_dm_wb_connector_funcs,
-					    &amdgpu_dm_wb_encoder_helper_funcs,
+					    encoder,
 					    amdgpu_dm_wb_formats,
-					    ARRAY_SIZE(amdgpu_dm_wb_formats),
-					    amdgpu_dm_get_encoder_crtc_mask(dm->adev));
+					    ARRAY_SIZE(amdgpu_dm_wb_formats));
 
 	if (res)
 		return res;

-- 
2.47.3

Re: [PATCH v6 3/9] drm/amd/display: use drmm_writeback_connector_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Dangling drmm cleanup action on writeback connector initialization error path leads to Use-After-Free, and possible build failure with drmm_kfree.

Pre-existing issues:
- [High] Pre-existing memory leak of `wb_info` and sleeping in atomic context in `dm_set_writeback()`.
--

commit 946e1af8401710237d25526b5d87b4357eceb542
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/amd/display: use drmm_writeback_connector_init()

The patch converts the driver to use drmm_writeback_connector_init() and
drmm_plain_encoder_alloc() to ensure proper cleanup of the writeback
connector instance and its job queue.

> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
[ ... ]
> @@ -11086,7 +11086,7 @@ static void dm_set_writeback(struct amdgpu_display_manager *dm,
>  		return;
>  	}
>  
> -	acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc);
> +	acrtc = to_amdgpu_crtc(crtc_state->base.crtc);

[Severity: High]
This is a pre-existing issue, but does dm_set_writeback() allocate memory
using GFP_KERNEL in an atomic context?

It appears that wb_info is allocated using kzalloc_obj() earlier in this
function, which implicitly uses GFP_KERNEL. However, this function is called
from amdgpu_dm_atomic_commit_tail(), which is an atomic context where sleeping
functions are forbidden.

Additionally, dc_stream_add_writeback() copies the contents of wb_info by
value, but the originally allocated pointer seems to never be freed before
dm_set_writeback() returns. Does this introduce a memory leak every time a
writeback job is queued?

>  	if (!acrtc) {
>  		drm_err(adev_to_drm(adev), "no amdgpu_crtc found\n");
>  		kfree(wb_info);

[ ... ]
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
[ ... ]
> @@ -188,17 +187,26 @@ int amdgpu_dm_wb_connector_init(struct amdgpu_display_manager *dm,
>  	struct dc *dc = dm->dc;
>  	struct dc_link *link = dc_get_link_at_index(dc, link_index);
>  	int res = 0;
> +	struct drm_encoder *encoder;
> +
> +	encoder = drmm_plain_encoder_alloc(&dm->adev->ddev, NULL,
> +					   DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);
> +
> +	drm_encoder_helper_add(encoder, &amdgpu_dm_wb_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(dm->adev);
>  
>  	wbcon->link = link;
>  
>  	drm_connector_helper_add(&wbcon->base.base, &amdgpu_dm_wb_conn_helper_funcs);
>  
> -	res = drm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
> +	res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon->base,
>  					    &amdgpu_dm_wb_connector_funcs,
> -					    &amdgpu_dm_wb_encoder_helper_funcs,
> +					    encoder,
>  					    amdgpu_dm_wb_formats,
> -					    ARRAY_SIZE(amdgpu_dm_wb_formats),
> -					    amdgpu_dm_get_encoder_crtc_mask(dm->adev));
> +					    ARRAY_SIZE(amdgpu_dm_wb_formats));

[Severity: High]
If drmm_writeback_connector_init() fails internally after successfully calling
drmm_connector_init(), will it leave the drmm_connector_cleanup_action
registered?

The caller, amdgpu_dm_initialize_drm_device(), appears to handle this failure
by attempting to free the connector using drmm_kfree(). Since drmm_kfree()
requires a dev argument, will this missing argument result in a build failure?

More importantly, since drmm_kfree() frees the memory but does not unregister
actions, will the managed resource list later execute
drmm_connector_cleanup_action on the already-freed pointer, resulting in a
use-after-free when the DRM device is torn down?

>  
>  	if (res)
>  		return res;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-wb-drop-encoder-v6-0-9f3a54f81310@oss.qualcomm.com?part=3

[PATCH v6 4/9] drm/komeda: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

The driver uses drm_writeback_connector_init() instead of its drmm
counterpart, but it doesn't perform the job queue cleanup (neither
manually nor by calling drm_writeback_connector_cleanup()). On the
contrary, the drmm_writeback_connector_init() function ensures the
proper cleanup of the job queue.

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++++++++----------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index 41cc3e080dc9..bcc53d4015f1 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -5,6 +5,7 @@
  *
  */
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_managed.h>
 #include "komeda_dev.h"
 #include "komeda_kms.h"
 
@@ -121,17 +122,10 @@ komeda_wb_connector_fill_modes(struct drm_connector *connector,
 	return 0;
 }
 
-static void komeda_wb_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_cleanup(connector);
-	kfree(to_kconn(to_wb_conn(connector)));
-}
-
 static const struct drm_connector_funcs komeda_wb_connector_funcs = {
 	.reset			= drm_atomic_helper_connector_reset,
 	.detect			= komeda_wb_connector_detect,
 	.fill_modes		= komeda_wb_connector_fill_modes,
-	.destroy		= komeda_wb_connector_destroy,
 	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
 };
@@ -143,13 +137,15 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
 	struct komeda_wb_connector *kwb_conn;
 	struct drm_writeback_connector *wb_conn;
 	struct drm_display_info *info;
+	struct drm_encoder *encoder;
+
 	u32 *formats, n_formats = 0;
 	int err;
 
 	if (!kcrtc->master->wb_layer)
 		return 0;
 
-	kwb_conn = kzalloc_obj(*kwb_conn);
+	kwb_conn = drmm_kzalloc(&kms->base, sizeof(*kwb_conn), GFP_KERNEL);
 	if (!kwb_conn)
 		return -ENOMEM;
 
@@ -165,11 +161,19 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
 		return -ENOMEM;
 	}
 
-	err = drm_writeback_connector_init(&kms->base, wb_conn,
-					   &komeda_wb_connector_funcs,
-					   &komeda_wb_encoder_helper_funcs,
-					   formats, n_formats,
-					   BIT(drm_crtc_index(&kcrtc->base)));
+	encoder = drmm_plain_encoder_alloc(&kms->base, NULL,
+					   DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &komeda_wb_encoder_helper_funcs);
+
+	encoder->possible_crtcs = drm_crtc_mask(&kcrtc->base);
+
+	err = drmm_writeback_connector_init(&kms->base, wb_conn,
+					    &komeda_wb_connector_funcs,
+					    encoder,
+					    formats, n_formats);
 	komeda_put_fourcc_list(formats);
 	if (err) {
 		kfree(kwb_conn);

-- 
2.47.3

Re: [PATCH v6 4/9] drm/komeda: use drmm_writeback_connector_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling kfree() on a pointer allocated by drmm_kzalloc() will cause a kernel panic or slab corruption.
- [Medium] The dynamically allocated `formats` array is leaked if `drmm_plain_encoder_alloc()` fails.
--

commit 05d31d945f92ca6cf7a26ad265d8715c134b9631
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/komeda: use drmm_writeback_connector_init()

This commit converts the writeback connector and its encoder initialization
to use DRM managed equivalents.

> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c

[ ... ]

> @@ -143,13 +137,15 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
>  	struct komeda_wb_connector *kwb_conn;
>  	struct drm_writeback_connector *wb_conn;
>  	struct drm_display_info *info;
> +	struct drm_encoder *encoder;
> +
>  	u32 *formats, n_formats = 0;
>  	int err;
>  
>  	if (!kcrtc->master->wb_layer)
>  		return 0;
>  
> -	kwb_conn = kzalloc_obj(*kwb_conn);
> +	kwb_conn = drmm_kzalloc(&kms->base, sizeof(*kwb_conn), GFP_KERNEL);
>  	if (!kwb_conn)
>  		return -ENOMEM;
>  
> @@ -165,11 +161,19 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
>  		return -ENOMEM;
>  	}

[Severity: High]
Does this error path in komeda_wb_connector_add() call kfree() on a DRM
managed allocation?

The kwb_conn pointer is now allocated via drmm_kzalloc(), which manages
memory via the DRM device lifecycle and returns a pointer pointing inside a
slab object (after the drmres tracking header).

The error path just above this context shows:

	if (!formats) {
		kfree(kwb_conn);
		return -ENOMEM;
	}

Calling kfree() on this managed pointer will pass an offset pointer to the
slab allocator, which can result in an invalid free that triggers a kernel
panic. DRM-managed memory must not be manually freed with kfree().

> -	err = drm_writeback_connector_init(&kms->base, wb_conn,
> -					   &komeda_wb_connector_funcs,
> -					   &komeda_wb_encoder_helper_funcs,
> -					   formats, n_formats,
> -					   BIT(drm_crtc_index(&kcrtc->base)));
> +	encoder = drmm_plain_encoder_alloc(&kms->base, NULL,
> +					   DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);

[Severity: Medium]
Does this error path leak the formats array in komeda_wb_connector_add()?

The formats array was dynamically allocated earlier, but returning directly
here skips calling komeda_put_fourcc_list(formats), leaking the memory.

> +
> +	drm_encoder_helper_add(encoder, &komeda_wb_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = drm_crtc_mask(&kcrtc->base);
> +
> +	err = drmm_writeback_connector_init(&kms->base, wb_conn,
> +					    &komeda_wb_connector_funcs,
> +					    encoder,
> +					    formats, n_formats);
>  	komeda_put_fourcc_list(formats);
>  	if (err) {
>  		kfree(kwb_conn);

[Severity: High]
Can this kfree() corrupt memory in komeda_wb_connector_add()?

Similar to the previous error path, kwb_conn is now allocated with
drmm_kzalloc(), and manually calling kfree() on a DRM managed pointer
can lead to a kernel panic.

>  		return err;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-wb-drop-encoder-v6-0-9f3a54f81310@oss.qualcomm.com?part=4

[PATCH v6 5/9] drm/mali: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

The driver uses drm_writeback_connector_init() instead of its drmm
counterpart, but it doesn't perform the job queue cleanup (neither
manually nor by calling drm_writeback_connector_cleanup()). On the
contrary, the drmm_writeback_connector_init() function ensures the
proper cleanup of the job queue.

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/arm/malidp_mw.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index f8be65119e3b..cfb7300e3e95 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -84,11 +84,6 @@ malidp_mw_connector_detect(struct drm_connector *connector, bool force)
 	return connector_status_connected;
 }
 
-static void malidp_mw_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_cleanup(connector);
-}
-
 static struct drm_connector_state *
 malidp_mw_connector_duplicate_state(struct drm_connector *connector)
 {
@@ -114,7 +109,6 @@ static const struct drm_connector_funcs malidp_mw_connector_funcs = {
 	.reset = malidp_mw_connector_reset,
 	.detect = malidp_mw_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = malidp_mw_connector_destroy,
 	.atomic_duplicate_state = malidp_mw_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
@@ -211,6 +205,7 @@ static u32 *get_writeback_formats(struct malidp_drm *malidp, int *n_formats)
 int malidp_mw_connector_init(struct drm_device *drm)
 {
 	struct malidp_drm *malidp = drm_to_malidp(drm);
+	struct drm_encoder *encoder;
 	u32 *formats;
 	int ret, n_formats;
 
@@ -224,11 +219,19 @@ int malidp_mw_connector_init(struct drm_device *drm)
 	if (!formats)
 		return -ENOMEM;
 
-	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
-					   &malidp_mw_connector_funcs,
-					   &malidp_mw_encoder_helper_funcs,
-					   formats, n_formats,
-					   1 << drm_crtc_index(&malidp->crtc));
+	encoder = drmm_plain_encoder_alloc(drm, NULL, DRM_MODE_ENCODER_VIRTUAL,
+					   NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &malidp_mw_encoder_helper_funcs);
+
+	encoder->possible_crtcs = drm_crtc_mask(&malidp->crtc);
+
+	ret = drmm_writeback_connector_init(drm, &malidp->mw_connector,
+					    &malidp_mw_connector_funcs,
+					    encoder,
+					    formats, n_formats);
 	kfree(formats);
 	if (ret)
 		return ret;

-- 
2.47.3

Re: [PATCH v6 5/9] drm/mali: use drmm_writeback_connector_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory leak of the `formats` array on the encoder allocation error path.
--

commit ca651199ad906493630752bf01737d3dca9cebbd
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
drm/mali: use drmm_writeback_connector_init()

This commit transitions the mali driver to use the managed
drmm_writeback_connector_init() function to ensure proper job queue
cleanup. It also allocates the virtual encoder using the managed
drmm_plain_encoder_alloc() helper.

> diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
> --- a/drivers/gpu/drm/arm/malidp_mw.c
> +++ b/drivers/gpu/drm/arm/malidp_mw.c

[ ... ]

> @@ -224,11 +219,19 @@ int malidp_mw_connector_init(struct drm_device *drm)
>  	if (!formats)
>  		return -ENOMEM;
>  
> -	ret = drm_writeback_connector_init(drm, &malidp->mw_connector,
> -					   &malidp_mw_connector_funcs,
> -					   &malidp_mw_encoder_helper_funcs,
> -					   formats, n_formats,
> -					   1 << drm_crtc_index(&malidp->crtc));
> +	encoder = drmm_plain_encoder_alloc(drm, NULL, DRM_MODE_ENCODER_VIRTUAL,
> +					   NULL);
> +	if (IS_ERR(encoder))
> +		return PTR_ERR(encoder);

[Severity: Medium]
The formats array is allocated earlier in this function via
get_writeback_formats(), but it does not appear to be freed on this
error path. Does this code leak the formats array if
drmm_plain_encoder_alloc() fails?

> +
> +	drm_encoder_helper_add(encoder, &malidp_mw_encoder_helper_funcs);
> +
> +	encoder->possible_crtcs = drm_crtc_mask(&malidp->crtc);
> +
> +	ret = drmm_writeback_connector_init(drm, &malidp->mw_connector,
> +					    &malidp_mw_connector_funcs,
> +					    encoder,
> +					    formats, n_formats);
>  	kfree(formats);
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-wb-drop-encoder-v6-0-9f3a54f81310@oss.qualcomm.com?part=5

[PATCH v6 6/9] drm: renesas: rcar-du: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

The driver uses drm_writeback_connector_init() instead of its drmm
counterpart, but it doesn't perform the job queue cleanup (neither
manually nor by calling drm_writeback_connector_cleanup()). On the
contrary, the drmm_writeback_connector_init() function ensures the
proper cleanup of the job queue.

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
index e5e6e6a156aa..ecfd4fc1f210 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c
@@ -134,7 +134,6 @@ static void rcar_du_wb_conn_reset(struct drm_connector *connector)
 static const struct drm_connector_funcs rcar_du_wb_conn_funcs = {
 	.reset = rcar_du_wb_conn_reset,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
 	.atomic_duplicate_state = rcar_du_wb_conn_duplicate_state,
 	.atomic_destroy_state = rcar_du_wb_conn_destroy_state,
 };
@@ -201,16 +200,25 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
 			   struct rcar_du_crtc *rcrtc)
 {
 	struct drm_writeback_connector *wb_conn = &rcrtc->writeback;
+	struct drm_encoder *encoder;
+
+	encoder = drmm_plain_encoder_alloc(&rcdu->ddev, NULL,
+					   DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (IS_ERR(encoder))
+		return PTR_ERR(encoder);
+
+	drm_encoder_helper_add(encoder, &rcar_du_wb_enc_helper_funcs);
+
+	encoder->possible_crtcs = drm_crtc_mask(&rcrtc->crtc);
 
 	drm_connector_helper_add(&wb_conn->base,
 				 &rcar_du_wb_conn_helper_funcs);
 
-	return drm_writeback_connector_init(&rcdu->ddev, wb_conn,
-					    &rcar_du_wb_conn_funcs,
-					    &rcar_du_wb_enc_helper_funcs,
-					    writeback_formats,
-					    ARRAY_SIZE(writeback_formats),
-					    1 << drm_crtc_index(&rcrtc->crtc));
+	return drmm_writeback_connector_init(&rcdu->ddev, wb_conn,
+					     &rcar_du_wb_conn_funcs,
+					     encoder,
+					     writeback_formats,
+					     ARRAY_SIZE(writeback_formats));
 }
 
 void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,

-- 
2.47.3

[PATCH v6 7/9] drm/vc4: use drmm_writeback_connector_init()

[Dmitry Baryshkov]

The driver uses drm_writeback_connector_init() instead of its drmm
counterpart, but it doesn't perform the job queue cleanup (neither
manually nor by calling drm_writeback_connector_cleanup()). On the
contrary, the drmm_writeback_connector_init() function ensures the
proper cleanup of the job queue.

Use drmm_plain_encoder_alloc() to allocate simple encoder and
drmm_writeback_connector_init() in order to initialize writeback
connector instance.

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/vc4/vc4_txp.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 66b6f2acf862..3fd89fccfa10 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -378,7 +378,6 @@ vc4_txp_connector_detect(struct drm_connector *connector, bool force)
 static const struct drm_connector_funcs vc4_txp_connector_funcs = {
 	.detect = vc4_txp_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
 	.reset = drm_atomic_helper_connector_reset,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -602,10 +601,10 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
 
 	drm_connector_helper_add(&txp->connector.base,
 				 &vc4_txp_connector_helper_funcs);
-	ret = drm_writeback_connector_init_with_encoder(drm, &txp->connector,
-							encoder,
-							&vc4_txp_connector_funcs,
-							drm_fmts, ARRAY_SIZE(drm_fmts));
+	ret = drmm_writeback_connector_init(drm, &txp->connector,
+					    &vc4_txp_connector_funcs,
+					    encoder,
+					    drm_fmts, ARRAY_SIZE(drm_fmts));
 	if (ret)
 		return ret;
 

-- 
2.47.3

Re: [PATCH v6 7/9] drm/vc4: use drmm_writeback_connector_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch converts the writeback connector to use DRM-managed initialization (`drmm_writeback_connector_init`) but leaves the manual cleanup in `vc4_txp_unbind`, causing a double-free and NULL pointer dereference during device teardown.
- [Medium] The commit message claims a change to encoder allocation that is entirely absent from the patch.
--

commit 2e74a8f54aeae971a4434e6be6dd5849a3cfa4ff
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/vc4: use drmm_writeback_connector_init()

The driver uses drm_writeback_connector_init() instead of its drmm
counterpart, but it doesn't perform the job queue cleanup. The
drmm_writeback_connector_init() function ensures the proper cleanup of
the job queue.

> Use drmm_plain_encoder_alloc() to allocate simple encoder and
> drmm_writeback_connector_init() in order to initialize writeback
> connector instance.

[Severity: Medium]
This isn't a bug, but the commit message mentions using
drmm_plain_encoder_alloc(), yet this function isn't used anywhere in the patch.

The encoder remains embedded in struct vc4_txp and continues to be initialized
via drmm_encoder_init() in vc4_txp_bind().

Could the commit message be updated to match the actual code?

> diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
> index 66b6f2acf8626..3fd89fccfa10a 100644
> --- a/drivers/gpu/drm/vc4/vc4_txp.c
> +++ b/drivers/gpu/drm/vc4/vc4_txp.c
[ ... ]
> @@ -602,10 +601,10 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
>  
>  	drm_connector_helper_add(&txp->connector.base,
>  				 &vc4_txp_connector_helper_funcs);
> -	ret = drm_writeback_connector_init_with_encoder(drm, &txp->connector,
> -							encoder,
> -							&vc4_txp_connector_funcs,
> -							drm_fmts, ARRAY_SIZE(drm_fmts));
> +	ret = drmm_writeback_connector_init(drm, &txp->connector,
> +					    &vc4_txp_connector_funcs,
> +					    encoder,
> +					    drm_fmts, ARRAY_SIZE(drm_fmts));

[Severity: High]
Since we are moving to drmm_writeback_connector_init(), which automatically
handles cleanup by registering a managed action, is it still safe to keep
the manual drm_connector_cleanup() call in vc4_txp_unbind()?

drivers/gpu/drm/vc4/vc4_txp.c:vc4_txp_unbind() {
	struct vc4_txp *txp = dev_get_drvdata(dev);

	drm_connector_cleanup(&txp->connector.base);
}

During teardown, vc4_txp_unbind() will manually clean up the connector,
but the drmm-managed action will later attempt to clean it up again when
the drm_device is released.

Does this cause a double-free or NULL pointer dereference on device teardown?

>  	if (ret)
>  		return ret;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-wb-drop-encoder-v6-0-9f3a54f81310@oss.qualcomm.com?part=7

[PATCH v6 8/9] drm: writeback: drop excess connector initialization functions

[Dmitry Baryshkov]

Now as all drivers have been converted to
drmm_writeback_connector_init(), drop drm_writeback_connector_init() and
drm_writeback_connector::encoder field, they are unused now.

Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_writeback.c | 55 -----------------------------------------
 include/drm/drm_writeback.h     | 18 --------------
 2 files changed, 73 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 4da5d6094721..9fc15168c34f 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -142,61 +142,6 @@ static int create_writeback_properties(struct drm_device *dev)
 	return 0;
 }
 
-static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
-	.destroy = drm_encoder_cleanup,
-};
-
-/**
- * drm_writeback_connector_init - Initialize a writeback connector and its properties
- * @dev: DRM device
- * @wb_connector: Writeback connector to initialize
- * @con_funcs: Connector funcs vtable
- * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
- * @formats: Array of supported pixel formats for the writeback engine
- * @n_formats: Length of the formats array
- * @possible_crtcs: possible crtcs for the internal writeback encoder
- *
- * This function creates the writeback-connector-specific properties if they
- * have not been already created, initializes the connector as
- * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
- * values. It will also create an internal encoder associated with the
- * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
- * the encoder helper.
- *
- * Drivers should always use this function instead of drm_connector_init() to
- * set up writeback connectors.
- *
- * Returns: 0 on success, or a negative error code
- */
-int drm_writeback_connector_init(struct drm_device *dev,
-				 struct drm_writeback_connector *wb_connector,
-				 const struct drm_connector_funcs *con_funcs,
-				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
-				 const u32 *formats, int n_formats,
-				 u32 possible_crtcs)
-{
-	int ret = 0;
-
-	drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
-
-	wb_connector->encoder.possible_crtcs = possible_crtcs;
-
-	ret = drm_encoder_init(dev, &wb_connector->encoder,
-			       &drm_writeback_encoder_funcs,
-			       DRM_MODE_ENCODER_VIRTUAL, NULL);
-	if (ret)
-		return ret;
-
-	ret = drm_writeback_connector_init_with_encoder(dev, wb_connector, &wb_connector->encoder,
-			con_funcs, formats, n_formats);
-
-	if (ret)
-		drm_encoder_cleanup(&wb_connector->encoder);
-
-	return ret;
-}
-EXPORT_SYMBOL(drm_writeback_connector_init);
-
 static void delete_writeback_properties(struct drm_device *dev)
 {
 	if (dev->mode_config.writeback_pixel_formats_property) {
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index c380a7b8f55a..879ca103320c 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -24,17 +24,6 @@ struct drm_writeback_connector {
 	 */
 	struct drm_connector base;
 
-	/**
-	 * @encoder: Internal encoder used by the connector to fulfill
-	 * the DRM framework requirements. The users of the
-	 * @drm_writeback_connector control the behaviour of the @encoder
-	 * by passing the @enc_funcs parameter to drm_writeback_connector_init()
-	 * function.
-	 * For users of drm_writeback_connector_init_with_encoder(), this field
-	 * is not valid as the encoder is managed within their drivers.
-	 */
-	struct drm_encoder encoder;
-
 	/**
 	 * @pixel_formats_blob_ptr:
 	 *
@@ -148,13 +137,6 @@ drm_connector_to_writeback(struct drm_connector *connector)
 	return container_of(connector, struct drm_writeback_connector, base);
 }
 
-int drm_writeback_connector_init(struct drm_device *dev,
-				 struct drm_writeback_connector *wb_connector,
-				 const struct drm_connector_funcs *con_funcs,
-				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
-				 const u32 *formats, int n_formats,
-				 u32 possible_crtcs);
-
 int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
 				struct drm_writeback_connector *wb_connector,
 				struct drm_encoder *enc,

-- 
2.47.3

[PATCH v6 9/9] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Dmitry Baryshkov]

Rename drm_writeback_connector_init_with_encoder() to
drm_writeback_connector_init() and adapt its interface to follow
drmm_writeback_connector_init().

Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_writeback.c | 14 +++++++-------
 include/drm/drm_writeback.h     | 10 +++++-----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 9fc15168c34f..68fdac745f42 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
 }
 
 /**
- * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
+ * drm_writeback_connector_init - Initialize a writeback connector with
  * a custom encoder
  *
  * @dev: DRM device
@@ -263,11 +263,11 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
  *
  * Returns: 0 on success, or a negative error code
  */
-int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
-					      struct drm_writeback_connector *wb_connector,
-					      struct drm_encoder *enc,
-					      const struct drm_connector_funcs *con_funcs,
-					      const u32 *formats, int n_formats)
+int drm_writeback_connector_init(struct drm_device *dev,
+				 struct drm_writeback_connector *wb_connector,
+				 const struct drm_connector_funcs *con_funcs,
+				 struct drm_encoder *enc,
+				 const u32 *formats, int n_formats)
 {
 	struct drm_connector *connector = &wb_connector->base;
 	int ret;
@@ -284,7 +284,7 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
 
 	return ret;
 }
-EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
+EXPORT_SYMBOL(drm_writeback_connector_init);
 
 /**
  * drm_writeback_connector_cleanup - Cleanup the writeback connector
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
index 879ca103320c..958466a05e60 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -137,11 +137,11 @@ drm_connector_to_writeback(struct drm_connector *connector)
 	return container_of(connector, struct drm_writeback_connector, base);
 }
 
-int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
-				struct drm_writeback_connector *wb_connector,
-				struct drm_encoder *enc,
-				const struct drm_connector_funcs *con_funcs, const u32 *formats,
-				int n_formats);
+int drm_writeback_connector_init(struct drm_device *dev,
+				 struct drm_writeback_connector *wb_connector,
+				 const struct drm_connector_funcs *con_funcs,
+				 struct drm_encoder *enc,
+				 const u32 *formats, int n_formats);
 
 int drmm_writeback_connector_init(struct drm_device *dev,
 				  struct drm_writeback_connector *wb_connector,

-- 
2.47.3

Re: [PATCH v6 9/9] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The kernel-doc comment for `drm_writeback_connector_init` has mismatched parameter ordering compared to the newly adapted function signature.

Pre-existing issues:
- [Critical] Failure to initialize a subsequent writeback connector destroys global properties attached to already initialized writeback connectors, causing a Use-After-Free.
--

commit 04d131d999200c125f2670f5258a856880015f21
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm: writeback: rename drm_writeback_connector_init_with_encoder()
    
Rename drm_writeback_connector_init_with_encoder() to
drm_writeback_connector_init() and adapt its interface to follow
drmm_writeback_connector_init().

> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index 9fc15168c34f6..68fdac745f42f 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>  }

[Severity: Critical]
This isn't a bug introduced by this patch, but does the error path in
__drm_writeback_connector_init() improperly destroy global properties?

When __drm_writeback_connector_init() fails, it jumps to the
failed_properties label:

failed_properties:
	delete_writeback_properties(dev);
	return ret;

Then delete_writeback_properties() destroys the properties attached to the
device:

	if (dev->mode_config.writeback_pixel_formats_property) {
		drm_property_destroy(dev, dev->mode_config.writeback_pixel_formats_property);
		dev->mode_config.writeback_pixel_formats_property = NULL;
	}

If a driver attempts to initialize a second writeback connector and it fails,
won't this unconditionally free the global property objects that are still
attached to the first successfully initialized connector, causing a
use-after-free when those dangling pointers are later accessed?

>  
>  /**
> - * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
> + * drm_writeback_connector_init - Initialize a writeback connector with
>   * a custom encoder

[ ... ]

> @@ -263,11 +263,11 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>   *
>   * Returns: 0 on success, or a negative error code
>   */
> -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> -					      struct drm_writeback_connector *wb_connector,
> -					      struct drm_encoder *enc,
> -					      const struct drm_connector_funcs *con_funcs,
> -					      const u32 *formats, int n_formats)
> +int drm_writeback_connector_init(struct drm_device *dev,
> +				 struct drm_writeback_connector *wb_connector,
> +				 const struct drm_connector_funcs *con_funcs,
> +				 struct drm_encoder *enc,
> +				 const u32 *formats, int n_formats)

[Severity: Low]
Since the parameter order of con_funcs and enc has been swapped in the
signature, does the kernel-doc comment above this function need to be
updated?

The kernel-doc still lists @enc before @con_funcs, which creates a mismatch
between the documentation and the new function signature.

>  {
>  	struct drm_connector *connector = &wb_connector->base;
>  	int ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-wb-drop-encoder-v6-0-9f3a54f81310@oss.qualcomm.com?part=9