Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] Missing error handling for `hdmi_frl_status_polling_wq` allocation failure allows initialization to continue, leading to a NULL pointer dereference when the workqueue is used. - [High] The `hdmi_frl_status_polling_wq` workqueue and its delayed work are never cleaned up, causing memory/thread leaks and potential use-after-free panics during driver unload. - [High] The shared `dm->hdmi_frl_status_polling_work` is unconditionally re-initialized for every connector, potentially corrupting active workqueue state and causing kernel panics. -- commit e498c7d6a60373b991eca0c000447c24090d3740 Author: Nathan Chancellor <[email protected]> drm/amd/display: Shorten hdmi_frl_status_polling_workqueue This commit shortens the name of the hdmi_frl_status_polling_wq workqueue. It prevents a kernel warning that the name exceeds WQ_NAME_LEN. > 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 2e74ff94dcacc..881626f0f258e 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -834,9 +834,9 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) > } > if (adev->dm.dc->caps.max_links > 0) { > adev->dm.hdmi_frl_status_polling_wq = > - > create_singlethread_workqueue("hdmi_frl_status_polling_workqueue"); > + > create_singlethread_workqueue("hdmi_frl_status_polling_wq"); > if (!adev->dm.hdmi_frl_status_polling_wq) > - drm_err(adev_to_drm(adev), "failed to initialize > hdmi_frl_status_polling_workqueue\n"); > + drm_err(adev_to_drm(adev), "failed to initialize > hdmi_frl_status_polling_wq\n"); [Severity: High] This isn't a bug introduced by this patch, but is it safe to continue driver initialization if create_singlethread_workqueue() fails? If adev->dm.hdmi_frl_status_polling_wq fails to allocate, it returns NULL and we log an error here, but we don't jump to the error path (unlike failures for hpd_rx_offload_wq). Later, in amdgpu_dm_atomic_commit_tail() or hdmi_frl_status_polling_work(), queue_delayed_work() is called with this potentially NULL workqueue pointer: if (queue_delayed_work(dm->hdmi_frl_status_polling_wq, &dm->hdmi_frl_status_polling_work, ...)) The kernel's workqueue API unconditionally dereferences the wq pointer in __queue_delayed_work(), so wouldn't this cause a NULL pointer dereference kernel panic? [Severity: High] This is a pre-existing issue, but is this workqueue ever cleaned up? Looking at amdgpu_dm_fini(), there appears to be no destroy_workqueue() or cancel_delayed_work_sync() for hdmi_frl_status_polling_wq and its delayed work. Could this leak the workqueue structure and its backing thread when the driver is unbound, or cause a use-after-free panic if the delayed work timer fires after the driver module has been unloaded? [Severity: High] This is another pre-existing issue, but does amdgpu_dm_connector_init_helper() corrupt the active workqueue state? In drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c, amdgpu_dm_connector_init_helper() unconditionally calls: INIT_DELAYED_WORK(&dm->hdmi_frl_status_polling_work, hdmi_frl_status_polling_work); This helper is executed for every connector, including dynamically when DisplayPort MST connectors are hotplugged via dm_dp_add_mst_connector() in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c. Because the delayed_work structure belongs to the shared global amdgpu_display_manager (dm), won't re-initializing it when an HDMI FRL stream is already actively polling overwrite the active work_struct's internal linked list pointers and timer state? > } > if (dc_is_dmub_outbox_supported(adev->dm.dc)) { > init_completion(&adev->dm.dmub_aux_transfer_done); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260618-amdgpu-fix-wq_name_len-warning-v2-1-ef0e2e6f5...@kernel.org?part=1
