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

[sashiko-bot@kernel.org]

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