hdcp_destroy() cancels callback_dwork, watchdog_timer_dwork and
property_validate_dwork, but leaves cpirq_work and property_update_work
queued. These works are queued with schedule_work(), recover struct
hdcp_workqueue through container_of(), and dereference it. If either work
runs after hdcp_destroy() frees hdcp_work, it can trigger a
use-after-free.
The HDCP callbacks also call process_output(), which can requeue the HDCP
works: it always queues property_validate_dwork and may queue
callback_dwork or watchdog_timer_dwork depending on the state-machine
output. A simple cancel sequence can therefore miss work requeued by
another callback. hdcp_handle_cpirq() can also queue cpirq_work from
outside the callback path.
Set a teardown flag under hdcp_work->mutex before draining the works.
The callbacks already hold this mutex while calling process_output(), and
hdcp_handle_cpirq() takes it before queueing cpirq_work, so once teardown
is set no path can requeue work. Then cancel cpirq_work, the delayed
works, and property_update_work before freeing hdcp_work.
This bug was found by static analysis.
Fixes: a193ed2094ba ("drm/amd/display: Create amdgpu_dm_hdcp")
Fixes: da3fd7ac0bcf ("drm/amd/display: Update CP property based on HW query")
Cc: [email protected]
Signed-off-by: Fan Wu <[email protected]>
---
Changes in v2:
- Gate hdcp_handle_cpirq() with the teardown flag so late CPIRQ handling
cannot requeue cpirq_work after hdcp_destroy() starts.
---
.../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 20 ++++++++++++++++++-
.../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h | 2 ++
2 files changed, 21 insertions(+), 1 deletion(-)
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 4c164ae4a4f9..2293e9761891 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
@@ -164,6 +164,9 @@ void process_output(struct hdcp_workqueue *hdcp_work)
{
struct mod_hdcp_output output = hdcp_work->output;
+ if (hdcp_work->teardown)
+ return;
+
if (output.callback_stop)
cancel_delayed_work(&hdcp_work->callback_dwork);
@@ -310,7 +313,10 @@ void hdcp_handle_cpirq(struct hdcp_workqueue *hdcp_work,
unsigned int link_index
{
struct hdcp_workqueue *hdcp_w = &hdcp_work[link_index];
- schedule_work(&hdcp_w->cpirq_work);
+ mutex_lock(&hdcp_w->mutex);
+ if (!hdcp_w->teardown)
+ schedule_work(&hdcp_w->cpirq_work);
+ mutex_unlock(&hdcp_w->mutex);
}
static void event_callback(struct work_struct *work)
@@ -487,9 +493,21 @@ void hdcp_destroy(struct kobject *kobj, struct
hdcp_workqueue *hdcp_work)
int i = 0;
for (i = 0; i < hdcp_work->max_link; i++) {
+ /*
+ * process_output() can requeue the HDCP works. Set teardown
+ * under the callback mutex first so no callback can requeue
+ * work after destroy starts, then drain any work already
+ * queued.
+ */
+ mutex_lock(&hdcp_work[i].mutex);
+ hdcp_work[i].teardown = true;
+ mutex_unlock(&hdcp_work[i].mutex);
+
+ cancel_work_sync(&hdcp_work[i].cpirq_work);
cancel_delayed_work_sync(&hdcp_work[i].callback_dwork);
cancel_delayed_work_sync(&hdcp_work[i].watchdog_timer_dwork);
cancel_delayed_work_sync(&hdcp_work[i].property_validate_dwork);
+ cancel_work_sync(&hdcp_work[i].property_update_work);
}
sysfs_remove_bin_file(kobj, &hdcp_work[0].attr);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
index 90b18c450ca6..2313434b57c0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
@@ -54,6 +54,8 @@ struct hdcp_workqueue {
struct amdgpu_dm_connector *aconnector[AMDGPU_DM_MAX_DISPLAY_INDEX];
struct mutex mutex;
+ bool teardown;
+
struct mod_hdcp hdcp;
struct mod_hdcp_output output;
struct mod_hdcp_display display;
--
2.34.1
