[PATCH] drm/amd/display: Fix slab-use-after-free in hdcp
Alex Hung
alex.hung at amd.com
Tue Apr 22 16:07:51 UTC 2025
Reviewed-by: Alex Hung <alex.hung at amd.com>
On 4/17/25 15:50, Mario Limonciello wrote:
> From: Chris Bainbridge <chris.bainbridge at gmail.com>
>
> The HDCP code in amdgpu_dm_hdcp.c copies pointers to amdgpu_dm_connector
> objects without incrementing the kref reference counts. When using a
> USB-C dock, and the dock is unplugged, the corresponding
> amdgpu_dm_connector objects are freed, creating dangling pointers in the
> HDCP code. When the dock is plugged back, the dangling pointers are
> dereferenced, resulting in a slab-use-after-free:
>
> [ 66.775837] BUG: KASAN: slab-use-after-free in event_property_validate+0x42f/0x6c0 [amdgpu]
> [ 66.776171] Read of size 4 at addr ffff888127804120 by task kworker/0:1/10
>
> [ 66.776179] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.14.0-rc7-00180-g54505f727a38-dirty #233
> [ 66.776183] Hardware name: HP HP Pavilion Aero Laptop 13-be0xxx/8916, BIOS F.17 12/18/2024
> [ 66.776186] Workqueue: events event_property_validate [amdgpu]
> [ 66.776494] Call Trace:
> [ 66.776496] <TASK>
> [ 66.776497] dump_stack_lvl+0x70/0xa0
> [ 66.776504] print_report+0x175/0x555
> [ 66.776507] ? __virt_addr_valid+0x243/0x450
> [ 66.776510] ? kasan_complete_mode_report_info+0x66/0x1c0
> [ 66.776515] kasan_report+0xeb/0x1c0
> [ 66.776518] ? event_property_validate+0x42f/0x6c0 [amdgpu]
> [ 66.776819] ? event_property_validate+0x42f/0x6c0 [amdgpu]
> [ 66.777121] __asan_report_load4_noabort+0x14/0x20
> [ 66.777124] event_property_validate+0x42f/0x6c0 [amdgpu]
> [ 66.777342] ? __lock_acquire+0x6b40/0x6b40
> [ 66.777347] ? enable_assr+0x250/0x250 [amdgpu]
> [ 66.777571] process_one_work+0x86b/0x1510
> [ 66.777575] ? pwq_dec_nr_in_flight+0xcf0/0xcf0
> [ 66.777578] ? assign_work+0x16b/0x280
> [ 66.777580] ? lock_is_held_type+0xa3/0x130
> [ 66.777583] worker_thread+0x5c0/0xfa0
> [ 66.777587] ? process_one_work+0x1510/0x1510
> [ 66.777588] kthread+0x3a2/0x840
> [ 66.777591] ? kthread_is_per_cpu+0xd0/0xd0
> [ 66.777594] ? trace_hardirqs_on+0x4f/0x60
> [ 66.777597] ? _raw_spin_unlock_irq+0x27/0x60
> [ 66.777599] ? calculate_sigpending+0x77/0xa0
> [ 66.777602] ? kthread_is_per_cpu+0xd0/0xd0
> [ 66.777605] ret_from_fork+0x40/0x90
> [ 66.777607] ? kthread_is_per_cpu+0xd0/0xd0
> [ 66.777609] ret_from_fork_asm+0x11/0x20
> [ 66.777614] </TASK>
>
> [ 66.777643] Allocated by task 10:
> [ 66.777646] kasan_save_stack+0x39/0x60
> [ 66.777649] kasan_save_track+0x14/0x40
> [ 66.777652] kasan_save_alloc_info+0x37/0x50
> [ 66.777655] __kasan_kmalloc+0xbb/0xc0
> [ 66.777658] __kmalloc_cache_noprof+0x1c8/0x4b0
> [ 66.777661] dm_dp_add_mst_connector+0xdd/0x5c0 [amdgpu]
> [ 66.777880] drm_dp_mst_port_add_connector+0x47e/0x770 [drm_display_helper]
> [ 66.777892] drm_dp_send_link_address+0x1554/0x2bf0 [drm_display_helper]
> [ 66.777901] drm_dp_check_and_send_link_address+0x187/0x1f0 [drm_display_helper]
> [ 66.777909] drm_dp_mst_link_probe_work+0x2b8/0x410 [drm_display_helper]
> [ 66.777917] process_one_work+0x86b/0x1510
> [ 66.777919] worker_thread+0x5c0/0xfa0
> [ 66.777922] kthread+0x3a2/0x840
> [ 66.777925] ret_from_fork+0x40/0x90
> [ 66.777927] ret_from_fork_asm+0x11/0x20
>
> [ 66.777932] Freed by task 1713:
> [ 66.777935] kasan_save_stack+0x39/0x60
> [ 66.777938] kasan_save_track+0x14/0x40
> [ 66.777940] kasan_save_free_info+0x3b/0x60
> [ 66.777944] __kasan_slab_free+0x52/0x70
> [ 66.777946] kfree+0x13f/0x4b0
> [ 66.777949] dm_dp_mst_connector_destroy+0xfa/0x150 [amdgpu]
> [ 66.778179] drm_connector_free+0x7d/0xb0
> [ 66.778184] drm_mode_object_put.part.0+0xee/0x160
> [ 66.778188] drm_mode_object_put+0x37/0x50
> [ 66.778191] drm_atomic_state_default_clear+0x220/0xd60
> [ 66.778194] __drm_atomic_state_free+0x16e/0x2a0
> [ 66.778197] drm_mode_atomic_ioctl+0x15ed/0x2ba0
> [ 66.778200] drm_ioctl_kernel+0x17a/0x310
> [ 66.778203] drm_ioctl+0x584/0xd10
> [ 66.778206] amdgpu_drm_ioctl+0xd2/0x1c0 [amdgpu]
> [ 66.778375] __x64_sys_ioctl+0x139/0x1a0
> [ 66.778378] x64_sys_call+0xee7/0xfb0
> [ 66.778381] do_syscall_64+0x87/0x140
> [ 66.778385] entry_SYSCALL_64_after_hwframe+0x4b/0x53
>
> Fix this by properly incrementing and decrementing the reference counts
> when making and deleting copies of the amdgpu_dm_connector pointers.
>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4006
> Signed-off-by: Chris Bainbridge <chris.bainbridge at gmail.com>
> Cc: <Stable at vger.kernel.org>
> Fixes: da3fd7ac0bcf3 ("drm/amd/display: Update CP property based on HW query")
> (Mario: rebase on current code and update fixes tag)
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
> .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> index 2bd8dee1b7c2..26922d870b89 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> @@ -202,6 +202,9 @@ void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
> unsigned int conn_index = aconnector->base.index;
>
> guard(mutex)(&hdcp_w->mutex);
> + drm_connector_get(&aconnector->base);
> + if (hdcp_w->aconnector[conn_index])
> + drm_connector_put(&hdcp_w->aconnector[conn_index]->base);
> hdcp_w->aconnector[conn_index] = aconnector;
>
> memset(&link_adjust, 0, sizeof(link_adjust));
> @@ -249,7 +252,6 @@ static void hdcp_remove_display(struct hdcp_workqueue *hdcp_work,
> unsigned int conn_index = aconnector->base.index;
>
> guard(mutex)(&hdcp_w->mutex);
> - hdcp_w->aconnector[conn_index] = aconnector;
>
> /* the removal of display will invoke auth reset -> hdcp destroy and
> * we'd expect the Content Protection (CP) property changed back to
> @@ -265,7 +267,10 @@ static void hdcp_remove_display(struct hdcp_workqueue *hdcp_work,
> }
>
> mod_hdcp_remove_display(&hdcp_w->hdcp, aconnector->base.index, &hdcp_w->output);
> -
> + if (hdcp_w->aconnector[conn_index]) {
> + drm_connector_put(&hdcp_w->aconnector[conn_index]->base);
> + hdcp_w->aconnector[conn_index] = NULL;
> + }
> process_output(hdcp_w);
> }
>
> @@ -283,6 +288,10 @@ void hdcp_reset_display(struct hdcp_workqueue *hdcp_work, unsigned int link_inde
> for (conn_index = 0; conn_index < AMDGPU_DM_MAX_DISPLAY_INDEX; conn_index++) {
> hdcp_w->encryption_status[conn_index] =
> MOD_HDCP_ENCRYPTION_STATUS_HDCP_OFF;
> + if (hdcp_w->aconnector[conn_index]) {
> + drm_connector_put(&hdcp_w->aconnector[conn_index]->base);
> + hdcp_w->aconnector[conn_index] = NULL;
> + }
> }
>
> process_output(hdcp_w);
> @@ -517,6 +526,7 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
> struct hdcp_workqueue *hdcp_work = handle;
> struct amdgpu_dm_connector *aconnector = config->dm_stream_ctx;
> int link_index = aconnector->dc_link->link_index;
> + unsigned int conn_index = aconnector->base.index;
> struct mod_hdcp_display *display = &hdcp_work[link_index].display;
> struct mod_hdcp_link *link = &hdcp_work[link_index].link;
> struct hdcp_workqueue *hdcp_w = &hdcp_work[link_index];
> @@ -573,7 +583,10 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
> guard(mutex)(&hdcp_w->mutex);
>
> mod_hdcp_add_display(&hdcp_w->hdcp, link, display, &hdcp_w->output);
> -
> + drm_connector_get(&aconnector->base);
> + if (hdcp_w->aconnector[conn_index])
> + drm_connector_put(&hdcp_w->aconnector[conn_index]->base);
> + hdcp_w->aconnector[conn_index] = aconnector;
> process_output(hdcp_w);
> }
>
More information about the amd-gfx
mailing list