Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Unconditional free of potentially registered DRM encoder and connector on probe failure paths causes Use-After-Free. -- --- Patch [2]: [PATCH v7 2/9] drm/amd/display: use drmm allocation for writeback connector --- commit 76da3d5e9ff0ab02ea5b4f6f7fccc6c370396c0b Author: Dmitry Baryshkov <[email protected]> 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. > 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 67b825cbb88fb..1fd48e28f5fef 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c [ ... ] > @@ -2597,7 +2598,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"); > @@ -2606,7 +2607,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(adev_to_drm(adev), wbcon); > continue; > } > [ ... ] > return 0; > fail: > kfree(aencoder); > kfree(aconnector); [Severity: High] This is a pre-existing issue, but could this unconditional kfree() sequence cause a use-after-free or list corruption during the error path? In amdgpu_dm_initialize_drm_device(), aconnector and aencoder are declared outside the connector iteration loop. If a memory allocation fails for aconnector during iteration N, the code jumps to the fail label. At this point, aencoder still retains the pointer from iteration N-1. Since that previous encoder was successfully initialized and linked into the DRM device's lists, freeing it directly here leaves a dangling pointer. When drm_mode_config_cleanup() iterates over the registered DRM objects during teardown, wouldn't it access this freed memory? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
