All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/amd/display: Shorten hdmi_frl_status_polling_workqueue
@ 2026-06-18 16:52 Nathan Chancellor
  2026-06-18 17:01 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Nathan Chancellor @ 2026-06-18 16:52 UTC (permalink / raw)
  To: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, Fangzhi Zuo
  Cc: amd-gfx, dri-devel, linux-kernel, Alex Hung, Nathan Chancellor

There is a warning when creating the hdmi_frl_status_polling_wq
workqueue because "hdmi_frl_status_polling_workqueue" excceds
WQ_NAME_LEN:

  workqueue: name exceeds WQ_NAME_LEN. Truncating to: hdmi_frl_status_polling_workque

Shorten the workqueue name to "hdmi_frl_status_polling_wq" like the
structure member to avoid the warning.

Fixes: 5c9b8b27a883 ("drm/amd/display: Tie FRL support into amdgpu_dm")
Reviewed-by: Alex Hung <alex.hung@amd.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
Changes in v2:
- Rebase to solve conflicts with 0e967e086e75 ("drm/amd/display: Extract
  connector and encoder code to amdgpu_dm_connector").
- Pick up Alex's Reviewed-by.
- Link to v1: https://patch.msgid.link/20260604-amdgpu-fix-wq_name_len-warning-v1-1-eb5415b45b27@kernel.org
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 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 2e74ff94dcac..881626f0f258 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");
 	}
 	if (dc_is_dmub_outbox_supported(adev->dm.dc)) {
 		init_completion(&adev->dm.dmub_aux_transfer_done);

---
base-commit: 991fd2cb908bf5d35a496760519442d6e9f8763d
change-id: 20260604-amdgpu-fix-wq_name_len-warning-63c8012e0c2b

Best regards,
--  
Cheers,
Nathan


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] drm/amd/display: Shorten hdmi_frl_status_polling_workqueue
  2026-06-18 16:52 [PATCH v2] drm/amd/display: Shorten hdmi_frl_status_polling_workqueue Nathan Chancellor
@ 2026-06-18 17:01 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-18 17:01 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: dri-devel

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 <nathan@kernel.org>

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-ef0e2e6f5be7@kernel.org?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-18 17:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 16:52 [PATCH v2] drm/amd/display: Shorten hdmi_frl_status_polling_workqueue Nathan Chancellor
2026-06-18 17:01 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.