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

[sashiko-bot@kernel.org]

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